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

[PATCH v4 0/4] 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/

It turns out that the ap->excl_link logic used for PMP that use CBS is
incompatible with the deferred qc issuing via a workqueue.
This is fixed in patch 3/4.

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 4/4.


Changes since v3:
-Use iter_mode PMP_FIRST rather than EDGE in ata_for_each_link(), such that
 the slave_link is included.
-Add patch 1/4 that is a cleanup patch.
-Add patch 2/4 that changes so that the deferred QC is only used for
 ATA_DEFER_LINK (i.e. not used for ATA_DEFER_PORT).
-Renamed ATA_DEFER_LINK_CBS to ATA_DEFER_LINK_EXCL.


Niklas Cassel (4):
  ata: libata-scsi: improve readability of ata_scsi_qc_issue()
  ata: libata-scsi: do not use the deferred QC feature for
    ATA_DEFER_PORT
  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 |  9 ++--
 drivers/ata/libata-eh.c   |  8 ++--
 drivers/ata/libata-pmp.c  | 18 +++++++-
 drivers/ata/libata-scsi.c | 89 +++++++++++++++++++++++----------------
 drivers/ata/sata_sil24.c  |  6 ++-
 include/linux/libata.h    |  7 +--
 6 files changed, 87 insertions(+), 50 deletions(-)


base-commit: 79b6e4dd96ec663ef51bb52b00e0bbbdf0ec9009
-- 
2.54.0

[PATCH v4 1/4] ata: libata-scsi: improve readability of ata_scsi_qc_issue()

[Niklas Cassel]

Improve readability of ata_scsi_qc_issue().

No functional changes.

Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/ata/libata-scsi.c | 43 +++++++++++++++++++--------------------
 1 file changed, 21 insertions(+), 22 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 6733f2b14521..b9064dd4390c 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1767,7 +1767,7 @@ static int ata_scsi_qc_issue(struct ata_port *ap, struct ata_queued_cmd *qc)
 	int ret;
 
 	if (!ap->ops->qc_defer)
-		goto issue;
+		goto issue_qc;
 
 	/*
 	 * If we already have a deferred qc, then rely on the SCSI layer to
@@ -1786,38 +1786,37 @@ static int ata_scsi_qc_issue(struct ata_port *ap, struct ata_queued_cmd *qc)
 		break;
 	case ATA_DEFER_LINK:
 		ret = SCSI_MLQUEUE_DEVICE_BUSY;
-		break;
+		goto defer_qc;
 	case ATA_DEFER_PORT:
 		ret = SCSI_MLQUEUE_HOST_BUSY;
-		break;
+		goto defer_qc;
 	default:
 		WARN_ON_ONCE(1);
 		ret = SCSI_MLQUEUE_HOST_BUSY;
-		break;
+		goto defer_qc;
 	}
 
-	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
-		 * we issued it so that it is not requeued. The deferred qc will
-		 * be issued with the port deferred_qc_work once all on-going
-		 * commands complete.
-		 */
-		if (!ata_is_ncq(qc->tf.protocol)) {
-			ap->deferred_qc = qc;
-			return 0;
-		}
+issue_qc:
+	ata_qc_issue(qc);
+	return 0;
 
-		/* Force a requeue of the command to defer its execution. */
-		ata_qc_free(qc);
-		return ret;
+defer_qc:
+	/*
+	 * 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
+	 * we issued it so that it is not requeued. The deferred qc will
+	 * be issued with the port deferred_qc_work once all on-going
+	 * commands complete.
+	 */
+	if (!ata_is_ncq(qc->tf.protocol)) {
+		ap->deferred_qc = qc;
+		return 0;
 	}
 
-issue:
-	ata_qc_issue(qc);
+	/* Force a requeue of the command to defer its execution. */
+	ata_qc_free(qc);
 
-	return 0;
+	return ret;
 }
 
 /**
-- 
2.54.0

[PATCH v4 2/4] ata: libata-scsi: do not use the deferred QC feature for ATA_DEFER_PORT

[Niklas Cassel]

The deferred QC feature was meant to handle mixed NCQ and non-NCQ commands,
i.e. for return value ATA_DEFER_LINK.

ATA_DEFER_PORT is returned by PATA drivers, but also certain SATA drivers
like sata_mv and sata_sil24 that uses ap->excl_link to workaround hardware
bugs in these HBAs. Regardless of the reason, using the deferred QC feature
for ATA_DEFER_PORT is always wrong, and will break the ap->excl_link usage
of the SATA drivers that rely on that feature.

Modify ata_scsi_qc_issue() to only use the deferred QC feature when mixing
NCQ and non-NCQ commands, i.e. ATA_DEFER_LINK.

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

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index b9064dd4390c..22642a0e6b84 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1789,11 +1789,11 @@ static int ata_scsi_qc_issue(struct ata_port *ap, struct ata_queued_cmd *qc)
 		goto defer_qc;
 	case ATA_DEFER_PORT:
 		ret = SCSI_MLQUEUE_HOST_BUSY;
-		goto defer_qc;
+		goto free_qc;
 	default:
 		WARN_ON_ONCE(1);
 		ret = SCSI_MLQUEUE_HOST_BUSY;
-		goto defer_qc;
+		goto free_qc;
 	}
 
 issue_qc:
@@ -1813,6 +1813,7 @@ static int ata_scsi_qc_issue(struct ata_port *ap, struct ata_queued_cmd *qc)
 		return 0;
 	}
 
+free_qc:
 	/* Force a requeue of the command to defer its execution. */
 	ata_qc_free(qc);
 
-- 
2.54.0

[PATCH v4 3/4] 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 sent to different links in
sata_pmp_qc_defer_cmd_switch() using ap->excl_link. sata_sil24 also makes
use of ap->excl_link.

A user on the list reported that commit 0ea84089dbf6 ("ata: libata-scsi:
avoid Non-NCQ command starvation") broke PMPs with CBS. The commit
introduced code that stores a deferred qc in ap->deferred_qc, to later be
issued via a workqueue. It turns out that this change is incompatible with
the existing ap->excl_link handling used by PMPs with CBS.

Thus, modify sata_pmp_qc_defer_cmd_switch() and sil24_qc_defer() to return
ATA_DEFER_LINK_EXCL, and make sure that the deferred QC handling via
workqueue is not used for this return value.

This way, PMPs with CBS will work once again. Note that the starvation
referenced in commit 0ea84089dbf6 ("ata: libata-scsi: avoid Non-NCQ
command starvation") can only happen on libsas ports, and libsas does not
support Port Multipliers, thus there is no harm of reverting back to the
previous way of deferring commands for PMPs with CBS.

Non-libsas ports connected to anything but a PMP with CBS (e.g. a normal
drive or a PMP with FBS) will continue using the deferred workqueue, since
it does result in lower completion latencies for non-NCQ commands, even
though the workqueue is not strictly needed to avoid starvation for
non-libsas ports.

If we want to modify the scope of the workqueue issuing to also handle
PMPs with CBS, then we should ensure that we can save both NCQ and non-NCQ
commands in ap->deferred_qc, while also removing the existing PMP CBS
handling using ap->excl_link, such that we don't duplicate features.

While at it, also add a comment explaining how the ap->excl_link mechanism
works.

Fixes: 0ea84089dbf6 ("ata: libata-scsi: avoid Non-NCQ command starvation")
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/ata/libata-pmp.c  | 13 ++++++++++++-
 drivers/ata/libata-scsi.c |  8 ++++++++
 drivers/ata/sata_sil24.c  |  6 +++++-
 include/linux/libata.h    |  1 +
 4 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/libata-pmp.c b/drivers/ata/libata-pmp.c
index e3adc008fed1..7e889534d73b 100644
--- a/drivers/ata/libata-pmp.c
+++ b/drivers/ata/libata-pmp.c
@@ -110,13 +110,24 @@ int sata_pmp_qc_defer_cmd_switch(struct ata_queued_cmd *qc)
 {
 	struct ata_link *link = qc->dev->link;
 	struct ata_port *ap = link->ap;
+	int ret;
 
 	if (ap->excl_link == NULL || ap->excl_link == link) {
 		if (ap->nr_active_links == 0 || ata_link_active(link)) {
 			qc->flags |= ATA_QCFLAG_CLEAR_EXCL;
-			return ata_std_qc_defer(qc);
+			ret = ata_std_qc_defer(qc);
+			if (ret == ATA_DEFER_LINK)
+				return ATA_DEFER_LINK_EXCL;
+			return ret;
 		}
 
+		/*
+		 * Note: ap->excl_link contains the link that is next in line,
+		 * i.e. implicit round robin. If there is only one link
+		 * dispatching, ap->excl_link will be left unclaimed, allowing
+		 * other links to set ap->excl_link, ensuring that the currently
+		 * active link cannot queue any more.
+		 */
 		ap->excl_link = link;
 	}
 
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 22642a0e6b84..02f2d7416c83 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;
 	case ATA_DEFER_PORT:
 		ret = SCSI_MLQUEUE_HOST_BUSY;
 		goto free_qc;
diff --git a/drivers/ata/sata_sil24.c b/drivers/ata/sata_sil24.c
index d642ece9f07a..57f1081b86db 100644
--- a/drivers/ata/sata_sil24.c
+++ b/drivers/ata/sata_sil24.c
@@ -789,6 +789,7 @@ static int sil24_qc_defer(struct ata_queued_cmd *qc)
 	struct ata_link *link = qc->dev->link;
 	struct ata_port *ap = link->ap;
 	u8 prot = qc->tf.protocol;
+	int ret;
 
 	/*
 	 * There is a bug in the chip:
@@ -826,7 +827,10 @@ static int sil24_qc_defer(struct ata_queued_cmd *qc)
 		qc->flags |= ATA_QCFLAG_CLEAR_EXCL;
 	}
 
-	return ata_std_qc_defer(qc);
+	ret = ata_std_qc_defer(qc);
+	if (ret == ATA_DEFER_LINK)
+		return ATA_DEFER_LINK_EXCL;
+	return ret;
 }
 
 static enum ata_completion_errors sil24_qc_prep(struct ata_queued_cmd *qc)
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 5c085ef4eda7..360776016b50 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -371,6 +371,7 @@ enum {
 	/* return values for ->qc_defer */
 	ATA_DEFER_LINK		= 1,
 	ATA_DEFER_PORT		= 2,
+	ATA_DEFER_LINK_EXCL	= 3,
 
 	/* desc_len for ata_eh_info and context */
 	ATA_EH_DESC_LEN		= 80,
-- 
2.54.0

[PATCH v4 4/4] 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 |  9 +++++---
 drivers/ata/libata-eh.c   |  8 ++++----
 drivers/ata/libata-pmp.c  |  5 ++++-
 drivers/ata/libata-scsi.c | 43 +++++++++++++++++++++++----------------
 include/linux/libata.h    |  6 +++---
 5 files changed, 42 insertions(+), 29 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index e76d15411e2a..3d0027ec33c2 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,12 +6291,15 @@ 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);
 
-	cancel_work_sync(&ap->deferred_qc_work);
 	cancel_delayed_work_sync(&ap->hotplug_task);
 	cancel_delayed_work_sync(&ap->scsi_rescan_task);
 
+	ata_for_each_link(link, ap, PMP_FIRST) {
+		WARN_ON(link->deferred_qc);
+		cancel_work_sync(&link->deferred_qc_work);
+	}
+
 	/* Delete port multiplier link transport devices */
 	if (ap->pmp_link) {
 		int i;
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 7e889534d73b..e8540931b4a1 100644
--- a/drivers/ata/libata-pmp.c
+++ b/drivers/ata/libata-pmp.c
@@ -582,8 +582,11 @@ 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) {
+		WARN_ON(tlink->deferred_qc);
+		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 02f2d7416c83..bd09d48d718d 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, PMP_FIRST) {
+		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;
 	}
@@ -1790,8 +1797,8 @@ static int ata_scsi_qc_issue(struct ata_port *ap, struct ata_queued_cmd *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.
+		 * link->deferred_qc, because the ap->excl_link handling is
+		 * incompatible with the link->deferred_qc workqueue handling.
 		 */
 		ret = SCSI_MLQUEUE_DEVICE_BUSY;
 		goto defer_qc;
@@ -1817,7 +1824,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 360776016b50..127229fbd1a6 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -855,6 +855,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 */
@@ -900,9 +903,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 v4 0/4] ata: fix deferred QC handling for port multipliers

[Tommy Kelly]

On 5/13/26 08:13, Niklas Cassel wrote: 
> Changes since v3:
> -Use iter_mode PMP_FIRST rather than EDGE in ata_for_each_link(), such that
>  the slave_link is included.
> -Add patch 1/4 that is a cleanup patch.
> -Add patch 2/4 that changes so that the deferred QC is only used for
>  ATA_DEFER_LINK (i.e. not used for ATA_DEFER_PORT).
> -Renamed ATA_DEFER_LINK_CBS to ATA_DEFER_LINK_EXCL.
> 
> 
> Niklas Cassel (4):
>   ata: libata-scsi: improve readability of ata_scsi_qc_issue()
>   ata: libata-scsi: do not use the deferred QC feature for
>     ATA_DEFER_PORT
>   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 |  9 ++--
>  drivers/ata/libata-eh.c   |  8 ++--
>  drivers/ata/libata-pmp.c  | 18 +++++++-
>  drivers/ata/libata-scsi.c | 89 +++++++++++++++++++++++----------------
>  drivers/ata/sata_sil24.c  |  6 ++-
>  include/linux/libata.h    |  7 +--
>  6 files changed, 87 insertions(+), 50 deletions(-)
> 
> 
> base-commit: 79b6e4dd96ec663ef51bb52b00e0bbbdf0ec9009

Hi Niklas + Damien,

I built linux v7.1-rc3 with this v4 patchset and libata/for-next applied.

With FBS, no warnings, errors, or noticable usability issues.

With CBS strange things are happening.
The first boot, I got a warning. It happened right at the beginning of the fsck-on-mount:

> kernel: ahci-dwc fc800000.sata: port 0 is not capable of FBS
> 19:29:15 smartd[1045]: Device: /dev/sdc [SAT], is SMART capable. Adding to "monitor" list.
> 19:29:15 smartd[1045]: Device: /dev/sdd, type changed from 'scsi' to 'sat'
> 19:29:15 smartd[1045]: Device: /dev/sdd [SAT], opened
> 19:29:15 smartd[1045]: Device: /dev/sdd [SAT], ADATA SU655, S/N:2J0720016679, WWN:0-000000-000000000, FW:V8X01c62, 240 GB
> 19:29:15 kernel: bcachefs: accounting_read... done (0 seconds)
> 19:29:15 smartd[1045]: Device: /dev/sdd [SAT], found in smartd database 7.5/6028: Silicon Motion based SSDs
> 19:29:16 kernel: ------------[ cut here ]------------
> 19:29:16 kernel: WARNING: drivers/ata/libata-scsi.c:1682 at ata_scsi_deferred_qc_work+0x88/0x98, CPU#0: kworker/0:1H/603
> 19:29:16 kernel: CPU: 0 UID: 0 PID: 603 Comm: kworker/0:1H Tainted: G           OE      ------  ---  7.1.0-0.rc3.1d5dcaa3bd65.25.test.fc44.aarch64 #1 PREEMPT(lazy) 
> 19:29:16 kernel: Hardware name: hardkernel Hardkernel ODROID-M1/Hardkernel ODROID-M1, BIOS 2026.04 04/01/2026
> 19:29:16 kernel: Workqueue: events_highpri ata_scsi_deferred_qc_work
> 19:29:16 kernel: pc : ata_scsi_deferred_qc_work+0x88/0x98
> 19:29:16 kernel: lr : ata_scsi_deferred_qc_work+0x54/0x98
> 19:29:16 kernel: Call trace:
> 19:29:16 kernel:  ata_scsi_deferred_qc_work+0x88/0x98 (P)
> 19:29:16 kernel:  process_one_work+0x188/0x520
> 19:29:16 kernel:  worker_thread+0x194/0x2f8
> 19:29:16 kernel:  kthread+0x11c/0x140
> 19:29:16 kernel:  ret_from_fork+0x10/0x20
> 19:29:16 kernel: ---[ end trace 0000000000000000 ]---

However the rest of the first boot proceeded fine and I was able to use the system normally.

---

The second boot, I got the libata-scsi warning in the exact same spot, right at the beginning of fsck. Same call trace.

> bcachefs: accounting_read... done (0 seconds)
> WARNING: drivers/ata/libata-scsi.c:1682 at ata_scsi_deferred_qc_work+0x88/0x98

Fsck completed without issue. The filesystem was mounted.
Then this smartd error log showed up for the first (and only) time.

> smartd[1045]: Device: /dev/sdd [SAT], no ATA CHECK POWER STATUS support, ignoring -n Directive

Then, some power issue(?) caused the system to go read-only and I can't debug
anything because I can't execute any programs. This has happened in the past a
few times. The system crashes after a few minutes in this state. No logs are
available because the log system fails. Unsure if its related to these patches
but seems to happen when I build my own kernels. For these CBS tests I'm using the
unpatched fdt from u-boot v2026.04.
My root/boot filesystems are on an nvme drive, not the SATA PMP.

--- 

Now the third boot with CBS.

We make it to fsck without issue.
Fsck begins, but its running very slowly. Normally a fsck only takes 9-11 mins.
The libata-scsi warning appears, but after a few mins into the fsck this time.
Eventually, a recurring error which appears to reset the PMP links.
And SRCU lock warnings appear which I haven't seen since linux 6.19.

smartd times out on the third drive:

> 19:44:05 smartd[1051]: Device: /dev/sdc, type changed from 'scsi' to 'sat'
> 19:44:05 smartd[1051]: Device: /dev/sdc [SAT], opened
> 19:44:05 smartd[1051]: Device: /dev/sdc [SAT], Hitachi HTS547575A9E384 ...
> 19:44:05 smartd[1051]: Device: /dev/sdc [SAT], found in smartd database 7.5/6028: Hitachi/HGST Travelstar 5K750
> 19:45:09 smartd[1051]: Device: /dev/sdc [SAT], not capable of SMART Health Status check
> 19:45:37 systemd[1]: smartd.service: start operation timed out. Terminating.
> 19:46:11 systemd[1]: smartd.service: Failed with result 'timeout'.
> 19:46:11 systemd[1]: Failed to start smartd.service - Self Monitoring and Reporting Technology (SMART) Daemon.

fsck begins:

> 19:44:05 kernel: bcachefs : recovering from unclean shutdown
> 19:44:05 kernel: bcachefs : starting journal read
> 19:47:58 kernel: bcachefs : check_allocations...
> 19:51:06 kernel:  done (191 seconds)
> 19:51:08 kernel: bcachefs : going read-write
> 19:52:09 kernel: ------------[ cut here ]------------
> 19:52:09 kernel: WARNING: drivers/ata/libata-scsi.c:1682 at ata_scsi_deferred_qc_work+0x88/0x98, CPU#0: kworker/0:1H/601
...
> 20:02:51 kernel: bcachefs : check_extents_to_backpointers...
> 20:02:51 kernel: ata1.00: failed to read SCR 1 (Emask=0x40)
> 20:02:51 kernel: ata1.01: failed to read SCR 1 (Emask=0x40)
> 20:02:51 kernel: ata1.02: failed to read SCR 1 (Emask=0x40)
> 20:02:51 kernel: ata1.03: failed to read SCR 1 (Emask=0x40)
> 20:02:51 kernel: ata1.04: failed to read SCR 1 (Emask=0x40)
> 20:02:51 kernel: ata1.04: exception Emask 0x2 SAct 0x0 SErr 0x0 action 0x6 frozen
> 20:02:51 kernel: ata1.04: incorrect PMP
> 20:02:51 kernel: ata1.04: failed command: DATA SET MANAGEMENT
> 20:02:51 kernel: ata1.04: cmd 06/01:01:00:00:00/00:00:00:00:00/a0 tag 16 dma 512 out
>                                         res 40/00:00:f0:67:fb/00:00:00:00:00/40 Emask 0x2 (HSM violation)
> 20:02:51 kernel: ata1.04: status: { DRDY }
> 20:02:52 kernel: ata1.15: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
> 20:02:52 kernel: ata1.00: SATA link up 3.0 Gbps (SStatus 123 SControl 330)
> 20:02:53 kernel: ata1.01: SATA link up 6.0 Gbps (SStatus 133 SControl 330)
> 20:02:53 kernel: ata1.02: SATA link up 3.0 Gbps (SStatus 123 SControl 330)
> 20:02:54 kernel: ata1.03: SATA link up 6.0 Gbps (SStatus 133 SControl 330)
> 20:02:54 kernel: ata1.04: hard resetting link
> 20:02:54 kernel: ata1.04: SATA link up 6.0 Gbps (SStatus 133 SControl 330)
> 20:02:54 kernel: ata1.00: configured for UDMA/133
> 20:02:54 kernel: ata1.01: configured for UDMA/133
> 20:02:54 kernel: ata1.02: configured for UDMA/133
> 20:02:54 kernel: ata1.03: configured for UDMA/133
> 20:02:54 kernel: ata1.04: configured for UDMA/133
> 20:02:54 kernel: ata1: EH complete
> 20:03:40 kernel:  done (49 seconds)
> 20:09:00 kernel: bcachefs : check_alloc_to_lru_refs...
> 20:09:25 kernel:  done (25 seconds)
> 20:14:39 kernel: ata1.00: failed to read SCR 1 (Emask=0x40)
> 20:14:39 kernel: ata1.01: failed to read SCR 1 (Emask=0x40)
> 20:14:39 kernel: ata1.02: failed to read SCR 1 (Emask=0x40)
> 20:14:39 kernel: ata1.03: failed to read SCR 1 (Emask=0x40)
> 20:14:39 kernel: ata1.04: failed to read SCR 1 (Emask=0x40)
> 20:14:39 kernel: ata1.03: exception Emask 0x100 SAct 0x0 SErr 0x0 action 0x6 frozen
> 20:14:39 kernel: ata1.03: failed command: FLUSH CACHE EXT
> 20:14:39 kernel: ata1.03: cmd ea/00:00:00:00:00/00:00:00:00:00/a0 tag 29
>                                         res 40/00:01:00:00:00/00:00:00:00:00/40 Emask 0x4 (timeout)
> 20:14:39 kernel: ata1.03: status: { DRDY }
> 20:14:39 kernel: ata1.15: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
> 20:14:40 kernel: ata1.00: SATA link up 3.0 Gbps (SStatus 123 SControl 330)
> 20:14:40 kernel: ata1.01: SATA link up 6.0 Gbps (SStatus 133 SControl 330)
> 20:14:41 kernel: ata1.02: SATA link up 3.0 Gbps (SStatus 123 SControl 330)
> 20:14:41 kernel: ata1.03: hard resetting link
> 20:14:41 kernel: ata1.03: SATA link up 6.0 Gbps (SStatus 133 SControl 330)
> 20:14:42 kernel: ata1.04: SATA link up 6.0 Gbps (SStatus 133 SControl 330)
> 20:14:42 kernel: ata1.00: configured for UDMA/133
> 20:14:42 kernel: ata1.01: configured for UDMA/133
> 20:14:42 kernel: ata1.02: configured for UDMA/133
> 20:14:42 kernel: ata1.03: configured for UDMA/133
> 20:14:42 kernel: ata1.03: retrying FLUSH 0xea Emask 0x4
> 20:14:42 kernel: ata1.04: configured for UDMA/133
> 20:14:42 kernel: ata1: EH complete
> 20:14:42 kernel: bcachefs : check_snapshot_trees... done (0 seconds)
> 20:15:43 kernel: bcachefs : check_snapshots... done (0 seconds)
> 20:15:43 kernel: bcachefs (dev/sda): error reading superblock: EBUSY
> 20:15:43 kernel: error starting filesystem: EBUSY
> 20:15:43 kernel: bcachefs: bch2_fs_get_tree() error: EBUSY
> 20:15:44 kernel: ata1.00: failed to read SCR 1 (Emask=0x40)
> 20:15:44 kernel: ata1.01: failed to read SCR 1 (Emask=0x40)
> 20:15:44 kernel: ata1.02: failed to read SCR 1 (Emask=0x40)
> 20:15:44 kernel: ata1.03: failed to read SCR 1 (Emask=0x40)
> 20:15:44 kernel: ata1.04: failed to read SCR 1 (Emask=0x40)
> 20:15:44 kernel: ata1.04: exception Emask 0x2 SAct 0x0 SErr 0x0 action 0x6 frozen
> 20:15:44 kernel: ata1.04: incorrect PMP
> 20:15:44 kernel: ata1.04: failed command: FLUSH CACHE EXT
> 20:15:44 kernel: ata1.04: cmd ea/00:00:00:00:00/00:00:00:00:00/a0 tag 24
>                                         res 40/00:c8:00:3c:f2/00:00:1b:00:00/40 Emask 0x2 (HSM violation)
> 20:15:44 kernel: ata1.04: status: { DRDY }
> 20:15:45 kernel: ata1.15: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
> 20:15:45 kernel: ata1.00: SATA link up 3.0 Gbps (SStatus 123 SControl 330)
> 20:15:46 kernel: ata1.01: SATA link up 6.0 Gbps (SStatus 133 SControl 330)
> 20:15:46 kernel: ata1.02: SATA link up 3.0 Gbps (SStatus 123 SControl 330)
> 20:15:46 kernel: ata1.03: SATA link up 6.0 Gbps (SStatus 133 SControl 330)
> 20:15:46 kernel: ata1.04: hard resetting link
> 20:15:47 kernel: ata1.04: SATA link up 6.0 Gbps (SStatus 133 SControl 330)
> 20:15:47 kernel: ata1.00: configured for UDMA/133
> 20:15:47 kernel: ata1.01: configured for UDMA/133
> 20:15:47 kernel: ata1.02: configured for UDMA/133
> 20:15:47 kernel: ata1.03: configured for UDMA/133
> 20:15:47 kernel: ata1.04: configured for UDMA/133
> 20:15:47 kernel: ata1.04: retrying FLUSH 0xea Emask 0x2
> 20:15:47 kernel: ata1.04: device reported invalid CHS sector 0
> 20:15:47 kernel: ata1: EH complete
> 20:16:50 kernel: ata1.00: failed to read SCR 1 (Emask=0x40)
> 20:16:50 kernel: ata1.01: failed to read SCR 1 (Emask=0x40)
> 20:16:50 kernel: ata1.02: failed to read SCR 1 (Emask=0x40)
> 20:16:50 kernel: ata1.03: failed to read SCR 1 (Emask=0x40)
> 20:16:50 kernel: ata1.04: failed to read SCR 1 (Emask=0x40)
> 20:16:50 kernel: ata1.15: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
> 20:16:51 kernel: ata1.00: SATA link up 3.0 Gbps (SStatus 123 SControl 330)
> 20:16:51 kernel: ata1.01: SATA link up 6.0 Gbps (SStatus 133 SControl 330)
> 20:16:52 kernel: ata1.02: SATA link up 3.0 Gbps (SStatus 123 SControl 330)
> 20:16:52 kernel: ata1.03: SATA link up 6.0 Gbps (SStatus 133 SControl 330)
> 20:16:53 kernel: ata1.04: SATA link up 6.0 Gbps (SStatus 133 SControl 330)
> 20:16:53 kernel: ata1.00: configured for UDMA/133
> 20:16:53 kernel: ata1.01: configured for UDMA/133
> 20:16:53 kernel: ata1.02: configured for UDMA/133
> 20:16:53 kernel: ata1.03: configured for UDMA/133
> 20:16:53 kernel: ata1.04: configured for UDMA/133
> 20:16:53 kernel: bcachefs : check_subvols... done (0 seconds)
> 20:16:54 kernel: bcachefs : check_subvol_children... done (0 seconds)
> 20:17:55 kernel: ata1: illegal qc_active transition (08000000->01800000)
> 20:17:55 kernel: ata1.00: failed to read SCR 1 (Emask=0x40)
> 20:17:55 kernel: ata1.01: failed to read SCR 1 (Emask=0x40)
> 20:17:55 kernel: ata1.02: failed to read SCR 1 (Emask=0x40)
> 20:17:55 kernel: ata1.03: failed to read SCR 1 (Emask=0x40)
> 20:17:55 kernel: ata1.04: failed to read SCR 1 (Emask=0x40)
> 20:17:55 kernel: ata1.04: exception Emask 0x100 SAct 0x8000000 SErr 0x0 action 0x6 frozen
> 20:17:55 kernel: ata1.04: failed command: WRITE FPDMA QUEUED
> 20:17:55 kernel: ata1.04: cmd 61/08:d8:b0:35:40/00:00:00:00:00/40 tag 27 ncq dma 4096 out
>                                         res 50/00:00:00:4f:c2/00:00:00:00:00/00 Emask 0x100 (unknown error)
> 20:17:55 kernel: ata1.04: status: { DRDY }
> 20:17:56 kernel: ata1.15: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
> 20:17:56 kernel: ata1.00: SATA link up 3.0 Gbps (SStatus 123 SControl 330)
> 20:17:57 kernel: ata1.01: SATA link up 6.0 Gbps (SStatus 133 SControl 330)
> 20:17:57 kernel: ata1.02: SATA link up 3.0 Gbps (SStatus 123 SControl 330)
> 20:17:58 kernel: ata1.03: SATA link up 6.0 Gbps (SStatus 133 SControl 330)
> 20:17:58 kernel: ata1.04: hard resetting link
> 20:17:58 kernel: ata1.04: SATA link up 6.0 Gbps (SStatus 133 SControl 330)
> 20:17:58 kernel: ata1.00: configured for UDMA/133
> 20:17:58 kernel: ata1.01: configured for UDMA/133
> 20:17:58 kernel: ata1.02: configured for UDMA/133
> 20:17:58 kernel: ata1.03: configured for UDMA/133
> 20:17:58 kernel: ata1.04: configured for UDMA/133
> 20:17:58 kernel: ata1: EH complete
> 20:18:30 kernel: ------------[ cut here ]------------
> 20:18:30 kernel: btree trans held srcu lock (delaying memory reclaim) for 31 seconds
>                                btree_node_write:
>                                journal_reclaim_finish:
>                                journal_reclaim_start:
>                                transaction_commit:
> 20:18:30 kernel: WARNING: src/fs/bcachefs/btree/locking.c:1055 at bch2_trans_unlock_long+0x258/0x270 [bcachefs], CPU#1: kworker/1:1/6027
> 20:18:30 kernel: Hardware name: hardkernel Hardkernel ODROID-M1/Hardkernel ODROID-M1, BIOS 2026.04 04/01/2026
> 20:18:30 kernel: Workqueue: bcachefs_write_ref bch2_do_discards_work [bcachefs]
> 20:18:30 kernel: pc : bch2_trans_unlock_long+0x258/0x270 [bcachefs]
> 20:18:30 kernel: lr : bch2_trans_unlock_long+0x258/0x270 [bcachefs]
> 20:18:30 kernel: Call trace:
> 20:18:30 kernel:  bch2_trans_unlock_long+0x258/0x270 [bcachefs] (P)
> 20:18:30 kernel:  bch2_trans_begin+0x488/0x5a8 [bcachefs]
> 20:18:30 kernel:  bch2_do_discards+0x1b8/0x7e8 [bcachefs]
> 20:18:30 kernel:  bch2_do_discards_work+0x28/0x50 [bcachefs]
> 20:18:30 kernel:  process_one_work+0x188/0x520
> 20:18:30 kernel:  worker_thread+0x194/0x2f8
> 20:18:30 kernel:  kthread+0x11c/0x140
> 20:18:30 kernel:  ret_from_fork+0x10/0x20
> 20:18:30 kernel: ---[ end trace 0000000000000000 ]---
> 20:19:01 kernel: ------------[ cut here ]------------
> 20:19:01 kernel: btree trans held srcu lock (delaying memory reclaim) for 30 seconds
> 20:19:01 kernel: WARNING: src/fs/bcachefs/btree/locking.c:1055 at bch2_trans_unlock_long+0x258/0x270 [bcachefs], CPU#1: kworker/1:1/6027

I get this same warning 6 times total, with the do_discards call trace.
Some more errors happen:

> 20:25:10 kernel: bcachefs : check_inodes...
> 20:25:10 kernel: bcachefs (dev/sda): error reading superblock: EBUSY
> 20:25:10 kernel: error starting filesystem: EBUSY
> 20:25:10 kernel: bcachefs: bch2_fs_get_tree() error: EBUSY
> 20:28:20 kernel: bcachefs (dev/sda): error reading superblock: EBUSY
> 20:28:20 kernel: error starting filesystem: EBUSY
> 20:28:20 kernel: bcachefs: bch2_fs_get_tree() error: EBUSY

Eventually we make it through the fsck.
I'm able to use the filesystem, watch a big video file.
But the 2nd video file I try to watch, it stalls out, and I get a warning.

> 20:38:21 kernel: btree trans held srcu lock (delaying memory reclaim) for 121 seconds
> 20:38:21 kernel: WARNING: src/fs/bcachefs/btree/locking.c:1055 at bch2_trans_unlock_long+0x258/0x270
> 20:38:21 kernel: Call trace:
> 20:38:21 kernel:  bch2_trans_unlock_long+0x258/0x270 [bcachefs] (P)
> 20:38:21 kernel:  bch2_trans_begin+0x488/0x5a8 [bcachefs]
> 20:38:21 kernel:  bchfs_read+0x18c/0x600 [bcachefs]
> 20:38:21 kernel:  bch2_readahead+0x2b0/0x4f0 [bcachefs]
> 20:38:21 kernel:  read_pages+0x70/0x2d8

After some minutes of loading I can watch the 2nd video.

---

With the v1 patchset, CBS was working fine. But perhaps surfacing the bug has
to do with how many / what type of commands are issued to the drives.

Thanks,
Tommy

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

[Tommy Kelly]

On 5/13/26 21:09, Tommy Kelly wrote:
>
> The second boot, I got the libata-scsi warning in the exact same spot, right at the beginning of fsck. Same call trace.
> 
>> bcachefs: accounting_read... done (0 seconds)
>> WARNING: drivers/ata/libata-scsi.c:1682 at ata_scsi_deferred_qc_work+0x88/0x98
> 
> Fsck completed without issue. The filesystem was mounted.
> Then this smartd error log showed up for the first (and only) time.
> 
>> smartd[1045]: Device: /dev/sdd [SAT], no ATA CHECK POWER STATUS support, ignoring -n Directive
> 
> Then, some power issue(?) caused the system to go read-only and I can't debug
> anything because I can't execute any programs. This has happened in the past a
> few times. The system crashes after a few minutes in this state. No logs are
> available because the log system fails. Unsure if its related to these patches
> but seems to happen when I build my own kernels. For these CBS tests I'm using the
> unpatched fdt from u-boot v2026.04.
> My root/boot filesystems are on an nvme drive, not the SATA PMP.
Hi again,

Quick followup.
I reinstalled u-boot + devicetree FBS patch, booted into Linux v7.1-rc3 + v4 patchset.
And I got the full system failure again. Unable to run any commands.
My root filesystem is btrfs on NVMe.
It happened 6 minutes after fsck completed and the system was running normally without any error logs.
I went and plugged in the UART cable and got a few logs this time from the serial terminal. 

> U-Boot 2026.07-rc2-dirty
> kernel: Linux version 7.1.0-0.rc3.1d5dcaa3bd65.25.test.fc44.aarch64
> [ 1539.481926] BTRFS error (device nvme0n1p4 state EA): run_delalloc_nocow failed, root=5 inode=103820306 start=274432 len=4096 cur_offset=274432 oe_cleanup=274432 oe_cleanup_len=0 untouched_start=274432 untouched_len=4096: -5
> [ 1539.483785] BTRFS error (device nvme0n1p4 state EA): failed to run delalloc range, root=5 ino=103820306 folio=274432 submit_bitmap=0 start=274432 len=4096: -5
> [ 1596.136409] systemd-journald[800]: Failed to rotate /var/log/journal/c5b05402ed674946a0b721d8b2c649d1/system.journal: Read-only file system (Dropped 122276 similar message(s))

Those 2 `BTRFS error` lines repeated over and over every couple seconds.

This one seems to happen randomly, unable to reproduce consistently.
No idea why its happening.
But it has happened with both patched and un-patched u-boot devicetree.

Thanks,
Tommy

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

[sashiko-bot]

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

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

[Niklas Cassel]

On Thu, May 14, 2026 at 06:19:52AM +0000, sashiko-bot@kernel.org wrote:
> 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.

Thank you Sashiko,

you are absolutely correct.

ATA_DEFER_LINK_PMP_CBS did correctly use "goto free_qc" in V3:
https://lore.kernel.org/linux-ide/20260508195051.188207-5-cassel@kernel.org/

I made a mistake when refactoring it and renaming it to ATA_DEFER_LINK_EXCL
in V4.

Will need to send out a V5.


Kind regards,
Niklas

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

[Niklas Cassel]

Hello Tommy,

On Wed, May 13, 2026 at 09:42:35PM -0700, Tommy Kelly wrote:
> 
> > U-Boot 2026.07-rc2-dirty
> > kernel: Linux version 7.1.0-0.rc3.1d5dcaa3bd65.25.test.fc44.aarch64
> > [ 1539.481926] BTRFS error (device nvme0n1p4 state EA): run_delalloc_nocow failed, root=5 inode=103820306 start=274432 len=4096 cur_offset=274432 oe_cleanup=274432 oe_cleanup_len=0 untouched_start=274432 untouched_len=4096: -5
> > [ 1539.483785] BTRFS error (device nvme0n1p4 state EA): failed to run delalloc range, root=5 ino=103820306 folio=274432 submit_bitmap=0 start=274432 len=4096: -5
> > [ 1596.136409] systemd-journald[800]: Failed to rotate /var/log/journal/c5b05402ed674946a0b721d8b2c649d1/system.journal: Read-only file system (Dropped 122276 similar message(s))
> 
> Those 2 `BTRFS error` lines repeated over and over every couple seconds.
> 
> This one seems to happen randomly, unable to reproduce consistently.
> No idea why its happening.
> But it has happened with both patched and un-patched u-boot devicetree.

Thanks a lot for testing!

The problem with PMP CBS is my fault, I made a mistake when creating V4.

The btrfs bug seems to be unrelated, even though it seems very bad.

It is good that you have UART so you could see what was going on.

Perhaps you could test on v7.0 (hopefully btrfs is more stable here) +
v5 of this patch series (once I've sent it out).

I'm sorry for my mistake... hopefully it will not go beyond v5.


Kind regards,
Niklas

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

[Tommy Kelly]

On 5/13/26 23:42, Niklas Cassel wrote:
> Hello Tommy,
> 
> On Wed, May 13, 2026 at 09:42:35PM -0700, Tommy Kelly wrote:
>>
>>> U-Boot 2026.07-rc2-dirty
>>> kernel: Linux version 7.1.0-0.rc3.1d5dcaa3bd65.25.test.fc44.aarch64
>>> [ 1539.481926] BTRFS error (device nvme0n1p4 state EA): run_delalloc_nocow failed, root=5 inode=103820306 start=274432 len=4096 cur_offset=274432 oe_cleanup=274432 oe_cleanup_len=0 untouched_start=274432 untouched_len=4096: -5
>>> [ 1539.483785] BTRFS error (device nvme0n1p4 state EA): failed to run delalloc range, root=5 ino=103820306 folio=274432 submit_bitmap=0 start=274432 len=4096: -5
>>> [ 1596.136409] systemd-journald[800]: Failed to rotate /var/log/journal/c5b05402ed674946a0b721d8b2c649d1/system.journal: Read-only file system (Dropped 122276 similar message(s))
>>
>> Those 2 `BTRFS error` lines repeated over and over every couple seconds.
>>
>> This one seems to happen randomly, unable to reproduce consistently.
>> No idea why its happening.
>> But it has happened with both patched and un-patched u-boot devicetree.
> 
> Thanks a lot for testing!
> 
> The problem with PMP CBS is my fault, I made a mistake when creating V4.
> 
> The btrfs bug seems to be unrelated, even though it seems very bad.
> 
> It is good that you have UART so you could see what was going on.
> 
> Perhaps you could test on v7.0 (hopefully btrfs is more stable here) +
> v5 of this patch series (once I've sent it out).
> 
> I'm sorry for my mistake... hopefully it will not go beyond v5.
> 
> 
> Kind regards,
> Niklas

Hi Niklas,

I'll test v5 with CBS + FBS, no problem.

I just discovered SysRq + R (Replay the kernel log messages).
I will try that over serial next time it happens.

Thanks,
Tommy

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

[Niklas Cassel]

On Wed, May 13, 2026 at 09:09:35PM -0700, Tommy Kelly wrote:
> 
> Then, some power issue(?) caused the system to go read-only and I can't debug
> anything because I can't execute any programs. This has happened in the past a
> few times. The system crashes after a few minutes in this state. No logs are
> available because the log system fails. Unsure if its related to these patches
> but seems to happen when I build my own kernels. For these CBS tests I'm using the
> unpatched fdt from u-boot v2026.04.
> My root/boot filesystems are on an nvme drive, not the SATA PMP.

FWIW, if you want to build using the same kernel config as your distro,
you can do:

$ cp /boot/config-$(uname -r) .config
$ make olddefconfig

or

$ zcat /proc/config.gz > .config
$ make olddefconfig


Kind regards,
Niklas

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Hardware exclusivity violation and race condition due to removed port-wide barrier.
- [High] Cross-link command starvation due to per-link deferred_qc wakeups.
--

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

ata: libata-scsi: do not needlessly defer commands when using PMP with FBS

This patch moves deferred_qc from struct ata_port to struct ata_link.
This allows mixing non-NCQ and NCQ commands on the same port across
different links when using Port Multipliers that support FIS-Based Switching.

> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 02f2d7416c839..bd09d48d718da 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
[ ... ]
> @@ -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);
>  	}

Could this lead to a hardware exclusivity violation?

Before this patch, ap->deferred_qc acted as a port-wide barrier. Now that
it's scoped to the link, it allows other links to issue commands while one
link has a deferred command pending.

If Link A defers a non-NCQ command, its workqueue gets scheduled. In the
window before the workqueue executes, Link B can receive and issue a command
since linkB->deferred_qc is NULL.

When Link A's workqueue runs, it evaluates the WARN_ON_ONCE above. Because
Link B is now active, qc_defer() would correctly flag a conflict and return
non-zero. However, the WARN_ON_ONCE does not abort execution, so the code
proceeds to unconditionally call ata_qc_issue(qc). This would issue Link A's
non-NCQ command while Link B's command is already active.

[ ... ]
> @@ -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);
>  }

Can this cause cross-link command starvation for deferred commands?

In ata_scsi_qc_issue(), if a CBS PMP link is blocked because another link
holds exclusive port access, it explicitly stores the command in
linkA->deferred_qc. When the active link (Link B) completes its commands,
ata_scsi_qc_complete() now only evaluates the completing link (Link B).

Since Link A's deferred_qc is completely ignored during Link B's completion,
and Link A has no active commands to trigger its own completion, will its
deferred command be permanently stranded, eventually causing a SCSI timeout?

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

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

[Niklas Cassel]

On Thu, May 14, 2026 at 10:05:29AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Critical] Hardware exclusivity violation and race condition due to removed port-wide barrier.
> - [High] Cross-link command starvation due to per-link deferred_qc wakeups.
> --
> 
> commit ec58a121e5df098567314ab23ec810f87424e7ab
> Author: Niklas Cassel <cassel@kernel.org>
> 
> ata: libata-scsi: do not needlessly defer commands when using PMP with FBS
> 
> This patch moves deferred_qc from struct ata_port to struct ata_link.
> This allows mixing non-NCQ and NCQ commands on the same port across
> different links when using Port Multipliers that support FIS-Based Switching.
> 
> > diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> > index 02f2d7416c839..bd09d48d718da 100644
> > --- a/drivers/ata/libata-scsi.c
> > +++ b/drivers/ata/libata-scsi.c
> [ ... ]
> > @@ -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);
> >  	}
> 
> Could this lead to a hardware exclusivity violation?
> 
> Before this patch, ap->deferred_qc acted as a port-wide barrier. Now that
> it's scoped to the link, it allows other links to issue commands while one
> link has a deferred command pending.
> 
> If Link A defers a non-NCQ command, its workqueue gets scheduled. In the
> window before the workqueue executes, Link B can receive and issue a command
> since linkB->deferred_qc is NULL.
> 
> When Link A's workqueue runs, it evaluates the WARN_ON_ONCE above. Because
> Link B is now active, qc_defer() would correctly flag a conflict and return
> non-zero. However, the WARN_ON_ONCE does not abort execution, so the code
> proceeds to unconditionally call ata_qc_issue(qc). This would issue Link A's
> non-NCQ command while Link B's command is already active.

Thank you Sashiko, but being able to issue commands to multiple links at
the same time is the whole point of this series.

With PMP FBS, it is allowed to mix NCQ and non-NCQ to *different drives*.

Mixing NCQ and non-NCQ commands to the *the same drive* will still not
be allowed.

So this change in behavior is very much intended.


> 
> [ ... ]
> > @@ -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);
> >  }
> 
> Can this cause cross-link command starvation for deferred commands?
> 
> In ata_scsi_qc_issue(), if a CBS PMP link is blocked because another link
> holds exclusive port access, it explicitly stores the command in
> linkA->deferred_qc. When the active link (Link B) completes its commands,
> ata_scsi_qc_complete() now only evaluates the completing link (Link B).
> 
> Since Link A's deferred_qc is completely ignored during Link B's completion,
> and Link A has no active commands to trigger its own completion, will its
> deferred command be permanently stranded, eventually causing a SCSI timeout?

This review comment is correct, but this was because of a mistake in
V4 which accidentally did "goto defer_qc" instead of "goto free_qc".

This is fixed in V5. In V5 PMP CBS will never make use of the deferred QC
issuing via workqueue feature, so this review command can be disregarded.


Kind regards,
Niklas