Re: [PATCH v4 3/4] ata: libata-scsi: do not use the deferred QC feature on PMPs with CBS

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The `ATA_DEFER_LINK_EXCL` case incorrectly uses `goto defer_qc;` instead of `goto free_qc;`, causing the command to still be stored in the incompatible deferred workqueue.
--

commit ea0512ae247e5510bc9041eabca7f2d8e0abcc9e
Author: Niklas Cassel <cassel@kernel.org>

ata: libata-scsi: do not use the deferred QC feature on PMPs with CBS

This commit introduces the ATA_DEFER_LINK_EXCL return value to prevent
commands from being deferred via the workqueue when using Port Multipliers
(PMPs) with Command-Based Switching (CBS). It modifies
sata_pmp_qc_defer_cmd_switch() and sil24_qc_defer() to return this new
value, and updates ata_scsi_qc_issue() to handle it without storing the
command in the deferred_qc workqueue.

[ ... ]

> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 22642a0e6b843..02f2d7416c839 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -1787,6 +1787,14 @@ static int ata_scsi_qc_issue(struct ata_port *ap, struct ata_queued_cmd *qc)
>  	case ATA_DEFER_LINK:
>  		ret = SCSI_MLQUEUE_DEVICE_BUSY;
>  		goto defer_qc;
> +	case ATA_DEFER_LINK_EXCL:
> +		/*
> +		 * Drivers making use of ap->excl_link cannot store the QC in
> +		 * ap->deferred_qc, because the ap->excl_link handling is
> +		 * incompatible with the ap->deferred_qc workqueue handling.
> +		 */
> +		ret = SCSI_MLQUEUE_DEVICE_BUSY;
> +		goto defer_qc;

Does this still store the command in the incompatible deferred workqueue?

If this jumps to defer_qc, a non-NCQ command will fall into this block in
ata_scsi_qc_issue():

defer_qc:
        if (!ata_is_ncq(qc->tf.protocol)) {
                ap->deferred_qc = qc;
                return 0;
        }

Should this case use goto free_qc instead to force a requeue without caching
the command in ap->deferred_qc and losing the SCSI_MLQUEUE_DEVICE_BUSY return
value?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260513151359.1075403-6-cassel@kernel.org?part=3