On 9/14/23 15:01, Hannes Reinecke wrote:
> On 9/13/23 17:04, Niklas Cassel wrote:
>> From: Niklas Cassel <niklas.cassel@xxxxxxx>
>>
>> Currently, we fetch sense data for a _successful_ command if either:
>> 1) Command was NCQ and ATA_DFLAG_CDL_ENABLED flag set (flag
>> ATA_DFLAG_CDL_ENABLED will only be set if the Successful NCQ command
>> sense data supported bit is set); or
>> 2) Command was non-NCQ and regular sense data reporting is enabled.
>>
>> This means that case 2) will trigger for a non-NCQ command which has
>> ATA_SENSE bit set, regardless if CDL is enabled or not.
>>
>> This decision was by design. If the device reports that it has sense data
>> available, it makes sense to fetch that sense data, since the sk/asc/ascq
>> could be important information regardless if CDL is enabled or not.
>>
>> However, the fetching of sense data for a successful command is done via
>> ATA EH. Considering how intricate the ATA EH is, we really do not want to
>> invoke ATA EH unless absolutely needed.
>>
>> Before commit 18bd7718b5c4 ("scsi: ata: libata: Handle completion of CDL
>> commands using policy 0xD") we never fetched sense data for successful
>> non-NCQ commands.
>>
>> In order to not invoke the ATA EH unless absolutely necessary, even if the
>> device claims support for sense data reporting, only fetch sense data for
>> successful (NCQ and non-NCQ commands) if CDL is supported and enabled.
>>
> Errm. We need sense data for zoned devices, do we not?
Any SATA device that supports sense reporting feature, including ZAC devices,
can still get sense data for failed commands.
This case is for the weird CDL aborts with policy 0xD which will fail a command
that exceeds its limits with a success status and sense data available bit set
in the SDB (which translate to ATA_SENSE flag). So for commands using CDL,
getting a success with ATA_SENSE set most likely means that the command was
aborted by the drive.
So this is not the regular error case. We still use EH to get that sense data
from the "sense data for successful NCQ command log" though, because otherwise
we can run into problems if we do not have a free tag.
>
> Cheers,
>
> Hannes
--
Damien Le Moal
Western Digital Research
