Re: [PATCH V2 03/20] blk-mq: introduce blk_mq_dispatch_rq_from_ctx()

Ming Lei
2017-08-24T04:52:40Z

On Tue, Aug 22, 2017 at 06:45:46PM +0000, Bart Van Assche wrote:
> On Sat, 2017-08-05 at 14:56 +0800, Ming Lei wrote:
> > More importantly, for some SCSI devices, driver
> > tags are host wide, and the number is quite big,
> > but each lun has very limited queue depth.
>
> This may be the case but is not always the case. Another important use-case
> is one LUN per host and where the queue depth per LUN is identical to the
> number of host tags.

This patchset won't hurt this case because the BUSY info is returned
from driver. In this case, BLK_STS_RESOURCE should seldom be returned
from .queue_rq generally.

Also one important fact is that once q->queue_depth is set, that
means there is per-request_queue limit on pending I/Os, and the
single LUN is just the special case which is covered by this whole
patchset. We don't need to pay special attention in this special case
at all.

>
> > +struct request *blk_mq_dispatch_rq_from_ctx(struct blk_mq_hw_ctx *hctx,
> > + struct blk_mq_ctx *start)
> > +{
> > + unsigned off = start ? start->index_hw : 0;
>
> Please consider to rename this function into blk_mq_dispatch_rq_from_next_ctx()
> and to start from start->index_hw + 1 instead of start->index_hw. I think that
> will not only result in simpler but also in faster code.

I believe this helper with blk_mq_next_ctx(hctx, rq->mq_ctx) together
will be much simpler and easier to implement, and code can be much
readable too.

blk_mq_dispatch_rq_from_next_ctx() is ugly and mixing two things
together.

Please see the actual implementation in patch of 'blk-mq-sched: improve
dispatching from sw queue'.

--
Ming