Re: [PATCH v3 17/18] ata: libata: handle completion of CDL commands using policy 0xD

Hannes Reinecke
2023-01-27T15:43:25Z

On 1/24/23 20:03, Niklas Cassel wrote:
> A CDL timeout for policy 0xF is defined as a NCQ error, just with a CDL
> specific sk/asc/ascq in the sense data. Therefore, the existing code in
> libata does not need to be modified to handle a policy 0xF CDL timeout.
>
> For Command Duration Limits policy 0xD:
> The device shall complete the command without error with the additional
> sense code set to DATA CURRENTLY UNAVAILABLE.
>
> Since a CDL timeout for policy 0xD is not an error, we cannot use the
> NCQ Command Error log (10h).
>
> Instead, we need to read the Sense Data for Successful NCQ Commands
> log (0Fh).
>
> In the success case, just like in the error case, we cannot simply read
> a log page from the interrupt handler itself, since reading a log page
> involves sending a READ LOG DMA EXT or READ LOG EXT command.
>
> Therefore, we add a new EH action ATA_EH_GET_SUCCESS_SENSE.
> When a command completes without error, and when the ATA_SENSE bit
> is set, this new action is set as pending, and EH is scheduled.
>
> This way, similar to the NCQ error case, the log page will be read
> from EH context.
>
> An alternative would have been to add a new kthread or workqueue to
> handle this. However, extending EH can be done with minimal changes
> and avoids the need to synchronize a new kthread/workqueue with EH.
>
> Co-developed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
> Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
> Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> ---
> drivers/ata/libata-core.c | 88 +++++++++++++++++++++++++++++++-
> drivers/ata/libata-eh.c | 105 +++++++++++++++++++++++++++++++++++++-
> drivers/ata/libata-sata.c | 92 +++++++++++++++++++++++++++++++++
> include/linux/ata.h | 3 ++
> include/linux/libata.h | 11 +++-
> 5 files changed, 295 insertions(+), 4 deletions(-)
>
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes