On Wed, 2018-07-18 at 20:16 +0800, Ming Lei wrote:
> On Wed, Jul 18, 2018 at 7:49 AM, Bart Van Assche <[email protected]>
> wrote:
> > @@ -3801,8 +3778,11 @@ int blk_pre_runtime_suspend(struct request_queue *q)
> > if (!q->dev)
> > return ret;
> >
> > + blk_set_preempt_only(q);
> > + blk_freeze_queue_start(q);
> > +
> > spin_lock_irq(q->queue_lock);
> > - if (q->nr_pending) {
> > + if (!percpu_ref_is_zero(&q->q_usage_counter)) {
>
> This way can't work reliably because the percpu ref isn't in atomic mode
> yet after blk_freeze_queue_start() returns, then percpu_ref_is_zero() won't
> see accurate value of the counter, finally the device may be put down before
> in-flight requests are completed by hardware.
Hello Ming,
The blk_freeze_queue_start() implementation is as follows:
void blk_freeze_queue_start(struct request_queue *q)
{
int freeze_depth;
freeze_depth = atomic_inc_return(&q->mq_freeze_depth);
if (freeze_depth == 1) {
percpu_ref_kill(&q->q_usage_counter);
if (q->mq_ops)
blk_mq_run_hw_queues(q, false);
}
}
>From the documentation header in include/linux/percpu-refcount.h above
percpu_ref_kill():
* Switches @ref into atomic mode before gathering up the percpu counters
* and dropping the initial ref.
In other words, I think that after blk_freeze_queue_start() returns that it is
guaranteed that q->q_usage_counter is in atomic mode. However, we may need to
serialize concurrent blk_freeze_queue_start() calls to guarantee that this is
always the case if multiple threads call blk_freeze_queue_start() concurrently.
Bart.
