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

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

On Wed, Sep 09, 2020 at 01:53:30PM -0700, Sagi Grimberg wrote:
> 
> > >   void blk_mq_quiesce_queue(struct request_queue *q)
> > >   {
> > > -	struct blk_mq_hw_ctx *hctx;
> > > -	unsigned int i;
> > > -	bool rcu = false;
> > > +	bool blocking = !!(q->tag_set->flags & BLK_MQ_F_BLOCKING);
> > > +	bool was_quiesced =__blk_mq_quiesce_queue_nowait(q);
> > > -	__blk_mq_quiesce_queue_nowait(q);
> > > +	if (!was_quiesced && blocking)
> > > +		percpu_ref_kill(&q->dispatch_counter);
> > > -	queue_for_each_hw_ctx(q, hctx, i) {
> > > -		if (hctx->flags & BLK_MQ_F_BLOCKING)
> > > -			synchronize_srcu(hctx->srcu);
> > > -		else
> > > -			rcu = true;
> > > -	}
> > > -	if (rcu)
> > > +	if (blocking)
> > > +		wait_event(q->mq_quiesce_wq,
> > > +				percpu_ref_is_zero(&q->dispatch_counter));
> > > +	else
> > >   		synchronize_rcu();
> > >   }
> > 
> > In the previous version, you had ensured no thread can unquiesce a queue
> > while another is waiting for quiescence. Now that the locking is gone,
> > a thread could unquiesce the queue before percpu_ref reaches zero, so
> > the wait_event() may never complete on the resurrected percpu_ref.
> 
> Yea, where did that go?

The mutex is removed because:

1) As Bart mentioned, blk_mq_quiesce_queue() may be called in context
which doesn't allow sleep.

2) Both percpu_ref_kill() and percpu_ref_resurrect() have been protected by
one global spinlock, so both two can be run concurrently.

3) warning may be triggered when percpu_ref_kill() is run on one DEAD
percpu-refcount, or when percpu_ref_resurrect() is run on one live
percpu-refcount. We can avoid the warning with test_and_{clear|test}_bit
exactly by running the actual quiesce/unquiesce action only once.


Thanks,
Ming


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme