Re: [PATCH v2 1/2] ata: libata-scsi: terminate deferred commands on time out

On 7/10/26 17:06, Niklas Cassel wrote:
> On Fri, Jul 10, 2026 at 04:25:38PM +0900, Damien Le Moal wrote:
>>> [Severity: High]
>>> Does this function need to set ATA_PFLAG_EH_PENDING?
>>>
>>> Because this flag is not set, ata_port_eh_scheduled() will evaluate to
>>> false. This allows the block layer to immediately retry the requeued
>>> commands, which __ata_scsi_queuecmd() will accept and re-defer, keeping
>>> the host busy.
>>
>> No it will not because this is called with SCSI EH already waiting to run, so
>> newly incoming commands are not passed down by the scsi layer.
> 
> Could you please point to specific functions + lines in the code?
> 
> AFAICT, I don't see how SCSI EH is waiting to run when
> scsi_eh_scmd_add() has not yet been called.

The wait is at the beginning of the EH task:

int scsi_error_handler(void *data)
{
...
              if ((shost->host_failed == 0 && shost->host_eh_scheduled == 0) ||
                  shost->host_failed != scsi_host_busy(shost)) {
                        SCSI_LOG_ERROR_RECOVERY(1,
                                shost_printk(KERN_INFO, shost,
                                             "scsi_eh_%d: sleeping\n",
                                             shost->host_no));
                        schedule();
                        continue;
                }

and... looking at the code from there, indeed, scsi_timeout() calls eventually
scsi_eh_scmd_add() which increases host_failed. So looks like Sashiko has a
point here, and we could see the requeued deferred QCs immediately re-issued,
which would keep scsi_host_busy() incremented.

Grrr... Need to dig further.
> 
> Note that all the places that currently call ata_scsi_requeue_deferred_qc()
> are called when ATA_PFLAG_EH_PENDING | ATA_PFLAG_EH_IN_PROGRESS
> (ata_port_eh_scheduled() evaluates to true).
> 
> We could add an WARN_ON(!ata_port_eh_scheduled) in ata_scsi_requeue_deferred_qc()
> and we would never see the warning.

You will see the WARN for a timeout because in that case, we enter EH from an
expired block layer timeout calling scsi_timeout(), not from an error IRQ
signaled by the ATA controller. So we do not have ATA_PFLAG_EH_PENDING set.

> With the new function ata_scsi_port_eh_timed_out(), AFAICT, it can call
> ata_scsi_requeue_deferred_qc() before ata_port_eh_scheduled() evaluates to true.

Yes.

> See e.g. the commit message for:
> e20e81a24a4d ("ata: libata-core: do not issue non-internal commands once EH is pending")
> of why I think __ata_scsi_queuecmd() will accept new commands.

Yes, it seems that nothing blocks new commands until scsi_eh_scmd_add() as that
is the function setting the host state to recovery.

So back to the drawing board. A simple requeue will not cut it.

-- 
Damien Le Moal
Western Digital Research