On Fri, Aug 25, 2017 at 09:41:08PM +0000, Bart Van Assche wrote:
> On Thu, 2017-08-24 at 12:52 +0800, Ming Lei wrote:
> > 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.
>
> The purpose of my comment was not to ask for further clarification but to
> report that the description of this patch is not correct.
OK, will change the commit log in V3.
>
> > >
> > > > +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.
>
> Sorry but I disagree with both of the above statements.
I will post out V3, please comment it on that patch about this issue,
especially total round robin is added.
--
Ming
