Re: [PATCH V2 0/2] blk-mq: implement queue quiesce via percpu_ref for BLK_MQ_F_BLOCKING

[Ming Lei <ming.lei@redhat.com>]

On Wed, Sep 02, 2020 at 11:20:36AM -0700, Sagi Grimberg wrote:
> 
> > > > Hi Jens,
> > > > 
> > > > The 1st patch add .mq_quiesce_mutex for serializing quiesce/unquiesce,
> > > > and prepares for replacing srcu with percpu_ref.
> > > > 
> > > > The 2nd patch replaces srcu with percpu_ref.
> > > > 
> > > > V2:
> > > > 	- add .mq_quiesce_lock
> > > > 	- add comment on patch 2 wrt. handling hctx_lock() failure
> > > > 	- trivial patch style change
> > > > 
> > > > 
> > > > Ming Lei (2):
> > > >    blk-mq: serialize queue quiesce and unquiesce by mutex
> > > >    blk-mq: implement queue quiesce via percpu_ref for BLK_MQ_F_BLOCKING
> 
> I thought we agreed to have a little more consolidation for blocking and
> !blocking paths (move fallbacks to common paths).

Could you describe the consolidation one more time for the two paths?

BTW, code will become a little messy if we move queue quiesce handling
out of __blk_mq_try_issue_directly(), because we have two conditions to
trigger insert request into scheduler queue:

1) hctx_lock() failure

2) blk_queue_quiesced() or blk_mq_hctx_stopped()

The former doesn't need to unlock hctx, however the latter needs that,
that is why I don't do the change if that is the consolidation you
mentioned.

> 
> > > > 
> > > >   block/blk-core.c       |   2 +
> > > >   block/blk-mq-sysfs.c   |   2 -
> > > >   block/blk-mq.c         | 125 +++++++++++++++++++++++------------------
> > > >   block/blk-sysfs.c      |   6 +-
> > > >   include/linux/blk-mq.h |   7 ---
> > > >   include/linux/blkdev.h |   6 ++
> > > >   6 files changed, 82 insertions(+), 66 deletions(-)
> > > > 
> > > > Cc: Lai Jiangshan <jiangshanlai@gmail.com>
> > > > Cc: Paul E. McKenney <paulmck@kernel.org>
> > > > Cc: Josh Triplett <josh@joshtriplett.org>
> > > > Cc: Sagi Grimberg <sagi@grimberg.me>
> > > > Cc: Bart Van Assche <bvanassche@acm.org>
> > > > Cc: Johannes Thumshirn <Johannes.Thumshirn@wdc.com>
> > > > Cc: Chao Leng <lengchao@huawei.com>
> > > > Cc: Christoph Hellwig <hch@lst.de>
> > > 
> > > Hello Guys,
> > > 
> > > Is there any objections on the two patches? If not, I'd suggest to move> on.
> > 
> > Seems like the nested case is one that should either be handled, or at
> > least detected.
> 
> Personally, I'd like to see the async quiesce piece as well here, which
> is the reason why this change was proposed. Don't see a strong urgency
> to move forward with it before that, especially as this could
> potentially affect various non-trivial reset flows.

OK, it shouldn't be easy to add the interface, will do that in next
version.


Thanks, 
Ming