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
