Re: [PATCH v2 6/8] blk-mq: don't stop queue for quiescing

On Sun, May 28, 2017 at 04:03:20PM +0000, Bart Van Assche wrote:

On Sun, 2017-05-28 at 18:50 +0800, Ming Lei wrote:

quoted

On Sat, May 27, 2017 at 09:49:27PM +0000, Bart Van Assche wrote:

quoted

On Sat, 2017-05-27 at 22:21 +0800, Ming Lei wrote:

quoted

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 032045841856..84cce67caee3 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c

@@ -168,8 +168,6 @@ void blk_mq_quiesce_queue(struct request_queue *q)
 	unsigned int i;
 	bool rcu = false;
 
-	__blk_mq_stop_hw_queues(q, true);
-
 	spin_lock_irq(q->queue_lock);
 	queue_flag_set(QUEUE_FLAG_QUIESCED, q);
 	spin_unlock_irq(q->queue_lock);

@@ -198,7 +196,12 @@ void blk_mq_unquiesce_queue(struct request_queue *q)
 	queue_flag_clear(QUEUE_FLAG_QUIESCED, q);
 	spin_unlock_irq(q->queue_lock);
 
-	blk_mq_start_stopped_hw_queues(q, true);
+	/*
+	 * During quiescing, requests can be inserted
+	 * to scheduler's queue or sw queue, so we run
+	 * queues for dispatching these requests.
+	 */
+	blk_mq_start_hw_queues(q);
 }
 EXPORT_SYMBOL_GPL(blk_mq_unquiesce_queue);

This looks really weird to me. If blk_mq_quiesce_queue() doesn't stop any
hardware queues, why should blk_mq_unquiesce_queue() start any hardware
queues?

Please see the above comment, especially in direct issue path, we have to
insert the request into sw/scheduler queue when queue is quiesced,
and these queued requests have to be dispatched out during unquiescing.

I considered to sleep the current context under this situation, but
turns out it is more complicated to handle, given we have very limited
queue depth, just let applications consumes all, then wait.

Hello Ming,

The above seems to me to be an argument to *run* the queue from inside
blk_mq_unquiesce_queue() instead of *starting* stopped queues from inside
that function.

Yes, it is run queues, as you can see in my comment.

The reality is that we can't run queue without clearing the STOPPED state.


Thanks,
Ming