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

[Alan Stern]

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 */