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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux