On Tue, 2017-05-30 at 08:27 +0800, Ming Lei wrote:

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

quoted

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_queu=

e *q)

quoted

quoted

quoted

quoted

 	unsigned int i;
 	bool rcu =3D false;
=20
-	__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_q=

ueue *q)

quoted

quoted

quoted

quoted

 	queue_flag_clear(QUEUE_FLAG_QUIESCED, q);
 	spin_unlock_irq(q->queue_lock);
=20
-	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);

=20
This looks really weird to me. If blk_mq_quiesce_queue() doesn't st=

op any

quoted

quoted

quoted

hardware queues, why should blk_mq_unquiesce_queue() start any hard=

ware

quoted

quoted

quoted

queues?

=20
Please see the above comment, especially in direct issue path, we hav=

e to

quoted

quoted

insert the request into sw/scheduler queue when queue is quiesced,
and these queued requests have to be dispatched out during unquiescin=

g.

quoted

quoted

=20
I considered to sleep the current context under this situation, but
turns out it is more complicated to handle, given we have very limite=

d

quoted

quoted

queue depth, just let applications consumes all, then wait.

=20
Hello Ming,
=20
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 insi=

de

quoted

that function.

=20
Yes, it is run queues, as you can see in my comment.
=20
The reality is that we can't run queue without clearing the STOPPED state=

.

Hello Ming,

I think it's completely wrong to make blk_mq_unquiesce_queue() to clear the
STOPPED state. Stopping a queue is typically used to tell the block layer t=
o
stop dispatching requests and not to resume dispatching requests until the
STOPPED flag is cleared. If stopping and quiescing a queue happen
simultaneously then dispatching should not occur before both
blk_mq_unquiesce_queue() and blk_mq_start_hw_queue() have been called. Your
patch would make dispatching start too early.

Bart.=