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:

quoted

+	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-

quoted

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.