Re: [PATCH v4 4/6] scsi: don't wait for quiesce in scsi_stop_queue()

[Bart Van Assche <bvanassche@acm.org>]

On 6/12/23 08:03, mwilck@suse.com wrote:
> -	 * However, we still need to wait until quiesce is done
> -	 * in case that queue has been stopped.
> +	 * After return, we still need to wait until quiesce is done.

The above comment would be more clear if "After return, we still need" 
would be changed into "The caller needs".

> @@ -2800,9 +2792,17 @@ static void scsi_device_block(struct scsi_device *sdev, void *data)
>   
>   	mutex_lock(&sdev->state_mutex);
>   	err = __scsi_internal_device_block_nowait(sdev);
> -	if (err == 0)
> -		scsi_stop_queue(sdev, false);
> -	mutex_unlock(&sdev->state_mutex);
> +	if (err == 0) {
> +		/*
> +		 * scsi_stop_queue() must be called with the state_mutex
> +		 * held. Otherwise a simultaneous scsi_start_queue() call
> +		 * might unquiesce the queue before we quiesce it.
> +		 */
> +		scsi_stop_queue(sdev);
> +		mutex_unlock(&sdev->state_mutex);
> +		blk_mq_wait_quiesce_done(sdev->request_queue->tag_set);
> +	} else
> +		mutex_unlock(&sdev->state_mutex);

Has it been considered to modify the above code such that there is a 
single mutex_unlock() call instead of two? I wouldn't mind if 
blk_mq_wait_quiesce_done() would be called if err != 0 since performance 
is not that important if this function fails.

Thanks,

Bart.