[PATCH] ata: libata-eh: Retry non-NCQ commands when their sense data asks for it

[PATCH] ata: libata-eh: Retry non-NCQ commands when their sense data asks for it

[Niklas Cassel]

When a failed command has valid sense data, ata_eh_analyze_tf() asks
scsi_check_sense() whether the command should be retried, and sets
ATA_QCFLAG_RETRY if it should. ata_eh_link_autopsy() then clears
AC_ERR_DEV and AC_ERR_OTHER from qc->err_mask, relying on that decision.

For an NCQ command, qc->err_mask still has AC_ERR_NCQ, so
ata_eh_finish() honors ATA_QCFLAG_RETRY. For a non-NCQ command, whose
sense data comes from REQUEST SENSE DATA EXT, qc->err_mask is now zero,
and ata_eh_finish() completes the command with scmd->retries set to
scmd->allowed, without looking at ATA_QCFLAG_RETRY. Such a command is
thus never retried, even when its sense data asks for a retry, e.g. a
transient error reported as ABORTED COMMAND.

Make ata_eh_finish() honor ATA_QCFLAG_RETRY for commands with valid
sense data. ata_eh_qc_retry() cannot be used for this: as qc->err_mask
is zero, it increments scmd->allowed, so every retry would allow one
more, and a persistent error would be retried forever. Instead,
complete the command without touching scmd->allowed or scmd->retries,
so that the SCSI midlayer retries it at most scmd->allowed times, like
any other failed command.

Fixes: 3852e3738266 ("libata: evaluate SCSI sense code")
Signed-off-by: Niklas Cassel <cassel@kernel.org>

---
Tested in QEMU: blkdebug fails a non-NCQ (PATA) read with EIO, and the
disk reports ABORTED COMMAND sense data. Before this patch, the read was
not retried (1 attempt); after it, a transient error is retried and the
read succeeds, and a persistent error is retried 5 times. NCQ commands,
and errors without sense data, were already retried and are unchanged.
---
 drivers/ata/libata-eh.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index b7e8545e569e..f99e422dfd9f 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -4080,8 +4080,20 @@ void ata_eh_finish(struct ata_port *ap)
 				ata_eh_qc_complete(qc);
 			}
 		} else {
-			if (qc->flags & ATA_QCFLAG_SENSE_VALID ||
-			    qc->flags & ATA_QCFLAG_EH_SUCCESS_CMD) {
+			if (qc->flags & ATA_QCFLAG_SENSE_VALID &&
+			    qc->flags & ATA_QCFLAG_RETRY) {
+				/*
+				 * The sense data asks for a retry, see
+				 * ata_eh_analyze_tf(). ata_eh_link_autopsy()
+				 * cleared qc->err_mask, so ata_eh_qc_retry()
+				 * would increment scmd->allowed. Leave
+				 * scmd->allowed alone, so that upper layer
+				 * only retries the command if it has not
+				 * already been retried too many times.
+				 */
+				__ata_eh_qc_complete(qc);
+			} else if (qc->flags & ATA_QCFLAG_SENSE_VALID ||
+				   qc->flags & ATA_QCFLAG_EH_SUCCESS_CMD) {
 				ata_eh_qc_complete(qc);
 			} else {
 				/* feed zero TF to sense generation */
-- 
2.56.0

Re: [PATCH] ata: libata-eh: Retry non-NCQ commands when their sense data asks for it

[Igor Pylypiv]

On Fri, Oct 30, 2026 at 10:00:48AM +0100, Niklas Cassel wrote:
> When a failed command has valid sense data, ata_eh_analyze_tf() asks
> scsi_check_sense() whether the command should be retried, and sets
> ATA_QCFLAG_RETRY if it should. ata_eh_link_autopsy() then clears
> AC_ERR_DEV and AC_ERR_OTHER from qc->err_mask, relying on that decision.
> 
> For an NCQ command, qc->err_mask still has AC_ERR_NCQ, so
> ata_eh_finish() honors ATA_QCFLAG_RETRY. For a non-NCQ command, whose
> sense data comes from REQUEST SENSE DATA EXT, qc->err_mask is now zero,
> and ata_eh_finish() completes the command with scmd->retries set to
> scmd->allowed, without looking at ATA_QCFLAG_RETRY. Such a command is
> thus never retried, even when its sense data asks for a retry, e.g. a
> transient error reported as ABORTED COMMAND.
> 
> Make ata_eh_finish() honor ATA_QCFLAG_RETRY for commands with valid
> sense data. ata_eh_qc_retry() cannot be used for this: as qc->err_mask
> is zero, it increments scmd->allowed, so every retry would allow one
> more, and a persistent error would be retried forever. Instead,
> complete the command without touching scmd->allowed or scmd->retries,
> so that the SCSI midlayer retries it at most scmd->allowed times, like
> any other failed command.
> 
> Fixes: 3852e3738266 ("libata: evaluate SCSI sense code")
> Signed-off-by: Niklas Cassel <cassel@kernel.org>

Reviewed-by: Igor Pylypiv <ipylypiv@google.com>

Thanks,
Igor