On Thu, 2017-06-01 at 10:21 +0800, Ming Lei wrote:

On Wed, May 31, 2017 at 03:37:30PM +0000, Bart Van Assche wrote:

quoted

On Wed, 2017-05-31 at 20:37 +0800, Ming Lei wrote:

quoted

=20
+	/* wait until queue is unquiesced */
+	wait_event_cmd(q->quiesce_wq, !blk_queue_quiesced(q),
+			may_sleep ?
+			srcu_read_unlock(&hctx->queue_rq_srcu, *srcu_idx) :
+			rcu_read_unlock(),
+			may_sleep ?
+			*srcu_idx =3D srcu_read_lock(&hctx->queue_rq_srcu) :
+			rcu_read_lock());
+
 	if (q->elevator)
 		goto insert;

=20
What I see is that in this patch a new waitqueue has been introduced
(quiesce_wq) and also that an explanation of why you think this new wai=

tqueue

quoted

is needed is missing completely. Why is it that you think that the
synchronize_scru() and synchronize_rcu() calls in blk_mq_quiesce_queue(=

) are

quoted

not sufficient? If this new waitqueue is not needed, please remove that
waitqueue again.

=20
OK, the reason is simple, and it is only related with direct issue.
=20
Under this situation, when the queue is quiesced, we have to
=20
	- insert the current request into sw queue(scheduler queue)
	OR
	-wait until queue becomes unquiesced like what this patch is doing
=20
The disadvantage of the 1st way is that we have to consider to run queue
again in blk_mq_unquiesce_queue() for the queued requests during quiescin=

g.

=20
For the 2nd way(what this patch is doing), one benefit is that applicatio=

n

can avoid to submit I/O to a quiesced queue. Another benefit is that we
needn't to consider to run queue in blk_mq_unquiesce_queue(). But with co=

st

of one waitqueue, the cost should be cheap, but if you persist on the 1st
approach, I am fine to change to that.

Hello Ming,

Since the runtime overhead of the alternative approach (insert into queue) =
is
significantly smaller and since it will result in a simpler implementation =
I
prefer the alternative approach.

Thanks,

Bart.=