Re: [PATCH] scsi: sd: add runtime pm to open / release

Alan Stern
2020-08-07T14:30:07Z

On Fri, Aug 07, 2020 at 11:51:21AM +0200, Martin Kepplinger wrote:
> it's really strange: below is the change I'm trying. Of course that's
> only for testing the functionality, nothing how a patch could look like.
>
> While I remember it had worked, now (weirdly since I tried that mounting
> via fstab) it doesn't anymore!
>
> What I understand (not much): I handle the error with "retry" via the
> new flag, but scsi_decide_disposition() returns SUCCESS because of "no
> more retries"; but it's the first and only time it's called.

Are you saying that scmd->allowed is set to 0? Or is scsi_notry_cmd()
returning a nonzero value? Whichever is true, why does it happen that
way?

What is the failing command? Is it a READ(10)?

> How can this be? What am I missing?

It's kind of hard to tell without seeing the error messages or system
log or any debugging information.

Alan Stern

> --- a/drivers/scsi/scsi_error.c
> +++ b/drivers/scsi/scsi_error.c
> @@ -565,6 +565,13 @@ int scsi_check_sense(struct scsi_cmnd *scmd)
> return NEEDS_RETRY;
> }
> }
> + if (scmd->device->expecting_media_change) {
> + if (sshdr.asc == 0x28 && sshdr.ascq == 0x00) {
> + scmd->device->expecting_media_change = 0;
> + return NEEDS_RETRY;
> + }
> + }
> +
> /*
> * we might also expect a cc/ua if another LUN on the target
> * reported a UA with an ASC/ASCQ of 3F 0E -
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index d90fefffe31b..bb583e403b81 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -3642,6 +3642,8 @@ static int sd_resume(struct device *dev)
> if (!sdkp) /* E.g.: runtime resume at the start of sd_probe() */
> return 0;
>
> + sdkp->device->expecting_media_change = 1;
> +
> if (!sdkp->device->manage_start_stop)
> return 0;
>
> diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
> index bc5909033d13..f5fc1af68e00 100644
> --- a/include/scsi/scsi_device.h
> +++ b/include/scsi/scsi_device.h
> @@ -169,6 +169,7 @@ struct scsi_device {
> * this device */
> unsigned expecting_cc_ua:1; /* Expecting a CHECK_CONDITION/UNIT_ATTN
> * because we did a bus reset. */
> + unsigned expecting_media_change:1;
> unsigned use_10_for_rw:1; /* first try 10-byte read / write */
> unsigned use_10_for_ms:1; /* first try 10-byte mode sense/select */
> unsigned set_dbd_for_ms:1; /* Set "DBD" field in mode sense */