Re: [PATCH v12 06/20] scsi: Have scsi-ml retry sd_spinup_disk errors

John Garry
2023-11-16T12:13:45Z

On 14/11/2023 01:37, Mike Christie wrote:

Hi Mike,

> This simplifies sd_spinup_disk so scsi-ml retries errors for it. Note that
> we retried specifically on a UA and also if scsi_status_is_good returned
> failed which would happen for all check conditions. In this patch we use
> SCMD_FAILURE_STAT_ANY which will trigger for the same conditions as
> when scsi_status_is_good returns false and there is status. This will
> cover all CCs including UAs so there is no explicit failures arrary entry

/s/arrary/array/

> for UAs.

But the first failure_defs member is for UNIT_ATTENTION, below, right?

Thanks,
John

>
> There is one behavior change where we no longer retry when
> scsi_execute_cmd returns < 0, but we should be ok. We don't need to retry
> for failures like the queue being removed, and for the case where there
> are no tags/reqs the block layer waits/retries for us. For possible memory
> allocation failures from blk_rq_map_kern we use GFP_NOIO, so retrying
> will probably not help.
>
> We do not handle the outside loop's retries because we want to sleep
> between tries and we don't support that yet.
>
> Signed-off-by: Mike Christie<michael.christie@oracle.com>
> ---
> drivers/scsi/sd.c | 77 +++++++++++++++++++++++++++--------------------
> 1 file changed, 45 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 641f9c9c0674..cda0d029ab7f 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -2235,55 +2235,68 @@ static int sd_done(struct scsi_cmnd *SCpnt)
> static void
> sd_spinup_disk(struct scsi_disk *sdkp)
> {
> - unsigned char cmd[10];
> + static const u8 cmd[10] = { TEST_UNIT_READY };
> unsigned long spintime_expire = 0;
> - int retries, spintime;
> + int spintime, sense_valid = 0;
> unsigned int the_result;
> struct scsi_sense_hdr sshdr;
> + struct scsi_failure failure_defs[] = {
> + /* Do not retry Medium Not Present */
> + {
> + .sense = UNIT_ATTENTION,
> + .asc = 0x3A,
> + .ascq = SCMD_FAILURE_ASCQ_ANY,
> + .result = SAM_STAT_CHECK_CONDITION,
> + },
> + {
> + .sense = NOT_READY,
> + .asc = 0x3A,
> + .ascq = SCMD_FAILURE_ASCQ_ANY,
> + .result = SAM_STAT_CHECK_CONDITION,
> + },
> + /* Retry when scsi_status_is_good would return false 3 times */
> + {
> + .result = SCMD_FAILURE_STAT_ANY,
> + .allowed = 3,
> + },
> + {}