On Wed, 2018-03-14 at 15:45 -0700, Bart Van Assche wrote:
> Several SCSI transport and LLD drivers surround code that does not
> tolerate concurrent calls of .queuecommand() with scsi_target_block()
> /
> scsi_target_unblock(). These last two functions use
> blk_mq_quiesce_queue() / blk_mq_unquiesce_queue() for scsi-mq request
> queues to prevent concurrent .queuecommand() calls. However, that is
> not sufficient to prevent .queuecommand() calls from
> scsi_send_eh_cmnd().
> Hence surround the .queuecommand() call from the SCSI error handler
> with
> code that avoids that .queuecommand() gets called in the quiesced
> state.
>
> Notes:
> - Converting the .queuecommand() call in scsi_send_eh_cmnd() into
> code that calls blk_get_request() + blk_execute_rq() is not an
> option since scsi_send_eh_cmnd() must be able to make forward
> progress
> even if all requests are allocated.
> - Converting the .queuecommand() call in scsi_send_eh_cmnd() into a
> blk_execute_rq() or blk_mq_requeue_request() call is not an option
> either
> because that would require to change every individual function in
> the I/O
> path. Each function in the I/O path would have to be modified such
> that it
> handles requests received from the block layer core and request
> received
> from the SCSI EH differently. Since struct scsi_cmnd is not
> initialized by
> the block layer for filesystem requests, it is not possible to
> determine
> in scsi_queue_rq() whether or not a request has been submitted by
> the
> SCSI EH without modifying the block layer.
>
> Signed-off-by: Bart Van Assche <bart.vanassche@xxxxxxx>
> Cc: Hannes Reinecke <hare@xxxxxxx>
> Cc: Johannes Thumshirn <jthumshirn@xxxxxxx>
> ---
> drivers/scsi/scsi_error.c | 13 +++++++++++++
> drivers/scsi/scsi_lib.c | 2 ++
> drivers/scsi/scsi_scan.c | 1 +
> include/scsi/scsi_device.h | 1 +
> 4 files changed, 17 insertions(+)
>
> diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
> index 946039117bf4..cfc805851a2a 100644
> --- a/drivers/scsi/scsi_error.c
> +++ b/drivers/scsi/scsi_error.c
> @@ -1042,6 +1042,7 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd
> *scmd, unsigned char *cmnd,
> unsigned long timeleft = timeout;
> struct scsi_eh_save ses;
> const unsigned long stall_for = msecs_to_jiffies(100);
> + DEFINE_WAIT(wait);
> int rtn;
>
> retry:
> @@ -1050,7 +1051,19 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd
> *scmd, unsigned char *cmnd,
>
> scsi_log_send(scmd);
> scmd->scsi_done = scsi_eh_done;
> + mutex_lock(&sdev->state_mutex);
> + while (sdev->sdev_state == SDEV_QUIESCE) {
> + prepare_to_wait(&sdev->state_wq, &wait,
> TASK_INTERRUPTIBLE);
> + mutex_unlock(&sdev->state_mutex);
> + SCSI_LOG_ERROR_RECOVERY(5, sdev_printk(KERN_DEBUG,
> sdev,
> + "%s: state %d <> %d\n", __func__, sdev-
> >sdev_state,
> + SDEV_QUIESCE));
> + schedule();
> + mutex_lock(&sdev->state_mutex);
> + }
> + finish_wait(&sdev->state_wq, &wait);
> rtn = shost->hostt->queuecommand(shost, scmd);
> + mutex_unlock(&sdev->state_mutex);
> if (rtn) {
> if (timeleft > stall_for) {
This has got to be minutely rare: why not just use the existing
stall_for timeout infrastructure instead of adding a waitqueue to every
device?
James
