Hello James,

On Mon, Nov 08, 2021 at 11:42:01AM -0500, James Bottomley wrote:

On Wed, 2021-11-03 at 11:43 +0800, Ming Lei wrote:
[...]

quoted

+void scsi_start_queue(struct scsi_device *sdev)
+{
+	if (cmpxchg(&sdev->queue_stopped, 1, 0))
+		blk_mq_unquiesce_queue(sdev->request_queue);
+}
+
+static void scsi_stop_queue(struct scsi_device *sdev, bool nowait)
+{
+	if (!cmpxchg(&sdev->queue_stopped, 0, 1)) {
+		if (nowait)
+			blk_mq_quiesce_queue_nowait(sdev-

quoted

request_queue);

+		else
+			blk_mq_quiesce_queue(sdev->request_queue);
+	} else {
+		if (!nowait)
+			blk_mq_wait_quiesce_done(sdev->request_queue);
+	}
+}

This looks counter intuitive.  I assume it's done so that if we call
scsi_stop_queue when the queue has already been stopped, it waits until

The motivation is to balance blk_mq_quiesce_queue_nowait()/blk_mq_quiesce_queue()
and blk_mq_unquiesce_queue().

That needs one extra mutex to cover the quiesce action and update
the flag, but we can't hold the mutex in scsi_internal_device_block_nowait(),
so take this way with the atomic flag.

the queue is actually quiesced before returning so the behaviour is the
same in the !nowait case?  Some sort of comment explaining that would
be useful.

I will add comment on the current usage.


Thanks,
Ming