On Wed, Jul 29, 2020 at 09:12:29AM -0700, Keith Busch wrote:
> On Tue, Jul 28, 2020 at 09:49:38PM +0800, Ming Lei 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
We still can handle this case by not dispatch in case that percpu_ref_tryget()
returns false, which will change the usage into the following way:
if (hctx_lock(hctx)) {
blk_mq_sched_dispatch_requests(hctx);
hctx_unlock(hctx);
}
And __blk_mq_try_issue_directly() needs a bit special treatment because
the request has to be inserted to queue after queue becomes quiesced.
Thanks,
Ming
