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

On 7/9/26 18:00, [email protected] wrote:
> [Severity: High]
> Does calling ata_scsi_qc_done() here prematurely complete the timed-out
> deferred command and free the libata qc?

Yes it does. But unless it is the deferred QC that suffered the timeout, the
scsi command will be requeued and retried.

> When a deferred command times out, ata_scsi_eh_timed_out() calls this function
> which sets the host byte to DID_TIME_OUT and calls ata_scsi_qc_done().
> ata_scsi_qc_done() then frees the qc and invokes the qc->scsidone callback.
> 
> Later, when the SCSI completion softirq routes the DID_TIME_OUT command to the
> EH queue and the EH thread runs ata_scsi_cmd_error_handler():
> 
> drivers/ata/libata-eh.c:ata_scsi_cmd_error_handler() {
>     ...
>     set_host_byte(scmd, DID_OK);
>     ata_qc_for_each_raw(ap, qc, i) {
>         if (qc->scsicmd != scmd)
>             continue;
>         if ((qc->flags & ATA_QCFLAG_ACTIVE) ||
>             qc == qc->dev->link->deferred_qc)
>             break;
>     }
>     ...
> }
> 
> Because the qc was already freed, the ATA_QCFLAG_ACTIVE flag is cleared
> and deferred_qc is NULL. The loop will fail to find the active command.
> 
> It seems this causes the handler to fall into the 'raced completion' branch:
> 
> drivers/ata/libata-eh.c:ata_scsi_cmd_error_handler() {
>     ...
>     } else {
>         /* Normal completion occurred after
>          * SCSI timeout but before this point.
>          * Successfully complete it.
>          */
>         scmd->retries = scmd->allowed;
>         scsi_eh_finish_cmd(scmd, &ap->eh_done_q);
>     }
>     ...
> }

And after this, scsi_eh_flush_done_q() will retry the command.
Tested and I do not see failure of the deferred QC :)


-- 
Damien Le Moal
Western Digital Research