Re: SCSI EH wakeup deadlock from deferred_qc

[Igor Pylypiv <ipylypiv@google.com>]

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