On Thu, Sep 03, 2020 at 05:37:02AM -0700, Keith Busch wrote:
> On Thu, Sep 03, 2020 at 08:35:45AM +0800, Ming Lei wrote:
> > On Wed, Sep 02, 2020 at 11:52:59AM -0600, Jens Axboe wrote:
> > > On 9/1/20 9:11 PM, Ming Lei wrote:
> > > > On Tue, Aug 25, 2020 at 10:17:32PM +0800, Ming Lei 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
> > > >>
> > > >> 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@xxxxxxxxx>
> > > >> Cc: Paul E. McKenney <paulmck@xxxxxxxxxx>
> > > >> Cc: Josh Triplett <josh@xxxxxxxxxxxxxxxx>
> > > >> Cc: Sagi Grimberg <sagi@xxxxxxxxxxx>
> > > >> Cc: Bart Van Assche <bvanassche@xxxxxxx>
> > > >> Cc: Johannes Thumshirn <Johannes.Thumshirn@xxxxxxx>
> > > >> Cc: Chao Leng <lengchao@xxxxxxxxxx>
> > > >> Cc: Christoph Hellwig <hch@xxxxxx>
> > > >
> > > > 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.
> >
> > Yeah, the 1st patch adds mutex for handling nested case correctly and efficiently.
>
> That doesn't really do anything about handling nested quiesce/unquiesce.
The mutex is required for avoiding warning in percpu_ref_kill_and_confirm() if
two queue quiesce are nested, and I will comment on this motivation in next
version.
> It just prevents two threads from doing it at the same time, but anyone
> can still undo the other's expected queue state. The following on top of
Right, the patch itself changes nothing wrt. this point, and it breaks nothing
too.
> your series will at least detect the condition:
>
> ---
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index ef6c6fa8dab0..52b53f2bb567 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -249,6 +249,7 @@ void blk_mq_unquiesce_queue(struct request_queue *q)
> {
> mutex_lock(&q->mq_quiesce_lock);
>
> + WARN_ON(!blk_queue_quiesced(q));
We can't do that simply, because queue unquiesce may be called
unconditionally on un-quiesced queue, such as nvme_dev_remove_admin(),
nvme_set_queue_dying(), ...
Thanks,
Ming
