Thread (35 messages) 35 messages, 3 authors, 2017-05-31

Re: [PATCH v2 4/8] blk-mq: fix blk_mq_quiesce_queue

From: Ming Lei <hidden>
Date: 2017-05-30 00:22:49

On Sun, May 28, 2017 at 04:10:09PM +0000, Bart Van Assche wrote:
On Sun, 2017-05-28 at 18:44 +0800, Ming Lei wrote:
quoted
On Sat, May 27, 2017 at 09:46:45PM +0000, Bart Van Assche wrote:
quoted
On Sat, 2017-05-27 at 22:21 +0800, Ming Lei wrote:
quoted
 bool blk_mq_can_queue(struct blk_mq_hw_ctx *hctx)
@@ -1108,13 +1119,15 @@ static void __blk_mq_run_hw_queue(struct blk_mq_hw_ctx *hctx)
 
 	if (!(hctx->flags & BLK_MQ_F_BLOCKING)) {
 		rcu_read_lock();
-		blk_mq_sched_dispatch_requests(hctx);
+		if (!blk_queue_quiesced(hctx->queue))
+			blk_mq_sched_dispatch_requests(hctx);
 		rcu_read_unlock();
 	} else {
 		might_sleep();
 
 		srcu_idx = srcu_read_lock(&hctx->queue_rq_srcu);
-		blk_mq_sched_dispatch_requests(hctx);
+		if (!blk_queue_quiesced(hctx->queue))
+			blk_mq_sched_dispatch_requests(hctx);
 		srcu_read_unlock(&hctx->queue_rq_srcu, srcu_idx);
 	}
 }
Sorry but I don't like these changes. Why have the blk_queue_quiesced()
calls be added at other code locations than the blk_mq_hctx_stopped() calls?
This will make the block layer unnecessary hard to maintain. Please consider
to change the blk_mq_hctx_stopped(hctx) calls in blk_mq_sched_dispatch_requests()
and *blk_mq_*run_hw_queue*() into blk_mq_hctx_stopped(hctx) || blk_queue_quiesced(q).
One benefit is that we make it explicit that the flag has to be checked
inside the RCU read-side critical sections. If you put it somewhere,
someone may put it out of read-side critical sections in future.
Hello Ming,

I really would like to see the blk_queue_quiesced() tests as close as possible to
the blk_mq_hctx_stopped() tests. But I agree that we need a way to document and/or
Could you explain why we have to do that? And checking on stopped state
doesn't need to hold RCU/SRCU read lock, and that two states are really
different.
verify that these tests occur with an RCU read-side lock held. Have you considered
to use rcu_read_lock_held() to document this?
Then we need to check if it is RCU or SRCU, and make code ugly as
current check on BLOCKING.

Thanks,
Ming
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help