On Thu, 2018-09-20 at 11:48 +0800, Ming Lei wrote:
> On Wed, Sep 19, 2018 at 03:45:29PM -0700, Bart Van Assche wrote:
> > + ret = -EBUSY;
> > + if (blk_requests_in_flight(q) == 0) {
> > + blk_freeze_queue_start(q);
> > + /*
> > + * Freezing a queue starts a transition of the
> > queue
> > + * usage counter to atomic mode. Wait until atomic
> > + * mode has been reached. This involves calling
> > + * call_rcu(). That call guarantees that later
> > + * blk_queue_enter() calls see the pm-only state.
> > See
> > + * also http://lwn.net/Articles/573497/.
> > + */
> > + percpu_ref_switch_to_atomic_sync(&q-
> > >q_usage_counter);
> > + if (percpu_ref_is_zero(&q->q_usage_counter))
> > + ret = 0;
> > + blk_mq_unfreeze_queue(q);
>
> Tejun doesn't agree on this kind of usage yet, so the ref has to be
> dropped before calling blk_mq_unfreeze_queue().
I read all Tejuns' recent e-mails but I have not found any e-mail from
Tejun in which he wrote that he disagrees with the above pattern.
> Also, this way still can't address the race in the following link:
>
> https://marc.info/?l=linux-block&m=153732992701093&w=2
I think that the following patch is sufficient to fix that race:
diff --git a/block/blk-core.c b/block/blk-core.c
index ae092ca121d5..16dd3a989753 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -942,8 +942,6 @@ int blk_queue_enter(struct request_queue *q,
blk_mq_req_flags_t flags)
if (success)
return 0;
- blk_pm_request_resume(q);
-
if (flags & BLK_MQ_REQ_NOWAIT)
return -EBUSY;
@@ -958,7 +956,8 @@ int blk_queue_enter(struct request_queue *q,
blk_mq_req_flags_t flags)
wait_event(q->mq_freeze_wq,
(atomic_read(&q->mq_freeze_depth) == 0 &&
- (pm || !blk_queue_pm_only(q))) ||
+ (pm || (blk_pm_request_resume(q),
+ !blk_queue_pm_only(q)))) ||
blk_queue_dying(q));
if (blk_queue_dying(q))
return -ENODEV;
Bart.
