Re: [PATCH v3 4/8] scsi: call scsi_stop_queue() without state_mutex held

Bart Van Assche
2023-06-07T19:16:10Z

On 6/7/23 11:22, mwilck@suse.com wrote:
> From: Martin Wilck <mwilck@suse.com>
>
> sdev->state_mutex protects only sdev->sdev_state. There's no reason
> to keep it held while calling scsi_stop_queue().
>
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
> drivers/scsi/scsi_lib.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index ce5788643011..26e7ce25fa05 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -2795,9 +2795,9 @@ static void scsi_device_block(struct scsi_device *sdev, void *data)
>
> mutex_lock(&sdev->state_mutex);
> err = __scsi_internal_device_block_nowait(sdev);
> + mutex_unlock(&sdev->state_mutex);
> if (err == 0)
> scsi_stop_queue(sdev, false);
> - mutex_unlock(&sdev->state_mutex);
>
> WARN_ONCE(err, "__scsi_internal_device_block_nowait(%s) failed: err = %d\n",
> dev_name(&sdev->sdev_gendev), err);

There is a reason why scsi_stop_queue() is called with the sdev state
mutex held: if this mutex is not held, unblocking of a SCSI device can
start before the scsi_stop_queue() call has finished. It is not allowed
to swap the order of the blk_mq_quiesce_queue() and
blk_mq_unquiesce_queue() calls.

Bart.