[PATCH 0/3] ata: fix deferred QC handling for port multipliers

[PATCH 0/3] ata: fix deferred QC handling for port multipliers

[Niklas Cassel]

Hello all,

Tommy Kelly reported a regression with PMP that use CBS:
https://lore.kernel.org/linux-ide/ce09cc21-a8e9-4845-b205-35411e22fba9@tkel.ly/

To me, this appears to be a problem in ata_pmp_qc_defer_cmd_switch() that
has been there since this function was introduced. ap->excl_link did not
always get set. This is fixed in patch 1/3.

While looking at the code, when using a PMP with CBS, it turns out that we
are incorrectly using the deferred_qc feature that issues a QC via a
workqueue, even for non-NCQ commands that are issued to another link
(a link that is not the active link). This workqueue feature was meant to
avoid dealing with non-NCQ vs NCQ commands for a single drive, not to
duplicate the existing ap->excl_link logic. This is fixed in patch 2/3.

While looking at the code, it turns out that the deferred qc issuing via
workqueue is misdesigned. It assumed that we can't mix NCQ and non-NCQ
commands on the same port. The limitation is that you can not mix NCQ and
non-NCQ commands on the same drive. However, with a PMP with FBS, you can
issue (mixed NCQ and non-NCQ commands) to the different drives. Thus, move
the saved deferred QC from struct ata_port to struct ata_link. This is
fixed in patch 3/3.

Tommy, it would be nice if you could apply this series and see if it solves
your problem. Right now the series is compile tested only.


Niklas Cassel (3):
  ata: libata-pmp: fix ata_pmp_qc_defer_cmd_switch()
  ata: libata-scsi: do not use the deferred QC feature on PMPs with CBS
  ata: libata-scsi: do not needlessly defer commands when using PMP with
    FBS

 drivers/ata/libata-core.c | 16 +++++++---
 drivers/ata/libata-eh.c   |  8 ++---
 drivers/ata/libata-pmp.c  | 13 +++++---
 drivers/ata/libata-scsi.c | 66 ++++++++++++++++++++++-----------------
 include/linux/libata.h    |  6 ++--
 5 files changed, 64 insertions(+), 45 deletions(-)

-- 
2.54.0

[PATCH 1/3] ata: libata-pmp: fix ata_pmp_qc_defer_cmd_switch()

[Niklas Cassel]

When using Port Multipliers (PMPs) with Command-Based Switching (CBS), you
can only issue commands to one link at a time.

This is implemented in sata_pmp_qc_defer_cmd_switch() by setting
ap->excl_link if there is no link currently being exclusive, and setting
qc->flags ATA_QCFLAG_CLEAR_EXCL to clear the exlusive link in
__ata_qc_complete().

ATA_QCFLAG_CLEAR_EXCL is only set if this is the first link to become
active, or if (since CBS can only have one active link) the current link
is active. __ata_qc_complete() clears ap->excl_link as long as
ATA_QCFLAG_CLEAR_EXCL is set, it does not matter if there are still
outstanding requests to the same link. This works because
sata_pmp_qc_defer_cmd_switch() will only allow new commands to be queued
if (ap->nr_active_links == 0 || ata_link_active(link)).

However, since sata_pmp_qc_defer_cmd_switch() does a:
return ata_std_qc_defer(qc); before setting ap->excl_link, this means that
in the case where ap->excl_link == NULL and ata_std_qc_defer() returns 0
(i.e. the QC should not be deferred), we would not set ap->excl_link.

Additionally, think of the following example:
NCQ command1 to drive1 issued:
sets ATA_QCFLAG_CLEAR_EXCL
does not set ap->excl_link

NCQ command2 to drive1 issued:
sets ATA_QCFLAG_CLEAR_EXCL
does not set ap->excl_link

NCQ command3 to drive2 issued:
ap->excl_link gets set
does not set ATA_QCFLAG_CLEAR_EXCL
returns ATA_DEFER_PORT

For command3, it seems wrong to set ap->excl_link while also deferring the
command with ATA_DEFER_PORT.

Modify sata_pmp_qc_defer_cmd_switch() to only set ap->excl_link if we can
actually issue a command (i.e. if we don't return ATA_DEFER_PORT or
ATA_DEFER_LINK).

Fixes: 31f88384443b ("libata-pmp: implement qc_defer for command switching PMP support")
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/ata/libata-pmp.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/ata/libata-pmp.c b/drivers/ata/libata-pmp.c
index e3adc008fed1..0575e1e283d8 100644
--- a/drivers/ata/libata-pmp.c
+++ b/drivers/ata/libata-pmp.c
@@ -113,11 +113,14 @@ int sata_pmp_qc_defer_cmd_switch(struct ata_queued_cmd *qc)
 
 	if (ap->excl_link == NULL || ap->excl_link == link) {
 		if (ap->nr_active_links == 0 || ata_link_active(link)) {
+			int ret;
+
 			qc->flags |= ATA_QCFLAG_CLEAR_EXCL;
-			return ata_std_qc_defer(qc);
+			ret = ata_std_qc_defer(qc);
+			if (!ret)
+				ap->excl_link = link;
+			return ret;
 		}
-
-		ap->excl_link = link;
 	}
 
 	return ATA_DEFER_PORT;
-- 
2.54.0

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

[Niklas Cassel]

When using Port Multipliers (PMPs) with Command-Based Switching (CBS), you
can only issue commands to one link at a time. For PMPs with CBS, there is
already code to handle commands being send to different links in
sata_pmp_qc_defer_cmd_switch(), which will return ATA_DEFER_PORT when
trying to send a command (NCQ or non-NCQ) to the non-active link.

After commit 0ea84089dbf6 ("ata: libata-scsi: avoid Non-NCQ command
starvation"), in addition to the existing handling PMP CBS handling, we
would also, incorrectly, save a non-NCQ command to another link in
ap->deferred_qc. The deferred qc feature was only meant to defer QCs to
the same link, not to duplicate the existing PMP CBS deferal logic when
dealing with separate links.

Note that when deferring commands to the same link (i.e. because NCQ and
non-NCQ commands are mixed), even when using a PMP with CBS, we will use
the deferred_qc feature that issues the deferred qc via a workqueue
(because sata_pmp_qc_defer_cmd_switch() in this case ATA_DEFER_LINK is
returned).

Since the deferred_qc issuing via workqueue was only meant to deal with a
single link, modify the code to feature to only set link->deferred_qc when
deferring a qc to the same link (ATA_DEFER_LINK), and not when deferring a
qc because we are not issuing a qc to the active link (ATA_DEFER_PORT).

If we want to modify the scope of the workqueue issuing to also handle the
case where we issue commands to the link that is not the active link, then
we should ensure that it can save both NCQ and non-NCQ commands, while also
removing some of the existing PMP CBS handling, such that we don't
duplicate features.

Thus, modify ata_scsi_qc_issue() to only use the deferred_qc workqueue for
ATA_DEFER_LINK and not for ATA_DEFER_PORT.

Fixes: 0ea84089dbf6 ("ata: libata-scsi: avoid Non-NCQ command starvation")
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/ata/libata-scsi.c | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index f44612e269a4..dca7ea7e6315 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1785,18 +1785,6 @@ static int ata_scsi_qc_issue(struct ata_port *ap, struct ata_queued_cmd *qc)
 	case 0:
 		break;
 	case ATA_DEFER_LINK:
-		ret = SCSI_MLQUEUE_DEVICE_BUSY;
-		break;
-	case ATA_DEFER_PORT:
-		ret = SCSI_MLQUEUE_HOST_BUSY;
-		break;
-	default:
-		WARN_ON_ONCE(1);
-		ret = SCSI_MLQUEUE_HOST_BUSY;
-		break;
-	}
-
-	if (ret) {
 		/*
 		 * We must defer this qc: if this is not an NCQ command, keep
 		 * this qc as a deferred one and report to the SCSI layer that
@@ -1811,7 +1799,20 @@ static int ata_scsi_qc_issue(struct ata_port *ap, struct ata_queued_cmd *qc)
 
 		/* Force a requeue of the command to defer its execution. */
 		ata_qc_free(qc);
-		return ret;
+		return SCSI_MLQUEUE_DEVICE_BUSY;
+	case ATA_DEFER_PORT:
+		/*
+		 * ATA_DEFER_PORT is returned when the host is busy, e.g. when
+		 * using a PMP with CBS and trying to issue a qc to a link that
+		 * is not the currently active link. In this case, simply
+		 * propagate the error back to the SCSI layer.
+		 */
+		ata_qc_free(qc);
+		return SCSI_MLQUEUE_HOST_BUSY;
+	default:
+		WARN_ON_ONCE(1);
+		ata_qc_free(qc);
+		return SCSI_MLQUEUE_HOST_BUSY;
 	}
 
 issue:
-- 
2.54.0

[PATCH 3/3] ata: libata-scsi: do not needlessly defer commands when using PMP with FBS

[Niklas Cassel]

The SATA specification does not allow a non-NCQ command to be issued while
an NCQ command is outstanding.

Commit 0ea84089dbf6 ("ata: libata-scsi: avoid Non-NCQ command starvation")
introduced a feature where a deferred non-NCQ command gets issued from a
workqueue. The design stores a single non-NCQ command per port.

However, when using Port Multipliers (PMPs), specifically PMPs that
support FIS-Based Switching (FBS), non-NCQ and NCQ commands can be mixed
on the same port, just not for the same link, see e.g. ata_std_qc_defer()
which is, and always has operated on a per-link basis.

Therefore, move the deferred_qc from struct ata_port to struct ata_link.
This way, when using a PMP with FBS, we will not needlessly defer commands
to all other links, just because one link issued a non-NCQ command while
having an NCQ command outstanding. Only commands for that specific link
will be deferred.

Fixes: 0ea84089dbf6 ("ata: libata-scsi: avoid Non-NCQ command starvation")
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/ata/libata-core.c | 16 +++++++++++-----
 drivers/ata/libata-eh.c   |  8 ++++----
 drivers/ata/libata-pmp.c  |  4 +++-
 drivers/ata/libata-scsi.c | 39 +++++++++++++++++++++++----------------
 include/linux/libata.h    |  6 +++---
 5 files changed, 44 insertions(+), 29 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index e76d15411e2a..c3a10e85a19c 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -5584,6 +5584,7 @@ void ata_link_init(struct ata_port *ap, struct ata_link *link, int pmp)
 	link->pmp = pmp;
 	link->active_tag = ATA_TAG_POISON;
 	link->hw_sata_spd_limit = UINT_MAX;
+	INIT_WORK(&link->deferred_qc_work, ata_scsi_deferred_qc_work);
 
 	/* can't use iterator, ap isn't initialized yet */
 	for (i = 0; i < ATA_MAX_DEVICES; i++) {
@@ -5666,7 +5667,6 @@ struct ata_port *ata_port_alloc(struct ata_host *host)
 	mutex_init(&ap->scsi_scan_mutex);
 	INIT_DELAYED_WORK(&ap->hotplug_task, ata_scsi_hotplug);
 	INIT_DELAYED_WORK(&ap->scsi_rescan_task, ata_scsi_dev_rescan);
-	INIT_WORK(&ap->deferred_qc_work, ata_scsi_deferred_qc_work);
 	INIT_LIST_HEAD(&ap->eh_done_q);
 	init_waitqueue_head(&ap->eh_wait_q);
 	init_completion(&ap->park_req_pending);
@@ -6291,9 +6291,9 @@ static void ata_port_detach(struct ata_port *ap)
 
 	/* It better be dead now and not have any remaining deferred qc. */
 	WARN_ON(!(ap->pflags & ATA_PFLAG_UNLOADED));
-	WARN_ON(ap->deferred_qc);
+	WARN_ON(ap->link.deferred_qc);
 
-	cancel_work_sync(&ap->deferred_qc_work);
+	cancel_work_sync(&ap->link.deferred_qc_work);
 	cancel_delayed_work_sync(&ap->hotplug_task);
 	cancel_delayed_work_sync(&ap->scsi_rescan_task);
 
@@ -6301,8 +6301,14 @@ static void ata_port_detach(struct ata_port *ap)
 	if (ap->pmp_link) {
 		int i;
 
-		for (i = 0; i < SATA_PMP_MAX_PORTS; i++)
-			ata_tlink_delete(&ap->pmp_link[i]);
+		for (i = 0; i < SATA_PMP_MAX_PORTS; i++) {
+			struct ata_link *pmp_link = &ap->pmp_link[i];
+
+			WARN_ON(pmp_link->deferred_qc);
+			cancel_work_sync(&pmp_link->deferred_qc_work);
+
+			ata_tlink_delete(pmp_link);
+		}
 	}
 
 	/* Remove the associated SCSI host */
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 9a4b67b90b17..d623eb32ed8b 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -651,11 +651,11 @@ int ata_scsi_cmd_error_handler(struct Scsi_Host *host, struct ata_port *ap,
 			if (qc->scsicmd != scmd)
 				continue;
 			if ((qc->flags & ATA_QCFLAG_ACTIVE) ||
-			    qc == ap->deferred_qc)
+			    qc == qc->dev->link->deferred_qc)
 				break;
 		}
 
-		if (i < ATA_MAX_QUEUE && qc == ap->deferred_qc) {
+		if (i < ATA_MAX_QUEUE && qc == qc->dev->link->deferred_qc) {
 			/*
 			 * This is a deferred command that timed out while
 			 * waiting for the command queue to drain. Since the qc
@@ -666,8 +666,8 @@ int ata_scsi_cmd_error_handler(struct Scsi_Host *host, struct ata_port *ap,
 			 * deferred qc work from issuing this qc.
 			 */
 			WARN_ON_ONCE(qc->flags & ATA_QCFLAG_ACTIVE);
-			ap->deferred_qc = NULL;
-			cancel_work(&ap->deferred_qc_work);
+			qc->dev->link->deferred_qc = NULL;
+			cancel_work(&qc->dev->link->deferred_qc_work);
 			set_host_byte(scmd, DID_TIME_OUT);
 			scsi_eh_finish_cmd(scmd, &ap->eh_done_q);
 		} else if (i < ATA_MAX_QUEUE) {
diff --git a/drivers/ata/libata-pmp.c b/drivers/ata/libata-pmp.c
index 0575e1e283d8..b0c484a591b8 100644
--- a/drivers/ata/libata-pmp.c
+++ b/drivers/ata/libata-pmp.c
@@ -574,8 +574,10 @@ static void sata_pmp_detach(struct ata_device *dev)
 	if (ap->ops->pmp_detach)
 		ap->ops->pmp_detach(ap);
 
-	ata_for_each_link(tlink, ap, EDGE)
+	ata_for_each_link(tlink, ap, EDGE) {
+		cancel_work_sync(&tlink->deferred_qc_work);
 		ata_eh_detach_dev(tlink->device);
+	}
 
 	spin_lock_irqsave(ap->lock, flags);
 	ap->nr_pmp_links = 0;
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index dca7ea7e6315..201dac46d0a3 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1664,8 +1664,9 @@ static void ata_scsi_qc_done(struct ata_queued_cmd *qc, bool set_result,
 
 void ata_scsi_deferred_qc_work(struct work_struct *work)
 {
-	struct ata_port *ap =
-		container_of(work, struct ata_port, deferred_qc_work);
+	struct ata_link *link =
+		container_of(work, struct ata_link, deferred_qc_work);
+	struct ata_port *ap = link->ap;
 	struct ata_queued_cmd *qc;
 	unsigned long flags;
 
@@ -1676,10 +1677,10 @@ void ata_scsi_deferred_qc_work(struct work_struct *work)
 	 * such case, we should not need any more deferring the qc, so warn if
 	 * qc_defer() says otherwise.
 	 */
-	qc = ap->deferred_qc;
+	qc = link->deferred_qc;
 	if (qc && !ata_port_eh_scheduled(ap)) {
 		WARN_ON_ONCE(ap->ops->qc_defer(qc));
-		ap->deferred_qc = NULL;
+		link->deferred_qc = NULL;
 		ata_qc_issue(qc);
 	}
 
@@ -1688,7 +1689,7 @@ void ata_scsi_deferred_qc_work(struct work_struct *work)
 
 void ata_scsi_requeue_deferred_qc(struct ata_port *ap)
 {
-	struct ata_queued_cmd *qc = ap->deferred_qc;
+	struct ata_link *link;
 
 	lockdep_assert_held(ap->lock);
 
@@ -1697,16 +1698,21 @@ void ata_scsi_requeue_deferred_qc(struct ata_port *ap)
 	 * do not try to be smart about what to do with this deferred command
 	 * and simply requeue it by completing it with DID_REQUEUE.
 	 */
-	if (qc) {
-		ap->deferred_qc = NULL;
-		cancel_work(&ap->deferred_qc_work);
-		ata_scsi_qc_done(qc, true, DID_REQUEUE << 16);
+	ata_for_each_link(link, ap, EDGE) {
+		struct ata_queued_cmd *qc = link->deferred_qc;
+
+		if (qc) {
+			link->deferred_qc = NULL;
+			cancel_work(&link->deferred_qc_work);
+			ata_scsi_qc_done(qc, true, DID_REQUEUE << 16);
+		}
 	}
 }
 
-static void ata_scsi_schedule_deferred_qc(struct ata_port *ap)
+static void ata_scsi_schedule_deferred_qc(struct ata_link *link)
 {
-	struct ata_queued_cmd *qc = ap->deferred_qc;
+	struct ata_queued_cmd *qc = link->deferred_qc;
+	struct ata_port *ap = link->ap;
 
 	lockdep_assert_held(ap->lock);
 
@@ -1723,12 +1729,12 @@ static void ata_scsi_schedule_deferred_qc(struct ata_port *ap)
 		return;
 	}
 	if (!ap->ops->qc_defer(qc))
-		queue_work(system_highpri_wq, &ap->deferred_qc_work);
+		queue_work(system_highpri_wq, &link->deferred_qc_work);
 }
 
 static void ata_scsi_qc_complete(struct ata_queued_cmd *qc)
 {
-	struct ata_port *ap = qc->ap;
+	struct ata_link *link = qc->dev->link;
 	struct scsi_cmnd *cmd = qc->scsicmd;
 	u8 *cdb = cmd->cmnd;
 	bool have_sense = qc->flags & ATA_QCFLAG_SENSE_VALID;
@@ -1759,11 +1765,12 @@ static void ata_scsi_qc_complete(struct ata_queued_cmd *qc)
 
 	ata_scsi_qc_done(qc, false, 0);
 
-	ata_scsi_schedule_deferred_qc(ap);
+	ata_scsi_schedule_deferred_qc(link);
 }
 
 static int ata_scsi_qc_issue(struct ata_port *ap, struct ata_queued_cmd *qc)
 {
+	struct ata_link *link = qc->dev->link;
 	int ret;
 
 	if (!ap->ops->qc_defer)
@@ -1774,7 +1781,7 @@ static int ata_scsi_qc_issue(struct ata_port *ap, struct ata_queued_cmd *qc)
 	 * requeue and defer all incoming commands until the deferred qc is
 	 * processed, once all on-going commands complete.
 	 */
-	if (ap->deferred_qc) {
+	if (link->deferred_qc) {
 		ata_qc_free(qc);
 		return SCSI_MLQUEUE_DEVICE_BUSY;
 	}
@@ -1793,7 +1800,7 @@ static int ata_scsi_qc_issue(struct ata_port *ap, struct ata_queued_cmd *qc)
 		 * commands complete.
 		 */
 		if (!ata_is_ncq(qc->tf.protocol)) {
-			ap->deferred_qc = qc;
+			link->deferred_qc = qc;
 			return 0;
 		}
 
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 5c085ef4eda7..a04384e67b84 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -854,6 +854,9 @@ struct ata_link {
 	unsigned int		sata_spd;	/* current SATA PHY speed */
 	enum ata_lpm_policy	lpm_policy;
 
+	struct work_struct	deferred_qc_work;
+	struct ata_queued_cmd	*deferred_qc;
+
 	/* record runtime error info, protected by host_set lock */
 	struct ata_eh_info	eh_info;
 	/* EH context */
@@ -899,9 +902,6 @@ struct ata_port {
 	u64			qc_active;
 	int			nr_active_links; /* #links with active qcs */
 
-	struct work_struct	deferred_qc_work;
-	struct ata_queued_cmd	*deferred_qc;
-
 	struct ata_link		link;		/* host default link */
 	struct ata_link		*slave_link;	/* see ata_slave_link_init() */
 
-- 
2.54.0

Re: [PATCH 0/3] ata: fix deferred QC handling for port multipliers

[John Garry]

On 01/05/2026 13:54, Niklas Cassel wrote:
> Hello all,
> 
> Tommy Kelly reported a regression with PMP that use CBS:
> https://lore.kernel.org/linux-ide/ce09cc21-a8e9-4845-b205-35411e22fba9@tkel.ly/
> 
> To me, this appears to be a problem in ata_pmp_qc_defer_cmd_switch() that
> has been there since this function was introduced. ap->excl_link did not
> always get set. This is fixed in patch 1/3.
> 
> While looking at the code, when using a PMP with CBS, it turns out that we
> are incorrectly using the deferred_qc feature that issues a QC via a
> workqueue, even for non-NCQ commands that are issued to another link
> (a link that is not the active link). This workqueue feature was meant to
> avoid dealing with non-NCQ vs NCQ commands for a single drive, not to
> duplicate the existing ap->excl_link logic. This is fixed in patch 2/3.
> 
> While looking at the code, it turns out that the deferred qc issuing via
> workqueue is misdesigned. It assumed that we can't mix NCQ and non-NCQ
> commands on the same port. The limitation is that you can not mix NCQ and
> non-NCQ commands on the same drive. However, with a PMP with FBS, you can
> issue (mixed NCQ and non-NCQ commands) to the different drives. Thus, move
> the saved deferred QC from struct ata_port to struct ata_link. This is
> fixed in patch 3/3.
> 
> Tommy, it would be nice if you could apply this series and see if it solves
> your problem. Right now the series is compile tested only.
> 
> 

Hi Niklas,

As I understand, this deferred QC scheme was intended to solve 
starvation problems for multiqueue SAS HBAs (using libsas-based 
drivers). libsas has no port multiplier support, so I wonder why we make 
deferred QC scheme work for port multiplier, i.e. non-libsas scenario.

Thanks,
John

> Niklas Cassel (3):
>    ata: libata-pmp: fix ata_pmp_qc_defer_cmd_switch()
>    ata: libata-scsi: do not use the deferred QC feature on PMPs with CBS
>    ata: libata-scsi: do not needlessly defer commands when using PMP with
>      FBS
> 
>   drivers/ata/libata-core.c | 16 +++++++---
>   drivers/ata/libata-eh.c   |  8 ++---
>   drivers/ata/libata-pmp.c  | 13 +++++---
>   drivers/ata/libata-scsi.c | 66 ++++++++++++++++++++++-----------------
>   include/linux/libata.h    |  6 ++--
>   5 files changed, 64 insertions(+), 45 deletions(-)
> 

Re: [PATCH 0/3] ata: fix deferred QC handling for port multipliers

[Niklas Cassel]

On Fri, May 01, 2026 at 03:19:03PM +0100, John Garry wrote:
> On 01/05/2026 13:54, Niklas Cassel wrote:
> 
> As I understand, this deferred QC scheme was intended to solve starvation
> problems for multiqueue SAS HBAs (using libsas-based drivers). libsas has no
> port multiplier support, so I wonder why we make deferred QC scheme work for
> port multiplier, i.e. non-libsas scenario.

Are you certain that there are no libsas HBAs that support port
multipliers?


Basically, before commit:
0ea84089dbf6 ("ata: libata-scsi: avoid Non-NCQ command starvation")

We always aborted a new command, if that would have caused NCQ and non-NCQ
commands to be mixed on the same device.
Regardless if libsas driver or any other SATA driver, e.g. AHCI.


After commit:
0ea84089dbf6 ("ata: libata-scsi: avoid Non-NCQ command starvation")
If sending a non-NCQ command while NCQ commands are in flight,
we instead save the non-NCQ command and issue it as soon all NCQ
commands are finished, from a workqueue.

This commit changed the behavior both for libsas based drivers and
all other SATA drivers, including AHCI, which does support Port
Multipliers.

As you might now, Port Multipliers are not that common, and I think
simply no one tested the change on Port Multipliers before the change
was included.


As you can see in patch 2/3, we basically stop using this deferred QC
issuing via workqueue for Port Multipliers with CBS - when sending
commands to different devices.

For Port Multipliers with FBS support, or PMPs with CBS support where
issuing commands within the same drive, we are still using it.

For PMPs with FBS, I don't see any problem of keep using the workqueue
to issue a deferred QC. It should basically be the same as if directly
connected on the port. (There is no extra logic to handle ap->excl_link.)


For PMPs with CBS, we could keep the same behavior as before, and just
abort the command, if NCQ and non-NCQ are mixed.

The advantage is that the workqueue issuing would not be able to somehow
collide with the ap->excl_link handling for CBS PMPs. Looking at the code,
it "should work", because we always will always have ATA_QCFLAG_CLEAR_EXCL
set on the QC issued by the workqueue.

This would require that we know that PMPs (with CBS) can't be used on
libsas based HBAs. Hell, we could probably do that if we wanted to, since
I don't think that people using an expense SAS HBA with a cheap CBS PMP
(does not not even support FBS) is very common in practice.,,


Kind regards,
Niklas

Re: [PATCH 0/3] ata: fix deferred QC handling for port multipliers

[John Garry]

On 01/05/2026 19:42, Niklas Cassel wrote:
> On Fri, May 01, 2026 at 03:19:03PM +0100, John Garry wrote:
>> On 01/05/2026 13:54, Niklas Cassel wrote:
>>
>> As I understand, this deferred QC scheme was intended to solve starvation
>> problems for multiqueue SAS HBAs (using libsas-based drivers). libsas has no
>> port multiplier support, so I wonder why we make deferred QC scheme work for
>> port multiplier, i.e. non-libsas scenario.
> 
> Are you certain that there are no libsas HBAs that support port
> multipliers?

No, I am just saying that libsas does not support port multipliers, see 
earlier effort @ 
https://lore.kernel.org/linux-scsi/1398346023-10225-1-git-send-email-yxlraid@gmail.com/

See this today:

int sas_discover_sata(struct domain_device *dev)
{
	if (dev->dev_type == SAS_SATA_PM)
		return -ENODEV;

> 
> 
> Basically, before commit:
> 0ea84089dbf6 ("ata: libata-scsi: avoid Non-NCQ command starvation")
> 
> We always aborted a new command, if that would have caused NCQ and non-NCQ
> commands to be mixed on the same device.
> Regardless if libsas driver or any other SATA driver, e.g. AHCI.
> 
> 
> After commit:
> 0ea84089dbf6 ("ata: libata-scsi: avoid Non-NCQ command starvation")
> If sending a non-NCQ command while NCQ commands are in flight,
> we instead save the non-NCQ command and issue it as soon all NCQ
> commands are finished, from a workqueue.
> 
> This commit changed the behavior both for libsas based drivers and
> all other SATA drivers, including AHCI, which does support Port
> Multipliers.
> 
> As you might now, Port Multipliers are not that common, and I think
> simply no one tested the change on Port Multipliers before the change
> was included.
> 
> 
> As you can see in patch 2/3, we basically stop using this deferred QC
> issuing via workqueue for Port Multipliers with CBS - when sending
> commands to different devices.
> 
> For Port Multipliers with FBS support, or PMPs with CBS support where
> issuing commands within the same drive, we are still using it.
> 
> For PMPs with FBS, I don't see any problem of keep using the workqueue
> to issue a deferred QC. It should basically be the same as if directly
> connected on the port. (There is no extra logic to handle ap->excl_link.)
> 
> 
> For PMPs with CBS, we could keep the same behavior as before, and just
> abort the command, if NCQ and non-NCQ are mixed.
> 
> The advantage is that the workqueue issuing would not be able to somehow
> collide with the ap->excl_link handling for CBS PMPs. Looking at the code,
> it "should work", because we always will always have ATA_QCFLAG_CLEAR_EXCL
> set on the QC issued by the workqueue.
> 
> This would require that we know that PMPs (with CBS) can't be used on
> libsas based HBAs.

Please see above.

> Hell, we could probably do that if we wanted to, since
> I don't think that people using an expense SAS HBA with a cheap CBS PMP
> (does not not even support FBS) is very common in practice.,,

Yes, SAS has expanders for this purpose.

Thanks,
John

Re: [PATCH 0/3] ata: fix deferred QC handling for port multipliers

[Niklas Cassel]

Hello John,

On Mon, May 04, 2026 at 08:27:06AM +0100, John Garry wrote:
> On 01/05/2026 19:42, Niklas Cassel wrote:
> > On Fri, May 01, 2026 at 03:19:03PM +0100, John Garry wrote:
> > > On 01/05/2026 13:54, Niklas Cassel wrote:
> > > 
> > > As I understand, this deferred QC scheme was intended to solve starvation
> > > problems for multiqueue SAS HBAs (using libsas-based drivers). libsas has no
> > > port multiplier support, so I wonder why we make deferred QC scheme work for
> > > port multiplier, i.e. non-libsas scenario.
> > 
> > Are you certain that there are no libsas HBAs that support port
> > multipliers?
> 
> No, I am just saying that libsas does not support port multipliers, see
> earlier effort @ https://lore.kernel.org/linux-scsi/1398346023-10225-1-git-send-email-yxlraid@gmail.com/
> 
> See this today:
> 
> int sas_discover_sata(struct domain_device *dev)
> {
> 	if (dev->dev_type == SAS_SATA_PM)
> 		return -ENODEV;
>

I see, thank you for the information!

Will send a V2 soon.


Kind regards,
Niklas

Re: [PATCH 0/3] ata: fix deferred QC handling for port multipliers

[Tommy Kelly]

On 5/1/26 05:54, Niklas Cassel wrote:
> Hello all,
> 
> Tommy Kelly reported a regression with PMP that use CBS:
> https://lore.kernel.org/linux-ide/ce09cc21-a8e9-4845-b205-35411e22fba9@tkel.ly/

> 
> Tommy, it would be nice if you could apply this series and see if it solves
> your problem. Right now the series is compile tested only.
> 
> 
> Niklas Cassel (3):
>   ata: libata-pmp: fix ata_pmp_qc_defer_cmd_switch()
>   ata: libata-scsi: do not use the deferred QC feature on PMPs with CBS
>   ata: libata-scsi: do not needlessly defer commands when using PMP with
>     FBS
> 
>  drivers/ata/libata-core.c | 16 +++++++---
>  drivers/ata/libata-eh.c   |  8 ++---
>  drivers/ata/libata-pmp.c  | 13 +++++---
>  drivers/ata/libata-scsi.c | 66 ++++++++++++++++++++++-----------------
>  include/linux/libata.h    |  6 ++--
>  5 files changed, 64 insertions(+), 45 deletions(-)
> 

Hi Niklas,

Yes, this patch series fixed the regression for me with a CBS Port Multiplier.

Here are the commits I tested with:

> (HEAD -> fedora-7.0-libata) ata: libata-scsi: do not needlessly defer commands when using PMP with FBS
> ata: libata-scsi: do not use the deferred QC feature on PMPs with CBS
> ata: libata-pmp: fix ata_pmp_qc_defer_cmd_switch()
> ata: ahci: fail probe if BAR too small for claimed ports
> ata: libahci: use ahci_nr_ports() helper
> ata: pata_parport: switch to dynamic root device
> Merge remote-tracking branch 'libata/for-7.1' into fedora-7.0-libata
> (tag: kernel-7.0.3-0, kernel-ark/fedora-7.0, fedora-7.0) [redhat] kernel-7.0.3-0
> Merge Linux v7.0.3
> (tag: v7.0.3, kernel-ark/linux-7.0.y, linux-7.0.y) Linux 7.0.3

And here's some logs showing ata_id (non-NCQ) resolving immediately.
After, I'm able to boot and mount and use the filesystem no problem.
No errors and no timeouts.

> Mar 12 17:00:10 localhost kernel: ahci-dwc fc800000.sata: port 0 is not capable of FBS
> May 02 17:12:39 (udev-worker)[874]: sdd: Starting 'ata_id --export /dev/sdd'
> May 02 17:12:39 (udev-worker)[874]: sdd: 'ata_id --export /dev/sdd'(out) 'ID_ATA=1'
> May 02 17:12:39 (udev-worker)[874]: sdd: Process 'ata_id --export /dev/sdd' succeeded.

> $ cat /sys/block/sd*/device/queue_depth
> 32
> 32
> 32
> 32
> 32

I will also apply the FBS devicetree change (5918bf2) and test with that too.

Thank you for your work on this!

- Tommy

Re: [PATCH 0/3] ata: fix deferred QC handling for port multipliers

[Tommy Kelly]

On 5/2/26 17:59, Tommy Kelly wrote:
> 
> I will also apply the FBS devicetree change (5918bf2) and test with that too.
> 

Special thank you to Heiko for the FBS support!
His patch setting PxCMD.FBSCP works: https://lore.kernel.org/all/20260201191804.41421-2-heiko@sntech.de/

I compiled the FBS devicetree change into u-boot (which passes the fdt to linux),
and running on unpatched linux 6.19.12,
FBS works, and no regressions are present.
Logs:

> Linux version 6.19.12-200.fc43.aarch64
> ahci-dwc fc800000.sata: supply ahci not found, using dummy regulator
> ahci-dwc fc800000.sata: supply phy not found, using dummy regulator
> platform fc800000.sata:sata-port@0: supply target not found, using dummy regulator
> ahci-dwc fc800000.sata: supply ahci not found, using dummy regulator
> ahci-dwc fc800000.sata: supply phy not found, using dummy regulator
> platform fc800000.sata:sata-port@0: supply target not found, using dummy regulator
> ahci-dwc fc800000.sata: supply ahci not found, using dummy regulator
> ahci-dwc fc800000.sata: supply phy not found, using dummy regulator
> platform fc800000.sata:sata-port@0: supply target not found, using dummy regulator
> ahci-dwc fc800000.sata: PMPn is limited up to 5 ports
> ahci-dwc fc800000.sata: forcing port_map 0x0 -> 0x1
> ahci-dwc fc800000.sata: masking port_map 0x1 -> 0x1
> ahci-dwc fc800000.sata: AHCI vers 0001.0300, 32 command slots, 6 Gbps, platform mode
> ahci-dwc fc800000.sata: 1/1 ports implemented (port mask 0x1)
> ahci-dwc fc800000.sata: flags: ncq sntf pm led clo only pmp fbs pio slum part ccc apst 
> scsi host0: ahci-dwc
> ata1: SATA max UDMA/133 mmio [mem 0xfc800000-0xfc800fff] port 0x100 irq 59 lpm-pol 0
> ata1: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
> ata1.15: Port Multiplier 1.2, 0x197b:0x5755 r0, 5 ports, feat 0x5/0xf
> ahci-dwc fc800000.sata: FBS is enabled
> ata1.00: hard resetting link
> ahci-dwc fc800000.sata: FBS is disabled
> ahci-dwc fc800000.sata: FBS is enabled
> ata1.00: SATA link up 3.0 Gbps (SStatus 123 SControl 330)
> ata1.01: hard resetting link
> ahci-dwc fc800000.sata: FBS is disabled
> ahci-dwc fc800000.sata: FBS is enabled
> ata1.01: SATA link up 6.0 Gbps (SStatus 133 SControl 330)
> ata1.02: hard resetting link
> ahci-dwc fc800000.sata: FBS is disabled
> ahci-dwc fc800000.sata: FBS is enabled
> ata1.02: SATA link up 3.0 Gbps (SStatus 123 SControl 330)
> ata1.03: hard resetting link
> ahci-dwc fc800000.sata: FBS is disabled
> ahci-dwc fc800000.sata: FBS is enabled
> ata1.03: SATA link up 6.0 Gbps (SStatus 133 SControl 330)
> ata1.04: hard resetting link
> ahci-dwc fc800000.sata: FBS is disabled
> ahci-dwc fc800000.sata: FBS is enabled
> ata1.04: SATA link up 6.0 Gbps (SStatus 133 SControl 330)


Again, that is before this patch series, I see no queue_depth regression with FBS.
As opposed to CBS which had a regression in the 6.19.x series.

I can test with this patch series + FBS if you'd like.

- Tommy

Re: [PATCH 0/3] ata: fix deferred QC handling for port multipliers

[Heiko Stuebner]

Am Mittwoch, 6. Mai 2026, 02:13:41 Mitteleuropäische Sommerzeit schrieb Tommy Kelly:
> On 5/2/26 17:59, Tommy Kelly wrote:
> > 
> > I will also apply the FBS devicetree change (5918bf2) and test with that too.
> > 
> 
> Special thank you to Heiko for the FBS support!
> His patch setting PxCMD.FBSCP works: https://lore.kernel.org/all/20260201191804.41421-2-heiko@sntech.de/

well that is a pleasant surprise :-D .

I just needed the port node to hook y phy-supply into, and therefore
checked what capabilities were supported/which properties needed to go
in there ... but really glad it helps you too :-)

Heiko


> I compiled the FBS devicetree change into u-boot (which passes the fdt to linux),
> and running on unpatched linux 6.19.12,
> FBS works, and no regressions are present.
> Logs:
> 
> > Linux version 6.19.12-200.fc43.aarch64
> > ahci-dwc fc800000.sata: supply ahci not found, using dummy regulator
> > ahci-dwc fc800000.sata: supply phy not found, using dummy regulator
> > platform fc800000.sata:sata-port@0: supply target not found, using dummy regulator
> > ahci-dwc fc800000.sata: supply ahci not found, using dummy regulator
> > ahci-dwc fc800000.sata: supply phy not found, using dummy regulator
> > platform fc800000.sata:sata-port@0: supply target not found, using dummy regulator
> > ahci-dwc fc800000.sata: supply ahci not found, using dummy regulator
> > ahci-dwc fc800000.sata: supply phy not found, using dummy regulator
> > platform fc800000.sata:sata-port@0: supply target not found, using dummy regulator
> > ahci-dwc fc800000.sata: PMPn is limited up to 5 ports
> > ahci-dwc fc800000.sata: forcing port_map 0x0 -> 0x1
> > ahci-dwc fc800000.sata: masking port_map 0x1 -> 0x1
> > ahci-dwc fc800000.sata: AHCI vers 0001.0300, 32 command slots, 6 Gbps, platform mode
> > ahci-dwc fc800000.sata: 1/1 ports implemented (port mask 0x1)
> > ahci-dwc fc800000.sata: flags: ncq sntf pm led clo only pmp fbs pio slum part ccc apst 
> > scsi host0: ahci-dwc
> > ata1: SATA max UDMA/133 mmio [mem 0xfc800000-0xfc800fff] port 0x100 irq 59 lpm-pol 0
> > ata1: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
> > ata1.15: Port Multiplier 1.2, 0x197b:0x5755 r0, 5 ports, feat 0x5/0xf
> > ahci-dwc fc800000.sata: FBS is enabled
> > ata1.00: hard resetting link
> > ahci-dwc fc800000.sata: FBS is disabled
> > ahci-dwc fc800000.sata: FBS is enabled
> > ata1.00: SATA link up 3.0 Gbps (SStatus 123 SControl 330)
> > ata1.01: hard resetting link
> > ahci-dwc fc800000.sata: FBS is disabled
> > ahci-dwc fc800000.sata: FBS is enabled
> > ata1.01: SATA link up 6.0 Gbps (SStatus 133 SControl 330)
> > ata1.02: hard resetting link
> > ahci-dwc fc800000.sata: FBS is disabled
> > ahci-dwc fc800000.sata: FBS is enabled
> > ata1.02: SATA link up 3.0 Gbps (SStatus 123 SControl 330)
> > ata1.03: hard resetting link
> > ahci-dwc fc800000.sata: FBS is disabled
> > ahci-dwc fc800000.sata: FBS is enabled
> > ata1.03: SATA link up 6.0 Gbps (SStatus 133 SControl 330)
> > ata1.04: hard resetting link
> > ahci-dwc fc800000.sata: FBS is disabled
> > ahci-dwc fc800000.sata: FBS is enabled
> > ata1.04: SATA link up 6.0 Gbps (SStatus 133 SControl 330)
> 
> 
> Again, that is before this patch series, I see no queue_depth regression with FBS.
> As opposed to CBS which had a regression in the 6.19.x series.
> 
> I can test with this patch series + FBS if you'd like.
> 
> - Tommy
> 
> 

Re: [PATCH 0/3] ata: fix deferred QC handling for port multipliers

[Niklas Cassel]

Hello Tommy,

On Tue, May 05, 2026 at 05:13:41PM -0700, Tommy Kelly wrote:
> > 
> > I will also apply the FBS devicetree change (5918bf2) and test with that too.
> > 
> 
> Special thank you to Heiko for the FBS support!
> His patch setting PxCMD.FBSCP works: https://lore.kernel.org/all/20260201191804.41421-2-heiko@sntech.de/
> 
> I compiled the FBS devicetree change into u-boot (which passes the fdt to linux),
> and running on unpatched linux 6.19.12,
> FBS works, and no regressions are present.

Nice!


> Again, that is before this patch series, I see no queue_depth regression with FBS.
> As opposed to CBS which had a regression in the 6.19.x series.
> 
> I can test with this patch series + FBS if you'd like.

That would very much be appreciated if you could.

Since:
1) This series actually modifies PMP FBS behavior. Basically after this
   series, the performance should be the same as with 6.19.
   (We currently have a performance regression in mainline.)
2) I do not have a PMP with FBS available to test with.
3) I tried to reproduce your problem with a PMP with CBS, but was
   unsuccessful to reproduce it.

Thank you once again for your help!


Kind regards,
Niklas

Re: [PATCH 0/3] ata: fix deferred QC handling for port multipliers

[Niklas Cassel]

Hello Tommy,

On Sat, May 02, 2026 at 05:59:12PM -0700, Tommy Kelly wrote:
> 
> Yes, this patch series fixed the regression for me with a CBS Port Multiplier.
> 
> Here are the commits I tested with:
> 
> > (HEAD -> fedora-7.0-libata) ata: libata-scsi: do not needlessly defer commands when using PMP with FBS
> > ata: libata-scsi: do not use the deferred QC feature on PMPs with CBS
> > ata: libata-pmp: fix ata_pmp_qc_defer_cmd_switch()
> > ata: ahci: fail probe if BAR too small for claimed ports
> > ata: libahci: use ahci_nr_ports() helper
> > ata: pata_parport: switch to dynamic root device
> > Merge remote-tracking branch 'libata/for-7.1' into fedora-7.0-libata
> > (tag: kernel-7.0.3-0, kernel-ark/fedora-7.0, fedora-7.0) [redhat] kernel-7.0.3-0
> > Merge Linux v7.0.3
> > (tag: v7.0.3, kernel-ark/linux-7.0.y, linux-7.0.y) Linux 7.0.3
> 
> And here's some logs showing ata_id (non-NCQ) resolving immediately.
> After, I'm able to boot and mount and use the filesystem no problem.
> No errors and no timeouts.
> 
> > Mar 12 17:00:10 localhost kernel: ahci-dwc fc800000.sata: port 0 is not capable of FBS
> > May 02 17:12:39 (udev-worker)[874]: sdd: Starting 'ata_id --export /dev/sdd'
> > May 02 17:12:39 (udev-worker)[874]: sdd: 'ata_id --export /dev/sdd'(out) 'ID_ATA=1'
> > May 02 17:12:39 (udev-worker)[874]: sdd: Process 'ata_id --export /dev/sdd' succeeded.

This all looks normal to me.

Thank you very much for testing!


Kind regards,
Niklas