Re: [PATCH 4/5] nvme: quiesce namespace queue in parallel

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

On Mon, Nov 22, 2021 at 10:07:24AM +0200, Sagi Grimberg wrote:
> 
> 
> On 11/19/21 4:18 AM, Ming Lei wrote:
> > Chao Leng reported that in case of lots of namespaces, it may take quite a
> > while for nvme_stop_queues() to quiesce all namespace queues.
> > 
> > Improve nvme_stop_queues() by running quiesce in parallel, and just wait
> > once if global quiesce wait is allowed.
> > 
> > Link: https://lore.kernel.org/linux-block/cc732195-c053-9ce4-e1a7-e7f6dcf762ac@huawei.com/
> > Reported-by: Chao Leng <lengchao@huawei.com>
> > Signed-off-by: Ming Lei <ming.lei@redhat.com>
> > ---
> >   drivers/nvme/host/core.c | 9 ++++++---
> >   1 file changed, 6 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> > index 4b5de8f5435a..06741d3ed72b 100644
> > --- a/drivers/nvme/host/core.c
> > +++ b/drivers/nvme/host/core.c
> > @@ -4517,9 +4517,7 @@ static void nvme_start_ns_queue(struct nvme_ns *ns)
> >   static void nvme_stop_ns_queue(struct nvme_ns *ns)
> >   {
> >   	if (!test_and_set_bit(NVME_NS_STOPPED, &ns->flags))
> > -		blk_mq_quiesce_queue(ns->queue);
> > -	else
> > -		blk_mq_wait_quiesce_done(ns->queue);
> > +		blk_mq_quiesce_queue_nowait(ns->queue);
> >   }
> >   /*
> > @@ -4620,6 +4618,11 @@ void nvme_stop_queues(struct nvme_ctrl *ctrl)
> >   	down_read(&ctrl->namespaces_rwsem);
> >   	list_for_each_entry(ns, &ctrl->namespaces, list)
> >   		nvme_stop_ns_queue(ns);
> > +	list_for_each_entry(ns, &ctrl->namespaces, list) {
> > +		blk_mq_wait_quiesce_done(ns->queue);
> > +		if (blk_mq_global_quiesce_wait(ns->queue))
> > +			break;
> > +	}
> >   	up_read(&ctrl->namespaces_rwsem);
> 
> 
> Can you quantify how much of a difference it is to do rcu_read_unlock()
> for every queue? The big improvement here is that it is done in parallel
> instead of serially. Just wandering if it is worth the less than elegant
> interface...

The biggest improvement is N * synchronize_rcu() -> 1 * synchronize_rcu()
in case of non blocking, that is what Chao Leng complained before.

Even for blocking case, the parallel quiesce is still good, such as, when
one synchronize_srcu() is done, other srcu syncs should be done usually too.


thanks, 
Ming