Re: [RFC PATCH V2 04/17] blk-mq: don't reserve tags for admin queue

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

 



On Tue, Aug 14, 2018 at 10:47:21AM +0800, jianchao.wang wrote:
> Hi Ming
> 
> On 08/14/2018 10:10 AM, Ming Lei wrote:
> > On Tue, Aug 14, 2018 at 09:29:25AM +0800, jianchao.wang wrote:
> >> Hi Ming
> >>
> >> On 08/13/2018 06:48 PM, Ming Lei wrote:
> >>> It is nothing to do with where the admin request is sent, so no any
> >>> difference wrt. this issue between with and without this patchset,
> >>> right?
> >>
> >> I'm afraid not.
> >>
> >> For example:
> >>   A scsi host has 8 LUNs associated with it.
> >>   Before this patch set,
> >>   When we send out the admin command, the budget is _per_ LUN, 1/8 of the total tags.
> >>   After this patch set,
> >>   When we send out the admin command, the budget is equal to _one_ LUN, 1/8 of the total tags.
> >>
> >> However, the 1/8 above is different.
> >>   Before the patch set, every LUN's admin command has 1/8 budget to use which is per LUN.
> > 
> > Strictly speaking, it is that all admin command and all other IOs share the 1/8 budget
> > if they aimed at same LUN.
> 
> Yes.
> 
> > 
> >>   After this patch set, all the 8 LUNs admin command has to share the 1/8 budget.
> > 
> > That only means number of active admin commands won't be bigger than 1/8 budget, which
> > is one extra implicit limit on admin queue. However, other LUN's budget is still 1/8.
> > 
> > So performance for IO queue won't be affected at all, will it?
> > 
> > scsi_execute_* can't be called often, it is really in slow path, so I
> > don't think there is any possible performance effect with this patch, or do
> > you have other performance concern wrt. this patch?
> > 
> > We still have q->queue_depth for enhancing any limit for admin queue, but up to now,
> > not see it is necessary.
> > 
> 
> I agree with you that the performance will not be affected.

OK, thanks for your confirm.

> But the adminq's budget here looks weird.
> We don't reserve budget for admin queue (not count tag->active_queues for it).
> But the admin queue has to comply to the limit in hctx_may_queue.
> 
> Since the there isn't many in-flight admin commands usually, we could take
> admin queue here out of the limit of hctx_may_queue, then things could be clearer. :)

I just didn't want to add one line code in the fast path of hctx_may_queue()
because it isn't necessary.

Now looks this way may have one implicit benefit: avoid too many in-flight admin
requests.

I will leave the code in this way, but add comment like below to hctx_may_queue():

	Needn't to deal with admin queue specially here even though we don't
	take it account to tags->active_queues, so blk_queue_admin() can be
	avoided to check in the fast path, also with implicit benefit of
	limiting too many in-flight admin requests.


Thanks, 
Ming



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux