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

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing call to ata_scsi_schedule_deferred_qc() in atapi_qc_complete() leads to starvation and timeouts of deferred commands.
--

--- Patch [1]: [PATCH v7 1/2] ata: libata-scsi: terminate deferred commands on time out ---
commit 3b26adffba8da435c70bdca03b617603f9f9ae76
Author: Damien Le Moal <[email protected]>

ata: libata-scsi: terminate deferred commands on time out

This commit avoids unnecessary SCSI EH task wake-ups by scheduling
a retry of all waiting deferred QCs using the new ata_scsi_eh_timed_out
operation. It also updates ata_scsi_requeue_deferred_qc to terminate
timed-out deferred commands directly and schedule libata EH to prevent
the block layer from re-issuing them immediately.

> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 5868526301a2b..c2f2f711fd987 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c

[ ... ]

> @@ -2936,7 +3014,15 @@ static void atapi_qc_complete(struct ata_queued_cmd *qc)
>  		if (qc->cdb[0] == ALLOW_MEDIUM_REMOVAL && qc->dev->sdev)
>  			qc->dev->sdev->locked = 0;
>  
> -		ata_scsi_qc_done(qc, true, SAM_STAT_CHECK_CONDITION);
> +		if (cmd->result)
> +			ata_scsi_qc_done(qc, false, 0);
> +		else
> +			ata_scsi_qc_done(qc, true, SAM_STAT_CHECK_CONDITION);
> +		return;
> +	}
> +
> +	if (cmd->result) {
> +		ata_scsi_qc_done(qc, false, 0);
>  		return;
>  	}
>  

[Severity: High]
This isn't a bug introduced by this patch, but should atapi_qc_complete()
be calling ata_scsi_schedule_deferred_qc(link) before returning?

Unlike ata_scsi_qc_complete() which kicks deferred commands,
atapi_qc_complete() misses this call on all of its exit paths, including
the ones modified here. If an ATAPI command is executing on a shared PATA
link and a concurrent command is issued to the other device, it gets
deferred by ata_std_qc_defer(). When the ATAPI command finishes, it
appears the deferred command will be trapped in link->deferred_qc and
never issued, eventually failing with DID_TIME_OUT and causing device hangs.

Could this function be missing a call to ata_scsi_schedule_deferred_qc()
to prevent starvation?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1