On Wed, Jul 29, 2020 at 03:42:29PM -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;
> > > > -
> > > > blk_mq_quiesce_queue_nowait(q);
> > > > - 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 (q->tag_set->flags & BLK_MQ_F_BLOCKING) {
> > > > + percpu_ref_kill(&q->dispatch_counter);
> > > > + wait_event(q->mq_quiesce_wq,
> > > > + percpu_ref_is_zero(&q->dispatch_counter));
> > > > + } else
> > > > synchronize_rcu();
> > > > }
> > >
> > >
> > >
> > > > +static void hctx_lock(struct blk_mq_hw_ctx *hctx)
> > > > {
> > > > - if (!(hctx->flags & BLK_MQ_F_BLOCKING)) {
> > > > - /* shut up gcc false positive */
> > > > - *srcu_idx = 0;
> > > > + if (!(hctx->flags & BLK_MQ_F_BLOCKING))
> > > > rcu_read_lock();
> > > > - } else
> > > > - *srcu_idx = srcu_read_lock(hctx->srcu);
> > > > + else
> > > > + percpu_ref_get(&hctx->queue->dispatch_counter);
> > > > }
> > >
> > > percpu_ref_get() will always succeed, even after quiesce kills it.
> > > Isn't it possible that 'percpu_ref_is_zero(&q->dispatch_counter))' may
> > > never reach 0? We only need to ensure that dispatchers will observe
> > > blk_queue_quiesced(). That doesn't require that there are no current
> > > dispatchers.
> >
> > IMO it shouldn't be one issue in reality, because:
> >
> > - when dispatch can't make progress, the submission side will finally
> > stop because we either run queue from submission side or request
> > completion
> > - submission side stops because we always have very limited requests
> >
> > - completion side stops because requests queued to device is limited
> > too
>
> I don't think that any requests should pass after the kill was called,
> otherwise how can we safely quiesce if requests can come in after
> it?
What we guarantee is that no request can be queued to LLD after
blk_mq_quiesce_queue returns.
With percpu_refcount, once percpu_ref_is_zero(&q->dispatch_counter)
returns true, all code path can observe the QUIESCED flag reliably just
like what SRCU does, so no any request can pass to LLD after blk_mq_quiesce_queue
returns.
>
> >
> > We still can handle this case by not dispatch in case that percpu_ref_tryget()
>
> You meant tryget_live right?
Both works, but tryget_live could be better.
Thanks,
Ming
