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