From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Thu, 27 Apr 2017 11:14:53 +0800 From: Ming Lei To: Jens Axboe Cc: Christoph Hellwig , linux-block@vger.kernel.org, Omar Sandoval , Jozef Mikovic Subject: Re: [PATCH 0/4] blk-mq-sched: allow to use hw tag for sched Message-ID: <20170427031445.GB4841@ming.t460p> References: <20170415123825.32716-1-ming.lei@redhat.com> <20170420004410.GA16917@ming.t460p> <90e249d6-996c-a8d9-f54d-e8142082bfa5@fb.com> <20170420010346.GC16917@ming.t460p> <20170420045408.GB2235@infradead.org> <20170420083041.GA1637@ming.t460p> <20170426104842.GB28062@ming.t460p> <38bc590d-8ab4-5c88-d7e7-10cd2ec7d900@kernel.dk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii In-Reply-To: List-ID: On Wed, Apr 26, 2017 at 11:22:43AM -0700, Jens Axboe wrote: > On 04/26/2017 11:15 AM, Jens Axboe wrote: > > On 04/26/2017 03:48 AM, Ming Lei wrote: > >> Hi Christoph, > >> > >> On Thu, Apr 20, 2017 at 04:30:42PM +0800, Ming Lei wrote: > >>> Hi Christoph, > >>> > >>> On Wed, Apr 19, 2017 at 09:54:08PM -0700, Christoph Hellwig wrote: > >>>> On Thu, Apr 20, 2017 at 09:03:47AM +0800, Ming Lei wrote: > >>>>> If we don't need to reserve tag for internal command, I am happy > >>>>> to fix it in the interim. However, the mtip32xx driver has to > >>>>> ask blk-mq to reserve the tag zero for internal command. Then > >>>>> if we can't allow the driver to use that reserved tag, it looks > >>>>> quite awkward, doesn't it? > >>>> > >>>> It doesn't. Just offset the hardware tags by 1 from the blk-mq tags. > >>>> > >>>> But I'm pretty sure the hardware wouldn't require a specific tag > >>>> for the internal ops. > >>> > >>> From mtip32xx code, the tag 0 is used to send internal command only, and > >>> not used for submitting normal request. > >>> > >>> From code of mtip_issue_non_ncq_command() and mtip_issue_ncq_command(), > >>> even same hardware address is writen to when issuing non-ncq and ncq > >>> commands. So I think the internal ops and normal requests share one same > >>> tag space on mtip32xx. > >>> > >>> Could you explain it a bit why you think the hw doesn't need the tag > >>> for internal ops in this case? > >> > >> Gentle ping... > >> > >> Looks there are some choices for this issue: > >> > >> 1) if internal ops uses independent tag space > >> - we need to clean up the mtip32xx driver > >> > >> 2) if internal ops shares tag space with normal request > >> - export blk_mq_get_driver_tag() for mtip32xx > >> > >> or > >> > >> - use private way to send internal ops, and the drawback > >> is that we still need to reserve tag and the reserved tag > >> can't be used at all. > >> > >> From mtip32xx source code, I can see both share same tag space. > >> When I try to search hw manual via google, not succeed yet. > >> > >> Christoph, could you share us your findings about if internal ops > >> uses independent tag space or not? > > > > Why aren't we just bypassing for RESERVED? If someone is asking > > for an reserved tag, ensure that we use the right tag pool (sched > > vs normal) and sub pool (reserved vs normal). Then insert just > > bypasses the scheduler, like it already does if we have a driver > > tag assigned already. > > > > I've got a few of these cards, but traveling. I'll wire up something > > and test it end this week. > > Something like this. Totally untested. > > diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c > index c974a1bbf4cb..11109b5b64b6 100644 > --- a/block/blk-mq-sched.c > +++ b/block/blk-mq-sched.c > @@ -119,7 +119,11 @@ struct request *blk_mq_sched_get_request(struct request_queue *q, > if (likely(!data->hctx)) > data->hctx = blk_mq_map_queue(q, data->ctx->cpu); > > - if (e) { > + /* > + * For a reserved tag, allocate a normal request since we might > + * have driver dependencies on the value of the internal tag. > + */ > + if (e && !(data->flags & BLK_MQ_REQ_RESERVED)) { > data->flags |= BLK_MQ_REQ_INTERNAL; > > /* This one looks reasonable, since reserved rqs often needn't scheduling, also works well on mtip32xx with another patch[1] together. Tested-by: Ming Lei [1] http://marc.info/?l=linux-block&m=149256194703481&w=2 Thanks, Ming