All of lore.kernel.org
 help / color / mirror / Atom feed
* SCSI EH wakeup deadlock from deferred_qc
@ 2026-07-08 19:36 Igor Pylypiv
  2026-07-08 22:54 ` Damien Le Moal
  2026-07-08 23:44 ` Damien Le Moal
  0 siblings, 2 replies; 7+ messages in thread
From: Igor Pylypiv @ 2026-07-08 19:36 UTC (permalink / raw)
  To: Damien Le Moal, Niklas Cassel; +Cc: linux-ide

Hi Damien,

I've stumbled upon an issue where SCSI EH didn't run upon command timeout
but ran ~10 or so seconds later. It seems like a recent regression from
the introduction of deffered_qc.

When an active command times out while a non-NCQ command is waiting in
deferred_qc, SCSI EH fails to wake up. Recovery stalls until the deferred
command's own timer expires.

When an NCQ command times out, scsi_timeout() calls scsi_eh_scmd_add(),
incrementing shost->host_failed (1). However, when scsi_eh_wakeup() checks
whether to wake the EH thread, scsi_host_busy(shost) counts 2 active
commands (1 timed-out + 1 in deferred_qc).

Because busy (2) != host_failed (1), scsi_eh_wakeup() refuses to wake
the EH thread, deadlocking error recovery until the deferred command
times out on its own.

Thanks,
Igor

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: SCSI EH wakeup deadlock from deferred_qc
  2026-07-08 19:36 SCSI EH wakeup deadlock from deferred_qc Igor Pylypiv
@ 2026-07-08 22:54 ` Damien Le Moal
  2026-07-08 23:44 ` Damien Le Moal
  1 sibling, 0 replies; 7+ messages in thread
From: Damien Le Moal @ 2026-07-08 22:54 UTC (permalink / raw)
  To: Igor Pylypiv, Niklas Cassel; +Cc: linux-ide

On 7/9/26 04:36, Igor Pylypiv wrote:
> Hi Damien,
> 
> I've stumbled upon an issue where SCSI EH didn't run upon command timeout
> but ran ~10 or so seconds later. It seems like a recent regression from
> the introduction of deffered_qc.
> 
> When an active command times out while a non-NCQ command is waiting in
> deferred_qc, SCSI EH fails to wake up. Recovery stalls until the deferred
> command's own timer expires.
> 
> When an NCQ command times out, scsi_timeout() calls scsi_eh_scmd_add(),
> incrementing shost->host_failed (1). However, when scsi_eh_wakeup() checks
> whether to wake the EH thread, scsi_host_busy(shost) counts 2 active
> commands (1 timed-out + 1 in deferred_qc).
> 
> Because busy (2) != host_failed (1), scsi_eh_wakeup() refuses to wake
> the EH thread, deadlocking error recovery until the deferred command
> times out on its own.

Igor,

Thank you for reporting this. Let me have a look.


-- 
Damien Le Moal
Western Digital Research

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: SCSI EH wakeup deadlock from deferred_qc
  2026-07-08 19:36 SCSI EH wakeup deadlock from deferred_qc Igor Pylypiv
  2026-07-08 22:54 ` Damien Le Moal
@ 2026-07-08 23:44 ` Damien Le Moal
  2026-07-09  1:01   ` Damien Le Moal
  1 sibling, 1 reply; 7+ messages in thread
From: Damien Le Moal @ 2026-07-08 23:44 UTC (permalink / raw)
  To: Igor Pylypiv, Niklas Cassel; +Cc: linux-ide

[-- Attachment #1: Type: text/plain, Size: 1022 bytes --]

On 7/9/26 04:36, Igor Pylypiv wrote:
> Hi Damien,
> 
> I've stumbled upon an issue where SCSI EH didn't run upon command timeout
> but ran ~10 or so seconds later. It seems like a recent regression from
> the introduction of deffered_qc.
> 
> When an active command times out while a non-NCQ command is waiting in
> deferred_qc, SCSI EH fails to wake up. Recovery stalls until the deferred
> command's own timer expires.
> 
> When an NCQ command times out, scsi_timeout() calls scsi_eh_scmd_add(),
> incrementing shost->host_failed (1). However, when scsi_eh_wakeup() checks
> whether to wake the EH thread, scsi_host_busy(shost) counts 2 active
> commands (1 timed-out + 1 in deferred_qc).
> 
> Because busy (2) != host_failed (1), scsi_eh_wakeup() refuses to wake
> the EH thread, deadlocking error recovery until the deferred command
> times out on its own.

Igor,

Can you try the attached diff ?
I will test on my end, but I need to hack something to trigger a timeout :)


-- 
Damien Le Moal
Western Digital Research

[-- Attachment #2: ata-timeout.diff --]
[-- Type: text/x-patch, Size: 1978 bytes --]

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 5868526301a2..13a8d2312f41 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1730,6 +1730,28 @@ static void ata_scsi_schedule_deferred_qc(struct ata_link *link)
 		queue_work(system_highpri_wq, &link->deferred_qc_work);
 }
 
+enum scsi_timeout_action ata_scsi_eh_timed_out(struct scsi_cmnd *scmd)
+{
+	struct ata_port *ap = ata_shost_to_port(scmd->device->host);
+	unsigned long flags;
+
+	/*
+	 * We had a timeout, either for an NCQ command or for one deferred
+	 * queued command. Either way, we must release all deferred queued
+	 * command so that scsi EH can trigger.
+	 */
+	spin_lock_irqsave(ap->lock, flags);
+	ata_scsi_requeue_deferred_qc(ap);
+	spin_unlock_irqrestore(ap->lock, flags);
+
+	/*
+	 * Let scsi_timeout() know that it must continue with handling the
+	 * timeout as we in fact did not do much here.
+	 */
+	return SCSI_EH_NOT_HANDLED;
+}
+EXPORT_SYMBOL_GPL(ata_scsi_eh_timed_out);
+
 static void ata_scsi_qc_complete(struct ata_queued_cmd *qc)
 {
 	struct ata_link *link = qc->dev->link;
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 96e626d6a7ca..327da43d7496 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -1153,6 +1153,7 @@ extern int ata_scsi_ioctl(struct scsi_device *dev, unsigned int cmd,
 #endif
 extern enum scsi_qc_status ata_scsi_queuecmd(struct Scsi_Host *h,
 					     struct scsi_cmnd *cmd);
+enum scsi_timeout_action ata_scsi_eh_timed_out(struct scsi_cmnd *cmd);
 #if IS_REACHABLE(CONFIG_ATA)
 bool ata_scsi_dma_need_drain(struct request *rq);
 #else
@@ -1464,6 +1465,7 @@ extern const struct attribute_group *ata_common_sdev_groups[];
 	.ioctl			= ata_scsi_ioctl,		\
 	ATA_SCSI_COMPAT_IOCTL					\
 	.queuecommand		= ata_scsi_queuecmd,		\
+	.eh_timed_out		= ata_scsi_eh_timed_out,	\
 	.dma_need_drain		= ata_scsi_dma_need_drain,	\
 	.this_id		= ATA_SHT_THIS_ID,		\
 	.emulated		= ATA_SHT_EMULATED,		\

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: SCSI EH wakeup deadlock from deferred_qc
  2026-07-08 23:44 ` Damien Le Moal
@ 2026-07-09  1:01   ` Damien Le Moal
  2026-07-09  2:24     ` Igor Pylypiv
  0 siblings, 1 reply; 7+ messages in thread
From: Damien Le Moal @ 2026-07-09  1:01 UTC (permalink / raw)
  To: Igor Pylypiv, Niklas Cassel; +Cc: linux-ide

[-- Attachment #1: Type: text/plain, Size: 1307 bytes --]

On 7/9/26 08:44, Damien Le Moal wrote:
> On 7/9/26 04:36, Igor Pylypiv wrote:
>> Hi Damien,
>>
>> I've stumbled upon an issue where SCSI EH didn't run upon command timeout
>> but ran ~10 or so seconds later. It seems like a recent regression from
>> the introduction of deffered_qc.
>>
>> When an active command times out while a non-NCQ command is waiting in
>> deferred_qc, SCSI EH fails to wake up. Recovery stalls until the deferred
>> command's own timer expires.
>>
>> When an NCQ command times out, scsi_timeout() calls scsi_eh_scmd_add(),
>> incrementing shost->host_failed (1). However, when scsi_eh_wakeup() checks
>> whether to wake the EH thread, scsi_host_busy(shost) counts 2 active
>> commands (1 timed-out + 1 in deferred_qc).
>>
>> Because busy (2) != host_failed (1), scsi_eh_wakeup() refuses to wake
>> the EH thread, deadlocking error recovery until the deferred command
>> times out on its own.
> 
> Igor,
> 
> Can you try the attached diff ?
> I will test on my end, but I need to hack something to trigger a timeout :)

Hacking libahci to trigger a timeout for a particular LBA read, I tested the
attached v2 and I see the deferred QC running right after the read command
timeout. So this looks like a good fix to me. Can you test please ?

-- 
Damien Le Moal
Western Digital Research

[-- Attachment #2: ata-timeout-v2.diff --]
[-- Type: text/x-patch, Size: 3138 bytes --]

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 5868526301a2..5221d7004c0e 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1685,7 +1685,8 @@ void ata_scsi_deferred_qc_work(struct work_struct *work)
 	spin_unlock_irqrestore(ap->lock, flags);
 }
 
-void ata_scsi_requeue_deferred_qc(struct ata_port *ap)
+static void ata_scsi_do_requeue_deferred_qc(struct ata_port *ap,
+					    struct scsi_cmnd *timed_out_scmd)
 {
 	struct ata_link *link;
 
@@ -1698,15 +1699,27 @@ void ata_scsi_requeue_deferred_qc(struct ata_port *ap)
 	 */
 	ata_for_each_link(link, ap, PMP_FIRST) {
 		struct ata_queued_cmd *qc = link->deferred_qc;
+		u32 host_byte;
 
-		if (qc) {
-			link->deferred_qc = NULL;
-			cancel_work(&link->deferred_qc_work);
-			ata_scsi_qc_done(qc, true, DID_REQUEUE << 16);
-		}
+		if (!qc)
+			continue;
+
+		link->deferred_qc = NULL;
+		cancel_work(&link->deferred_qc_work);
+
+		if (qc->scsicmd == timed_out_scmd)
+			host_byte = DID_TIME_OUT;
+		else
+			host_byte = DID_REQUEUE;
+		ata_scsi_qc_done(qc, true, host_byte << 16);
 	}
 }
 
+void ata_scsi_requeue_deferred_qc(struct ata_port *ap)
+{
+	ata_scsi_do_requeue_deferred_qc(ap, NULL);
+}
+
 static void ata_scsi_schedule_deferred_qc(struct ata_link *link)
 {
 	struct ata_queued_cmd *qc = link->deferred_qc;
@@ -1730,6 +1743,28 @@ static void ata_scsi_schedule_deferred_qc(struct ata_link *link)
 		queue_work(system_highpri_wq, &link->deferred_qc_work);
 }
 
+enum scsi_timeout_action ata_scsi_eh_timed_out(struct scsi_cmnd *scmd)
+{
+	struct ata_port *ap = ata_shost_to_port(scmd->device->host);
+	unsigned long flags;
+
+	/*
+	 * We had a timeout, either for an NCQ command or for one deferred
+	 * queued command. Either way, we must release all deferred queued
+	 * command so that scsi EH can trigger.
+	 */
+	spin_lock_irqsave(ap->lock, flags);
+	ata_scsi_do_requeue_deferred_qc(ap, scmd);
+	spin_unlock_irqrestore(ap->lock, flags);
+
+	/*
+	 * Let scsi_timeout() know that it must continue with handling the
+	 * timeout as we in fact did not do much here.
+	 */
+	return SCSI_EH_NOT_HANDLED;
+}
+EXPORT_SYMBOL_GPL(ata_scsi_eh_timed_out);
+
 static void ata_scsi_qc_complete(struct ata_queued_cmd *qc)
 {
 	struct ata_link *link = qc->dev->link;
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 96e626d6a7ca..327da43d7496 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -1153,6 +1153,7 @@ extern int ata_scsi_ioctl(struct scsi_device *dev, unsigned int cmd,
 #endif
 extern enum scsi_qc_status ata_scsi_queuecmd(struct Scsi_Host *h,
 					     struct scsi_cmnd *cmd);
+enum scsi_timeout_action ata_scsi_eh_timed_out(struct scsi_cmnd *cmd);
 #if IS_REACHABLE(CONFIG_ATA)
 bool ata_scsi_dma_need_drain(struct request *rq);
 #else
@@ -1464,6 +1465,7 @@ extern const struct attribute_group *ata_common_sdev_groups[];
 	.ioctl			= ata_scsi_ioctl,		\
 	ATA_SCSI_COMPAT_IOCTL					\
 	.queuecommand		= ata_scsi_queuecmd,		\
+	.eh_timed_out		= ata_scsi_eh_timed_out,	\
 	.dma_need_drain		= ata_scsi_dma_need_drain,	\
 	.this_id		= ATA_SHT_THIS_ID,		\
 	.emulated		= ATA_SHT_EMULATED,		\

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: SCSI EH wakeup deadlock from deferred_qc
  2026-07-09  1:01   ` Damien Le Moal
@ 2026-07-09  2:24     ` Igor Pylypiv
  2026-07-09  3:37       ` Damien Le Moal
  0 siblings, 1 reply; 7+ messages in thread
From: Igor Pylypiv @ 2026-07-09  2:24 UTC (permalink / raw)
  To: Damien Le Moal; +Cc: Niklas Cassel, linux-ide

[-- Attachment #1: Type: text/plain, Size: 5034 bytes --]

On Thu, Jul 09, 2026 at 10:01:11AM +0900, Damien Le Moal wrote:
> On 7/9/26 08:44, Damien Le Moal wrote:
> > On 7/9/26 04:36, Igor Pylypiv wrote:
> >> Hi Damien,
> >>
> >> I've stumbled upon an issue where SCSI EH didn't run upon command timeout
> >> but ran ~10 or so seconds later. It seems like a recent regression from
> >> the introduction of deffered_qc.
> >>
> >> When an active command times out while a non-NCQ command is waiting in
> >> deferred_qc, SCSI EH fails to wake up. Recovery stalls until the deferred
> >> command's own timer expires.
> >>
> >> When an NCQ command times out, scsi_timeout() calls scsi_eh_scmd_add(),
> >> incrementing shost->host_failed (1). However, when scsi_eh_wakeup() checks
> >> whether to wake the EH thread, scsi_host_busy(shost) counts 2 active
> >> commands (1 timed-out + 1 in deferred_qc).
> >>
> >> Because busy (2) != host_failed (1), scsi_eh_wakeup() refuses to wake
> >> the EH thread, deadlocking error recovery until the deferred command
> >> times out on its own.
> > 
> > Igor,
> > 
> > Can you try the attached diff ?
> > I will test on my end, but I need to hack something to trigger a timeout :)
> 
> Hacking libahci to trigger a timeout for a particular LBA read, I tested the
> attached v2 and I see the deferred QC running right after the read command
> timeout. So this looks like a good fix to me. Can you test please ?

Thank you for coming up with a fix so quickly, Damien!
The fix works for the issue I'm seeing!

Note: I tested on a 6.18 based kernel with your changes ported to libsas
to fix the issue for the pm80xx driver. Attached libsas diff for reference.

Thank you!
Igor

> 
> -- 
> Damien Le Moal
> Western Digital Research

> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 5868526301a2..5221d7004c0e 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -1685,7 +1685,8 @@ void ata_scsi_deferred_qc_work(struct work_struct *work)
>  	spin_unlock_irqrestore(ap->lock, flags);
>  }
>  
> -void ata_scsi_requeue_deferred_qc(struct ata_port *ap)
> +static void ata_scsi_do_requeue_deferred_qc(struct ata_port *ap,
> +					    struct scsi_cmnd *timed_out_scmd)
>  {
>  	struct ata_link *link;
>  
> @@ -1698,15 +1699,27 @@ void ata_scsi_requeue_deferred_qc(struct ata_port *ap)
>  	 */
>  	ata_for_each_link(link, ap, PMP_FIRST) {
>  		struct ata_queued_cmd *qc = link->deferred_qc;
> +		u32 host_byte;
>  
> -		if (qc) {
> -			link->deferred_qc = NULL;
> -			cancel_work(&link->deferred_qc_work);
> -			ata_scsi_qc_done(qc, true, DID_REQUEUE << 16);
> -		}
> +		if (!qc)
> +			continue;
> +
> +		link->deferred_qc = NULL;
> +		cancel_work(&link->deferred_qc_work);
> +
> +		if (qc->scsicmd == timed_out_scmd)
> +			host_byte = DID_TIME_OUT;
> +		else
> +			host_byte = DID_REQUEUE;
> +		ata_scsi_qc_done(qc, true, host_byte << 16);
>  	}
>  }
>  
> +void ata_scsi_requeue_deferred_qc(struct ata_port *ap)
> +{
> +	ata_scsi_do_requeue_deferred_qc(ap, NULL);
> +}
> +
>  static void ata_scsi_schedule_deferred_qc(struct ata_link *link)
>  {
>  	struct ata_queued_cmd *qc = link->deferred_qc;
> @@ -1730,6 +1743,28 @@ static void ata_scsi_schedule_deferred_qc(struct ata_link *link)
>  		queue_work(system_highpri_wq, &link->deferred_qc_work);
>  }
>  
> +enum scsi_timeout_action ata_scsi_eh_timed_out(struct scsi_cmnd *scmd)
> +{
> +	struct ata_port *ap = ata_shost_to_port(scmd->device->host);
> +	unsigned long flags;
> +
> +	/*
> +	 * We had a timeout, either for an NCQ command or for one deferred
> +	 * queued command. Either way, we must release all deferred queued
> +	 * command so that scsi EH can trigger.
> +	 */
> +	spin_lock_irqsave(ap->lock, flags);
> +	ata_scsi_do_requeue_deferred_qc(ap, scmd);
> +	spin_unlock_irqrestore(ap->lock, flags);
> +
> +	/*
> +	 * Let scsi_timeout() know that it must continue with handling the
> +	 * timeout as we in fact did not do much here.
> +	 */
> +	return SCSI_EH_NOT_HANDLED;
> +}
> +EXPORT_SYMBOL_GPL(ata_scsi_eh_timed_out);
> +
>  static void ata_scsi_qc_complete(struct ata_queued_cmd *qc)
>  {
>  	struct ata_link *link = qc->dev->link;
> diff --git a/include/linux/libata.h b/include/linux/libata.h
> index 96e626d6a7ca..327da43d7496 100644
> --- a/include/linux/libata.h
> +++ b/include/linux/libata.h
> @@ -1153,6 +1153,7 @@ extern int ata_scsi_ioctl(struct scsi_device *dev, unsigned int cmd,
>  #endif
>  extern enum scsi_qc_status ata_scsi_queuecmd(struct Scsi_Host *h,
>  					     struct scsi_cmnd *cmd);
> +enum scsi_timeout_action ata_scsi_eh_timed_out(struct scsi_cmnd *cmd);
>  #if IS_REACHABLE(CONFIG_ATA)
>  bool ata_scsi_dma_need_drain(struct request *rq);
>  #else
> @@ -1464,6 +1465,7 @@ extern const struct attribute_group *ata_common_sdev_groups[];
>  	.ioctl			= ata_scsi_ioctl,		\
>  	ATA_SCSI_COMPAT_IOCTL					\
>  	.queuecommand		= ata_scsi_queuecmd,		\
> +	.eh_timed_out		= ata_scsi_eh_timed_out,	\
>  	.dma_need_drain		= ata_scsi_dma_need_drain,	\
>  	.this_id		= ATA_SHT_THIS_ID,		\
>  	.emulated		= ATA_SHT_EMULATED,		\


[-- Attachment #2: 6.18_libsas_ata_timeout.diff --]
[-- Type: text/x-diff, Size: 1891 bytes --]

diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
index ffa5b49aaf08..b48dcc281042 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -500,6 +500,30 @@ int sas_eh_target_reset_handler(struct scsi_cmnd *cmd)
 }
 EXPORT_SYMBOL_GPL(sas_eh_target_reset_handler);
 
+enum scsi_timeout_action sas_eh_timed_out(struct scsi_cmnd *cmd)
+{
+	struct domain_device *dev;
+	struct ata_port *ap;
+	unsigned long flags;
+
+	if (!cmd)
+		return SCSI_EH_NOT_HANDLED;
+
+	dev = cmd_to_domain_dev(cmd);
+	if (!dev || !dev_is_sata(dev))
+		return SCSI_EH_NOT_HANDLED;
+
+	ap = dev->sata_dev.ap;
+	if (ap) {
+		spin_lock_irqsave(ap->lock, flags);
+		ata_scsi_do_requeue_deferred_qc(ap, cmd);
+		spin_unlock_irqrestore(ap->lock, flags);
+	}
+
+	return SCSI_EH_NOT_HANDLED;
+}
+EXPORT_SYMBOL_GPL(sas_eh_timed_out);
+
 /* Try to reset a device */
 static int try_to_reset_cmd_device(struct scsi_cmnd *cmd)
 {
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index c4f6a6a7f6bd..852de6a55448 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -712,6 +712,7 @@ void sas_task_abort(struct sas_task *);
 int sas_eh_abort_handler(struct scsi_cmnd *cmd);
 int sas_eh_device_reset_handler(struct scsi_cmnd *cmd);
 int sas_eh_target_reset_handler(struct scsi_cmnd *cmd);
+enum scsi_timeout_action sas_eh_timed_out(struct scsi_cmnd *cmd);
 
 extern void sas_target_destroy(struct scsi_target *);
 extern int sas_sdev_init(struct scsi_device *);
@@ -743,6 +744,7 @@ void sas_notify_phy_event(struct asd_sas_phy *phy, enum phy_event event,
 	.name				= DRV_NAME,			\
 	.proc_name			= DRV_NAME,			\
 	.queuecommand			= sas_queuecommand,		\
+	.eh_timed_out			= sas_eh_timed_out,		\
 	.dma_need_drain			= ata_scsi_dma_need_drain,	\
 	.target_alloc			= sas_target_alloc,		\
 	.change_queue_depth		= sas_change_queue_depth,	\

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: SCSI EH wakeup deadlock from deferred_qc
  2026-07-09  2:24     ` Igor Pylypiv
@ 2026-07-09  3:37       ` Damien Le Moal
  2026-07-09  4:36         ` Igor Pylypiv
  0 siblings, 1 reply; 7+ messages in thread
From: Damien Le Moal @ 2026-07-09  3:37 UTC (permalink / raw)
  To: Igor Pylypiv; +Cc: Niklas Cassel, linux-ide

On 7/9/26 11:24, Igor Pylypiv wrote:
> On Thu, Jul 09, 2026 at 10:01:11AM +0900, Damien Le Moal wrote:
>> On 7/9/26 08:44, Damien Le Moal wrote:
>>> On 7/9/26 04:36, Igor Pylypiv wrote:
>>>> Hi Damien,
>>>>
>>>> I've stumbled upon an issue where SCSI EH didn't run upon command timeout
>>>> but ran ~10 or so seconds later. It seems like a recent regression from
>>>> the introduction of deffered_qc.
>>>>
>>>> When an active command times out while a non-NCQ command is waiting in
>>>> deferred_qc, SCSI EH fails to wake up. Recovery stalls until the deferred
>>>> command's own timer expires.
>>>>
>>>> When an NCQ command times out, scsi_timeout() calls scsi_eh_scmd_add(),
>>>> incrementing shost->host_failed (1). However, when scsi_eh_wakeup() checks
>>>> whether to wake the EH thread, scsi_host_busy(shost) counts 2 active
>>>> commands (1 timed-out + 1 in deferred_qc).
>>>>
>>>> Because busy (2) != host_failed (1), scsi_eh_wakeup() refuses to wake
>>>> the EH thread, deadlocking error recovery until the deferred command
>>>> times out on its own.
>>>
>>> Igor,
>>>
>>> Can you try the attached diff ?
>>> I will test on my end, but I need to hack something to trigger a timeout :)
>>
>> Hacking libahci to trigger a timeout for a particular LBA read, I tested the
>> attached v2 and I see the deferred QC running right after the read command
>> timeout. So this looks like a good fix to me. Can you test please ?
> 
> Thank you for coming up with a fix so quickly, Damien!
> The fix works for the issue I'm seeing!
> 
> Note: I tested on a 6.18 based kernel with your changes ported to libsas
> to fix the issue for the pm80xx driver. Attached libsas diff for reference.

Thanks for the diff. I will add it to the patch, and if that's OK with you, also
add a co-developed-by tag ?

> 
> Thank you!
> Igor
> 
>>
>> -- 
>> Damien Le Moal
>> Western Digital Research
> 
>> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
>> index 5868526301a2..5221d7004c0e 100644
>> --- a/drivers/ata/libata-scsi.c
>> +++ b/drivers/ata/libata-scsi.c
>> @@ -1685,7 +1685,8 @@ void ata_scsi_deferred_qc_work(struct work_struct *work)
>>  	spin_unlock_irqrestore(ap->lock, flags);
>>  }
>>  
>> -void ata_scsi_requeue_deferred_qc(struct ata_port *ap)
>> +static void ata_scsi_do_requeue_deferred_qc(struct ata_port *ap,
>> +					    struct scsi_cmnd *timed_out_scmd)
>>  {
>>  	struct ata_link *link;
>>  
>> @@ -1698,15 +1699,27 @@ void ata_scsi_requeue_deferred_qc(struct ata_port *ap)
>>  	 */
>>  	ata_for_each_link(link, ap, PMP_FIRST) {
>>  		struct ata_queued_cmd *qc = link->deferred_qc;
>> +		u32 host_byte;
>>  
>> -		if (qc) {
>> -			link->deferred_qc = NULL;
>> -			cancel_work(&link->deferred_qc_work);
>> -			ata_scsi_qc_done(qc, true, DID_REQUEUE << 16);
>> -		}
>> +		if (!qc)
>> +			continue;
>> +
>> +		link->deferred_qc = NULL;
>> +		cancel_work(&link->deferred_qc_work);
>> +
>> +		if (qc->scsicmd == timed_out_scmd)
>> +			host_byte = DID_TIME_OUT;
>> +		else
>> +			host_byte = DID_REQUEUE;
>> +		ata_scsi_qc_done(qc, true, host_byte << 16);
>>  	}
>>  }
>>  
>> +void ata_scsi_requeue_deferred_qc(struct ata_port *ap)
>> +{
>> +	ata_scsi_do_requeue_deferred_qc(ap, NULL);
>> +}
>> +
>>  static void ata_scsi_schedule_deferred_qc(struct ata_link *link)
>>  {
>>  	struct ata_queued_cmd *qc = link->deferred_qc;
>> @@ -1730,6 +1743,28 @@ static void ata_scsi_schedule_deferred_qc(struct ata_link *link)
>>  		queue_work(system_highpri_wq, &link->deferred_qc_work);
>>  }
>>  
>> +enum scsi_timeout_action ata_scsi_eh_timed_out(struct scsi_cmnd *scmd)
>> +{
>> +	struct ata_port *ap = ata_shost_to_port(scmd->device->host);
>> +	unsigned long flags;
>> +
>> +	/*
>> +	 * We had a timeout, either for an NCQ command or for one deferred
>> +	 * queued command. Either way, we must release all deferred queued
>> +	 * command so that scsi EH can trigger.
>> +	 */
>> +	spin_lock_irqsave(ap->lock, flags);
>> +	ata_scsi_do_requeue_deferred_qc(ap, scmd);
>> +	spin_unlock_irqrestore(ap->lock, flags);
>> +
>> +	/*
>> +	 * Let scsi_timeout() know that it must continue with handling the
>> +	 * timeout as we in fact did not do much here.
>> +	 */
>> +	return SCSI_EH_NOT_HANDLED;
>> +}
>> +EXPORT_SYMBOL_GPL(ata_scsi_eh_timed_out);
>> +
>>  static void ata_scsi_qc_complete(struct ata_queued_cmd *qc)
>>  {
>>  	struct ata_link *link = qc->dev->link;
>> diff --git a/include/linux/libata.h b/include/linux/libata.h
>> index 96e626d6a7ca..327da43d7496 100644
>> --- a/include/linux/libata.h
>> +++ b/include/linux/libata.h
>> @@ -1153,6 +1153,7 @@ extern int ata_scsi_ioctl(struct scsi_device *dev, unsigned int cmd,
>>  #endif
>>  extern enum scsi_qc_status ata_scsi_queuecmd(struct Scsi_Host *h,
>>  					     struct scsi_cmnd *cmd);
>> +enum scsi_timeout_action ata_scsi_eh_timed_out(struct scsi_cmnd *cmd);
>>  #if IS_REACHABLE(CONFIG_ATA)
>>  bool ata_scsi_dma_need_drain(struct request *rq);
>>  #else
>> @@ -1464,6 +1465,7 @@ extern const struct attribute_group *ata_common_sdev_groups[];
>>  	.ioctl			= ata_scsi_ioctl,		\
>>  	ATA_SCSI_COMPAT_IOCTL					\
>>  	.queuecommand		= ata_scsi_queuecmd,		\
>> +	.eh_timed_out		= ata_scsi_eh_timed_out,	\
>>  	.dma_need_drain		= ata_scsi_dma_need_drain,	\
>>  	.this_id		= ATA_SHT_THIS_ID,		\
>>  	.emulated		= ATA_SHT_EMULATED,		\
> 


-- 
Damien Le Moal
Western Digital Research

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: SCSI EH wakeup deadlock from deferred_qc
  2026-07-09  3:37       ` Damien Le Moal
@ 2026-07-09  4:36         ` Igor Pylypiv
  0 siblings, 0 replies; 7+ messages in thread
From: Igor Pylypiv @ 2026-07-09  4:36 UTC (permalink / raw)
  To: Damien Le Moal; +Cc: Niklas Cassel, linux-ide

On Thu, Jul 09, 2026 at 12:37:07PM +0900, Damien Le Moal wrote:
> On 7/9/26 11:24, Igor Pylypiv wrote:
> > On Thu, Jul 09, 2026 at 10:01:11AM +0900, Damien Le Moal wrote:
> >> On 7/9/26 08:44, Damien Le Moal wrote:
> >>> On 7/9/26 04:36, Igor Pylypiv wrote:
> >>>> Hi Damien,
> >>>>
> >>>> I've stumbled upon an issue where SCSI EH didn't run upon command timeout
> >>>> but ran ~10 or so seconds later. It seems like a recent regression from
> >>>> the introduction of deffered_qc.
> >>>>
> >>>> When an active command times out while a non-NCQ command is waiting in
> >>>> deferred_qc, SCSI EH fails to wake up. Recovery stalls until the deferred
> >>>> command's own timer expires.
> >>>>
> >>>> When an NCQ command times out, scsi_timeout() calls scsi_eh_scmd_add(),
> >>>> incrementing shost->host_failed (1). However, when scsi_eh_wakeup() checks
> >>>> whether to wake the EH thread, scsi_host_busy(shost) counts 2 active
> >>>> commands (1 timed-out + 1 in deferred_qc).
> >>>>
> >>>> Because busy (2) != host_failed (1), scsi_eh_wakeup() refuses to wake
> >>>> the EH thread, deadlocking error recovery until the deferred command
> >>>> times out on its own.
> >>>
> >>> Igor,
> >>>
> >>> Can you try the attached diff ?
> >>> I will test on my end, but I need to hack something to trigger a timeout :)
> >>
> >> Hacking libahci to trigger a timeout for a particular LBA read, I tested the
> >> attached v2 and I see the deferred QC running right after the read command
> >> timeout. So this looks like a good fix to me. Can you test please ?
> > 
> > Thank you for coming up with a fix so quickly, Damien!
> > The fix works for the issue I'm seeing!
> > 
> > Note: I tested on a 6.18 based kernel with your changes ported to libsas
> > to fix the issue for the pm80xx driver. Attached libsas diff for reference.
> 
> Thanks for the diff. I will add it to the patch, and if that's OK with you, also
> add a co-developed-by tag ?

Of course! Thanks again for fixing this quickly, Damien!

Best,
Igor

> 
> > 
> > Thank you!
> > Igor
> > 
> >>
> >> -- 
> >> Damien Le Moal
> >> Western Digital Research
> > 
> >> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> >> index 5868526301a2..5221d7004c0e 100644
> >> --- a/drivers/ata/libata-scsi.c
> >> +++ b/drivers/ata/libata-scsi.c
> >> @@ -1685,7 +1685,8 @@ void ata_scsi_deferred_qc_work(struct work_struct *work)
> >>  	spin_unlock_irqrestore(ap->lock, flags);
> >>  }
> >>  
> >> -void ata_scsi_requeue_deferred_qc(struct ata_port *ap)
> >> +static void ata_scsi_do_requeue_deferred_qc(struct ata_port *ap,
> >> +					    struct scsi_cmnd *timed_out_scmd)
> >>  {
> >>  	struct ata_link *link;
> >>  
> >> @@ -1698,15 +1699,27 @@ void ata_scsi_requeue_deferred_qc(struct ata_port *ap)
> >>  	 */
> >>  	ata_for_each_link(link, ap, PMP_FIRST) {
> >>  		struct ata_queued_cmd *qc = link->deferred_qc;
> >> +		u32 host_byte;
> >>  
> >> -		if (qc) {
> >> -			link->deferred_qc = NULL;
> >> -			cancel_work(&link->deferred_qc_work);
> >> -			ata_scsi_qc_done(qc, true, DID_REQUEUE << 16);
> >> -		}
> >> +		if (!qc)
> >> +			continue;
> >> +
> >> +		link->deferred_qc = NULL;
> >> +		cancel_work(&link->deferred_qc_work);
> >> +
> >> +		if (qc->scsicmd == timed_out_scmd)
> >> +			host_byte = DID_TIME_OUT;
> >> +		else
> >> +			host_byte = DID_REQUEUE;
> >> +		ata_scsi_qc_done(qc, true, host_byte << 16);
> >>  	}
> >>  }
> >>  
> >> +void ata_scsi_requeue_deferred_qc(struct ata_port *ap)
> >> +{
> >> +	ata_scsi_do_requeue_deferred_qc(ap, NULL);
> >> +}
> >> +
> >>  static void ata_scsi_schedule_deferred_qc(struct ata_link *link)
> >>  {
> >>  	struct ata_queued_cmd *qc = link->deferred_qc;
> >> @@ -1730,6 +1743,28 @@ static void ata_scsi_schedule_deferred_qc(struct ata_link *link)
> >>  		queue_work(system_highpri_wq, &link->deferred_qc_work);
> >>  }
> >>  
> >> +enum scsi_timeout_action ata_scsi_eh_timed_out(struct scsi_cmnd *scmd)
> >> +{
> >> +	struct ata_port *ap = ata_shost_to_port(scmd->device->host);
> >> +	unsigned long flags;
> >> +
> >> +	/*
> >> +	 * We had a timeout, either for an NCQ command or for one deferred
> >> +	 * queued command. Either way, we must release all deferred queued
> >> +	 * command so that scsi EH can trigger.
> >> +	 */
> >> +	spin_lock_irqsave(ap->lock, flags);
> >> +	ata_scsi_do_requeue_deferred_qc(ap, scmd);
> >> +	spin_unlock_irqrestore(ap->lock, flags);
> >> +
> >> +	/*
> >> +	 * Let scsi_timeout() know that it must continue with handling the
> >> +	 * timeout as we in fact did not do much here.
> >> +	 */
> >> +	return SCSI_EH_NOT_HANDLED;
> >> +}
> >> +EXPORT_SYMBOL_GPL(ata_scsi_eh_timed_out);
> >> +
> >>  static void ata_scsi_qc_complete(struct ata_queued_cmd *qc)
> >>  {
> >>  	struct ata_link *link = qc->dev->link;
> >> diff --git a/include/linux/libata.h b/include/linux/libata.h
> >> index 96e626d6a7ca..327da43d7496 100644
> >> --- a/include/linux/libata.h
> >> +++ b/include/linux/libata.h
> >> @@ -1153,6 +1153,7 @@ extern int ata_scsi_ioctl(struct scsi_device *dev, unsigned int cmd,
> >>  #endif
> >>  extern enum scsi_qc_status ata_scsi_queuecmd(struct Scsi_Host *h,
> >>  					     struct scsi_cmnd *cmd);
> >> +enum scsi_timeout_action ata_scsi_eh_timed_out(struct scsi_cmnd *cmd);
> >>  #if IS_REACHABLE(CONFIG_ATA)
> >>  bool ata_scsi_dma_need_drain(struct request *rq);
> >>  #else
> >> @@ -1464,6 +1465,7 @@ extern const struct attribute_group *ata_common_sdev_groups[];
> >>  	.ioctl			= ata_scsi_ioctl,		\
> >>  	ATA_SCSI_COMPAT_IOCTL					\
> >>  	.queuecommand		= ata_scsi_queuecmd,		\
> >> +	.eh_timed_out		= ata_scsi_eh_timed_out,	\
> >>  	.dma_need_drain		= ata_scsi_dma_need_drain,	\
> >>  	.this_id		= ATA_SHT_THIS_ID,		\
> >>  	.emulated		= ATA_SHT_EMULATED,		\
> > 
> 
> 
> -- 
> Damien Le Moal
> Western Digital Research

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-07-09  4:36 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-08 19:36 SCSI EH wakeup deadlock from deferred_qc Igor Pylypiv
2026-07-08 22:54 ` Damien Le Moal
2026-07-08 23:44 ` Damien Le Moal
2026-07-09  1:01   ` Damien Le Moal
2026-07-09  2:24     ` Igor Pylypiv
2026-07-09  3:37       ` Damien Le Moal
2026-07-09  4:36         ` Igor Pylypiv

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.