On 11/16/23 6:13 AM, John Garry wrote:
> 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?
>
I should not have written "on a UA" above. We want to retry every UA
except that first entries in that array.
This first and second entry says if we see it then fail. However, if
it's not that specific failure value, then the last entry
SCMD_FAILURE_STAT_ANY kicks in and scsi_execute_cmd will retry.
It's so for this chunk:
- the_result = scsi_execute_cmd(sdkp->device, cmd,
- REQ_OP_DRV_IN, NULL, 0,
- SD_TIMEOUT,
- sdkp->max_retries,
- &exec_args);
- if (the_result > 0) {
- /*
- * If the drive has indicated to us that it
- * doesn't have any media in it, don't bother
- * with any more polling.
- */
- if (media_not_present(sdkp, &sshdr)) {
- if (media_was_present)
- sd_printk(KERN_NOTICE, sdkp,
- "Media removed, stopped polling\n");
- return;
- }
we will not retry if we get media_not_present but then retry here:
- } while (retries < 3 &&
- (!scsi_status_is_good(the_result) ||
- (scsi_status_is_check_condition(the_result) &&
- sense_valid && sshdr.sense_key == UNIT_ATTENTION)));
>> + 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,
>> + },
>> + {}
>
