linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@infradead.org>
To: Ming Lei <ming.lei@redhat.com>
Cc: Christoph Hellwig <hch@infradead.org>,
	Jens Axboe <axboe@kernel.dk>,
	linux-block@vger.kernel.org, Sagi Grimberg <sagi@grimberg.me>,
	Baolin Wang <baolin.wang7@gmail.com>
Subject: Re: [PATCH 3/9] blk-mq: don't predicate last flag in blk_mq_dispatch_rq_list
Date: Wed, 13 May 2020 22:50:02 -0700	[thread overview]
Message-ID: <20200514055002.GA22388@infradead.org> (raw)
In-Reply-To: <20200514005043.GE2073570@T590>

On Thu, May 14, 2020 at 08:50:43AM +0800, Ming Lei wrote:
> On Wed, May 13, 2020 at 05:27:53AM -0700, Christoph Hellwig wrote:
> > On Wed, May 13, 2020 at 05:54:37PM +0800, Ming Lei wrote:
> > > .commit_rqs() is supposed to handle partial dispatch when driver may not
> > > see .last of flag passed to .queue_rq().
> > > 
> > > We have added .commit_rqs() in case of partial dispatch and all consumers
> > > of bd->last have implemented .commit_rqs() callback, so it is perfect to
> > > pass real .last flag of the request list to .queue_rq() instead of faking
> > > it by trying to allocate driver tag for next request in the batching list.
> > 
> > The current case still seems like a nice optimization to avoid an extra
> > indirect function call.  So if you want to get rid of it I think it at
> > least needs a better rationale.
> 
> You mean marking .last by trying to allocate for next request can
> replace .commit_rqs()? No, it can't because .commit_rqs() can be
> called no matter .last is set or not, both two are independent.
> 
> Removing it can avoid to pre-allocate one extra driver tag, and
> improve driver tag's utilization.

What I said is that the current scheme works, and the new one will
need an additional indirect function call in various scenarios.  The
commit log doesn't really "sell" that change very well.  Your new
explanation is much better, as would be saying it helps you with
the hanges in this series.

  reply	other threads:[~2020-05-14  5:50 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-13  9:54 [PATCH 0/9] blk-mq: support batching dispatch from scheduler Ming Lei
2020-05-13  9:54 ` [PATCH 1/9] blk-mq: pass request queue into get/put budget callback Ming Lei
2020-05-13 10:06   ` Johannes Thumshirn
2020-05-13 12:24   ` Christoph Hellwig
2020-05-13 14:36   ` Doug Anderson
2020-05-13 22:48   ` Sagi Grimberg
2020-05-13  9:54 ` [PATCH 2/9] blk-mq: pass hctx to blk_mq_dispatch_rq_list Ming Lei
2020-05-13 12:26   ` Christoph Hellwig
2020-05-13 22:49   ` Sagi Grimberg
2020-05-13  9:54 ` [PATCH 3/9] blk-mq: don't predicate last flag in blk_mq_dispatch_rq_list Ming Lei
2020-05-13 12:27   ` Christoph Hellwig
2020-05-14  0:50     ` Ming Lei
2020-05-14  5:50       ` Christoph Hellwig [this message]
2020-05-14  2:09     ` Ming Lei
2020-05-14  2:19       ` Ming Lei
2020-05-14  3:21       ` Keith Busch
2020-05-14  8:28         ` Ming Lei
2020-05-13  9:54 ` [PATCH 4/9] blk-mq: move getting driver tag and bugget into one helper Ming Lei
2020-05-13 12:37   ` Christoph Hellwig
2020-05-13 22:54   ` Sagi Grimberg
2020-05-13  9:54 ` [PATCH 5/9] blk-mq: move .queue_rq code " Ming Lei
2020-05-13 12:38   ` Christoph Hellwig
2020-05-13  9:54 ` [PATCH 6/9] blk-mq: move code for handling partial dispatch " Ming Lei
2020-05-13 12:56   ` Christoph Hellwig
2020-05-13 13:01     ` Christoph Hellwig
2020-05-14  1:25       ` Ming Lei
2020-05-13  9:54 ` [PATCH 7/9] blk-mq: remove dead check from blk_mq_dispatch_rq_list Ming Lei
2020-05-13 12:57   ` Christoph Hellwig
2020-05-13 23:24   ` Sagi Grimberg
2020-05-13  9:54 ` [PATCH 8/9] blk-mq: pass obtained budget count to blk_mq_dispatch_rq_list Ming Lei
2020-05-13 13:26   ` Christoph Hellwig
2020-05-13  9:54 ` [PATCH 9/9] blk-mq: support batching dispatch in case of io scheduler Ming Lei
2020-05-23  7:45 ` [PATCH 0/9] blk-mq: support batching dispatch from scheduler Baolin Wang
2020-05-25  2:17   ` Ming Lei

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200514055002.GA22388@infradead.org \
    --to=hch@infradead.org \
    --cc=axboe@kernel.dk \
    --cc=baolin.wang7@gmail.com \
    --cc=linux-block@vger.kernel.org \
    --cc=ming.lei@redhat.com \
    --cc=sagi@grimberg.me \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).