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

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

[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.

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 <dlemoal@kernel.org>
---
 drivers/ata/libata-eh.c | 28 +++++++++++++++++++---------
 include/linux/libata.h  |  3 ++-
 2 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index b373cceb95d2..c93423ce5258 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -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);
 
diff --git a/include/linux/libata.h b/include/linux/libata.h
index db87c99e4189..c872d1f9f43b 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -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
-- 
2.53.0

Re: [PATCH] ata: libata-eh: avoid unnecessary calls to ata_scsi_port_error_handler()

[Niklas Cassel]

On Tue, Feb 24, 2026 at 11:06:38AM +0900, Damien Le Moal wrote:
> 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 <dlemoal@kernel.org>

Reviewed-by: Niklas Cassel <cassel@kernel.org>


> diff --git a/include/linux/libata.h b/include/linux/libata.h
> index db87c99e4189..c872d1f9f43b 100644
> --- a/include/linux/libata.h
> +++ b/include/linux/libata.h
> @@ -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

Re: [PATCH] ata: libata-eh: avoid unnecessary calls to ata_scsi_port_error_handler()

[Damien Le Moal]

On 2/24/26 23:08, Niklas Cassel wrote:
> On Tue, Feb 24, 2026 at 11:06:38AM +0900, Damien Le Moal wrote:
>> 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 <dlemoal@kernel.org>
> 
> Reviewed-by: Niklas Cassel <cassel@kernel.org>
> 
> 
>> diff --git a/include/linux/libata.h b/include/linux/libata.h
>> index db87c99e4189..c872d1f9f43b 100644
>> --- a/include/linux/libata.h
>> +++ b/include/linux/libata.h
>> @@ -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 ?


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH] ata: libata-eh: avoid unnecessary calls to ata_scsi_port_error_handler()

[Martin K. Petersen]

Damien,

> 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 <martin.petersen@oracle.com>

-- 
Martin K. Petersen

Re: [PATCH] ata: libata-eh: avoid unnecessary calls to ata_scsi_port_error_handler()

[Niklas Cassel]

On Tue, Feb 24, 2026 at 11:06:38AM +0900, Damien Le Moal wrote:
> 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.
> 
> 

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