ata: libata-eh: avoid unnecessary calls to ata_scsi_port_error_handler()

PATCH Applied Damien Le Moal 3 participants 5 messages Feb 24 – Feb 25 libata

Share & tools

Damien Le Moal

author maintainer

2026-02-24 02:11 UTC #0 lore raw

PATCH 1/1 ata: libata-eh: avoid unnecessary calls to ata_scsi_port_error_handler()

When handling SCSI command timeouts, if we had no actual command timeouts (either because the command was a deferred qc or the completion path won the race with ata_scsi_cmd_error_handler()), we do not need to go through a port error handling, as there was in fact no errors at all.

Modify ata_scsi_cmd_error_handler() to return the number of commands that timed out and use this return value in ata_scsi_error() to call ata_scsi_port_error_handler() only if we had command timeouts, or if the port EH has already been scheduled due to failed commands. Otherwise, simply call scsi_eh_flush_done_q() to finish the completed commands without running the full port error handling.

Signed-off-by Damien Le Moal <[email protected]>

---
 drivers/ata/libata-eh.c | 28 +++++++++++++++++++---------
 include/linux/libata.h  |  3 ++-
 2 files changed, 21 insertions(+), 10 deletions(-)

drivers/ata/libata-eh.c +19 -9

index b373cceb95d2..c93423ce5258 100644
@@ -560,21 +560,27 @@ void ata_scsi_error(struct Scsi_Host *host)
 {
 	struct ata_port *ap = ata_shost_to_port(host);
 	unsigned long flags;
+	int nr_timedout;
 	LIST_HEAD(eh_work_q);
 
 	spin_lock_irqsave(host->host_lock, flags);
 	list_splice_init(&host->eh_cmd_q, &eh_work_q);
 	spin_unlock_irqrestore(host->host_lock, flags);
 
-	ata_scsi_cmd_error_handler(host, ap, &eh_work_q);
-
-	/* If we timed raced normal completion and there is nothing to
-	   recover nr_timedout == 0 why exactly are we doing error recovery ? */
-	ata_scsi_port_error_handler(host, ap);
+	/*
+	 * First check what errors we got with ata_scsi_cmd_error_handler().
+	 * If we had no command timeouts and EH is not scheduled for this port,
+	 * meaning that we do not have any failed command, then there is no
+	 * need to go through the full port error handling. We only need to
+	 * flush the completed commands we have.
+	 */
+	nr_timedout = ata_scsi_cmd_error_handler(host, ap, &eh_work_q);
+	if (nr_timedout || ata_port_eh_scheduled(ap))
+		ata_scsi_port_error_handler(host, ap);
+	else
+		scsi_eh_flush_done_q(&ap->eh_done_q);
 
-	/* finish or retry handled scmd's and clean up */
 	WARN_ON(!list_empty(&eh_work_q));
-
 }
 
 /**

@@ -586,9 +592,11 @@ void ata_scsi_error(struct Scsi_Host *host)
  * process the given list of commands and return those finished to the
  * ap->eh_done_q.  This function is the first part of the libata error
  * handler which processes a given list of failed commands.
+ *
+ * Return the number of commands that timed out.
  */
-void ata_scsi_cmd_error_handler(struct Scsi_Host *host, struct ata_port *ap,
-				struct list_head *eh_work_q)
+int ata_scsi_cmd_error_handler(struct Scsi_Host *host, struct ata_port *ap,
+			       struct list_head *eh_work_q)
 {
 	int i;
 	unsigned long flags;

@@ -694,6 +702,8 @@ void ata_scsi_cmd_error_handler(struct Scsi_Host *host, struct ata_port *ap,
 	ap->eh_tries = ATA_EH_MAX_TRIES;
 
 	spin_unlock_irqrestore(ap->lock, flags);
+
+	return nr_timedout;
 }
 EXPORT_SYMBOL(ata_scsi_cmd_error_handler);

include/linux/libata.h +2 -1

index db87c99e4189..c872d1f9f43b 100644
@@ -1225,7 +1225,8 @@ extern int ata_ncq_prio_enable(struct ata_port *ap, struct scsi_device *sdev,
 extern struct ata_device *ata_dev_pair(struct ata_device *adev);
 int ata_set_mode(struct ata_link *link, struct ata_device **r_failed_dev);
 extern void ata_scsi_port_error_handler(struct Scsi_Host *host, struct ata_port *ap);
-extern void ata_scsi_cmd_error_handler(struct Scsi_Host *host, struct ata_port *ap, struct list_head *eh_q);
+extern int ata_scsi_cmd_error_handler(struct Scsi_Host *host,
+				struct ata_port *ap, struct list_head *eh_q);
 
 /*
  * SATA specific code - drivers/ata/libata-sata.c

Signature · 1 line

2.53.0

Niklas Cassel

maintainer Reviewed-by on v1 1/1

2026-02-24 14:08 UTC #1 lore raw

Quoted 5 lines · Damien Le Moal

When handling SCSI command timeouts, if we had no actual command timeouts (either because the command was a deferred qc or the completion path won the race with ata_scsi_cmd_error_handler()), we do not need to go through a port error handling, as there was in fact no errors at all.

Quoted 8 lines

Modify ata_scsi_cmd_error_handler() to return the number of commands that timed out and use this return value in ata_scsi_error() to call ata_scsi_port_error_handler() only if we had command timeouts, or if the port EH has already been scheduled due to failed commands. Otherwise, simply call scsi_eh_flush_done_q() to finish the completed commands without running the full port error handling.
Signed-off-by Damien Le Moal <[email protected]>

Reviewed-by Niklas Cassel <[email protected]>

include/linux/libata.h extern int ata_ncq_prio_enable(struct ata_port *ap, struct scsi_device *sdev, · line 1229

Quoted 11 lines

include/linux/libata.h +2 -1

index db87c99e4189..c872d1f9f43b 100644
@@ -1225,7 +1225,8 @@ extern int ata_ncq_prio_enable(struct ata_port *ap, struct scsi_device *sdev,
 extern struct ata_device *ata_dev_pair(struct ata_device *adev);
 int ata_set_mode(struct ata_link *link, struct ata_device **r_failed_dev);
 extern void ata_scsi_port_error_handler(struct Scsi_Host *host, struct ata_port *ap);
-extern void ata_scsi_cmd_error_handler(struct Scsi_Host *host, struct ata_port *ap, struct list_head *eh_q);
+extern int ata_scsi_cmd_error_handler(struct Scsi_Host *host,
+				struct ata_port *ap, struct list_head *eh_q);

Nit: we could drop the extern while touching this line anyway.

Kind regards,
Niklas

Damien Le Moal

author maintainer

2026-02-24 20:59 UTC #2 lore raw

Quoted 18 lines · Niklas Cassel

Quoted 5 lines · Damien Le Moal
When handling SCSI command timeouts, if we had no actual command timeouts (either because the command was a deferred qc or the completion path won the race with ata_scsi_cmd_error_handler()), we do not need to go through a port error handling, as there was in fact no errors at all.
Quoted 8 lines
Modify ata_scsi_cmd_error_handler() to return the number of commands that timed out and use this return value in ata_scsi_error() to call ata_scsi_port_error_handler() only if we had command timeouts, or if the port EH has already been scheduled due to failed commands. Otherwise, simply call scsi_eh_flush_done_q() to finish the completed commands without running the full port error handling.
Signed-off-by Damien Le Moal <[email protected]>
Reviewed-by Niklas Cassel <[email protected]>

Quoted 13 lines

include/linux/libata.h extern int ata_ncq_prio_enable(struct ata_port *ap, struct scsi_device *sdev, · line 1229

Quoted 11 lines

include/linux/libata.h +2 -1

index db87c99e4189..c872d1f9f43b 100644
@@ -1225,7 +1225,8 @@ extern int ata_ncq_prio_enable(struct ata_port *ap, struct scsi_device *sdev,
 extern struct ata_device *ata_dev_pair(struct ata_device *adev);
 int ata_set_mode(struct ata_link *link, struct ata_device **r_failed_dev);
 extern void ata_scsi_port_error_handler(struct Scsi_Host *host, struct ata_port *ap);
-extern void ata_scsi_cmd_error_handler(struct Scsi_Host *host, struct ata_port *ap, struct list_head *eh_q);
+extern int ata_scsi_cmd_error_handler(struct Scsi_Host *host,
+				struct ata_port *ap, struct list_head *eh_q);

Nit: we could drop the extern while touching this line anyway.

Sure. Can you fix that up when applying ?

Signature · 2 lines

Damien Le Moal
Western Digital Research

Martin K. Petersen

Reviewed-by on v1 1/1

2026-02-24 21:22 UTC #3 lore raw

In reply to Damien Le Moal

Damien,

Quoted 6 lines

Modify ata_scsi_cmd_error_handler() to return the number of commands that timed out and use this return value in ata_scsi_error() to call ata_scsi_port_error_handler() only if we had command timeouts, or if the port EH has already been scheduled due to failed commands. Otherwise, simply call scsi_eh_flush_done_q() to finish the completed commands without running the full port error handling.

Looks OK.

Reviewed-by Martin K. Petersen <[email protected]>

Signature · 1 line

Martin K. Petersen

Niklas Cassel

maintainer Applied

2026-02-25 06:22 UTC #4 lore raw

In reply to Damien Le Moal

Quoted 5 lines · Damien Le Moal

When handling SCSI command timeouts, if we had no actual command timeouts (either because the command was a deferred qc or the completion path won the race with ata_scsi_cmd_error_handler()), we do not need to go through a port error handling, as there was in fact no errors at all.

Quoted 8 lines

Modify ata_scsi_cmd_error_handler() to return the number of commands that timed out and use this return value in ata_scsi_error() to call ata_scsi_port_error_handler() only if we had command timeouts, or if the port EH has already been scheduled due to failed commands. Otherwise, simply call scsi_eh_flush_done_q() to finish the completed commands without running the full port error handling.

Applied to libata/linux.git (for-7.1), thanks!

[1/1] ata: libata-eh: avoid unnecessary calls to ata_scsi_port_error_handler()

      https://git.kernel.org/libata/linux/c/46a9d970

Kind regards,
Niklas