On Thu 07-01-21 14:19:18, Ming Lei wrote:
> On Wed, Jan 06, 2021 at 11:24:28AM +0100, Jan Kara wrote:
> > +/* Check if there are requests queued in hctx lists. */
> > +static bool blk_mq_hctx_has_queued_rq(struct blk_mq_hw_ctx *hctx)
> > +{
> > + return !list_empty_careful(&hctx->dispatch) ||
> > + sbitmap_any_bit_set(&hctx->ctx_map);
> > +}
> > +
>
> blk_mq_hctx_mark_pending() is only called in case of none scheduler, so
> looks not necessary to check hctx->ctx_map in blk_mq_hctx_has_queued_rq()
> which is supposed to be used when real io scheduler is attached to MQ queue.
Yes, I know. I just wanted to make the code less fragile... In particular I
was somewhat uneasy that we'd rely on the implicit behavior that
blk_mq_get_sqsched_hctx() can return non-NULL only if sbitmap_any_bit_set()
is not needed. But maybe we could structure the code like:
sq_hctx = NULL;
if (blk_mq_has_sqsched(q))
sq_hctx = blk_mq_get_sq_hctx(q);
queue_for_each_hw_ctx(q, hctx, i) {
...
if (!sq_hctx || sq_hctx == hctx ||
!list_empty_careful(&hctx->dispatch))
... run ...
}
Because then it is kind of obvious that sq_hctx is set only if there's IO
scheduler for the queue and thus ctx_map is unused. What do you think?
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
