Re: [PATCH v3 11/18] ata: libata: change ata_eh_request_sense() to not set CHECK_CONDITION

Hannes Reinecke
2023-01-27T15:36:51Z

On 1/24/23 20:02, Niklas Cassel wrote:
> Currently, ata_eh_request_sense() unconditionally sets the scsicmd->result
> to SAM_STAT_CHECK_CONDITION.
>
> For Command Duration Limits policy 0xD:
> The device shall complete the command without error (SAM_STAT_GOOD)
> with the additional sense code set to DATA CURRENTLY UNAVAILABLE.
>
> It is perfectly fine to have sense data for a command that returned
> completion without error.
>
> In order to support for CDL policy 0xD, we have to remove this
> assumption that having sense data means that the command failed
> (SAM_STAT_CHECK_CONDITION).
>
> Change ata_eh_request_sense() to not set SAM_STAT_CHECK_CONDITION,
> and instead move the setting of SAM_STAT_CHECK_CONDITION to the single
> caller that wants SAM_STAT_CHECK_CONDITION set, that way
> ata_eh_request_sense() can be reused in a follow-up patch that adds
> support for CDL policy 0xD.
>
> The only caller of ata_eh_request_sense() is protected by:
> if (!(qc->flags & ATA_QCFLAG_SENSE_VALID)), so we can remove this
> duplicated check from ata_eh_request_sense() itself.
>
> Additionally, ata_eh_request_sense() is only called from
> ata_eh_analyze_tf(), which is only called when iteratating the QCs using
> ata_qc_for_each_raw(), which does not include the internal tag,
> so cmd can never be NULL (all non-internal commands have qc->scsicmd set),
> so remove the !cmd check as well.
>
> Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> ---
> drivers/ata/libata-eh.c | 25 ++++++++++++++++---------
> 1 file changed, 16 insertions(+), 9 deletions(-)
>
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes