ata: fix deferred QC handling for port multipliers
Niklas Cassel
maintainerv3 cover ata: fix deferred QC handling for port multipliers
Hello all,
Tommy Kelly reported a regression with PMP that use CBS:
https://lore.kernel.org/linux-ide/[email protected]/
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 1/2.
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 2/2.
Changes since v1:
-Avoid a local variable in sata_pmp_qc_defer_cmd_switch(), call
ata_std_qc_defer(qc) inside the switch() directly instead.
-Fix a typo, such that the the code actually compiles... Sorry for that.
Niklas Cassel (2):
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 | 26 +++++++++-- drivers/ata/libata-scsi.c | 95 ++++++++++++++++++++++++--------------- include/linux/libata.h | 8 ++-- 5 files changed, 101 insertions(+), 52 deletions(-)
Signature · 1 line
2.54.0
Niklas Cassel
maintainerPATCH v3 1/2 ata: libata-scsi: do not use the deferred QC feature on PMPs with CBS
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.
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() code to return ATA_DEFER_PORT_PMP_CBS and ATA_DEFER_LINK_PMP_CBS, and make sure that the deferred QC handling via workqueue is not used for these return values.
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.
--- drivers/ata/libata-pmp.c | 21 ++++++++++++-- drivers/ata/libata-scsi.c | 58 ++++++++++++++++++++++++--------------- include/linux/libata.h | 2 ++ 3 files changed, 57 insertions(+), 24 deletions(-)
drivers/ata/libata-pmp.c +19 -2
@@ -114,13 +114,30 @@ 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)) { qc->flags |=ATA_QCFLAG_CLEAR_EXCL ; -return ata_std_qc_defer (qc); +switch (ata_std_qc_defer (qc)) { +case 0 : +return 0 ; +case ATA_DEFER_LINK : +return ATA_DEFER_LINK_PMP_CBS ; +case ATA_DEFER_PORT : +return ATA_DEFER_PORT_PMP_CBS ; +default : +WARN_ON_ONCE (1 ); +return ATA_DEFER_PORT_PMP_CBS ; + } } +/* +* 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; } -return ATA_DEFER_PORT ; +return ATA_DEFER_PORT_PMP_CBS ; }/**
drivers/ata/libata-scsi.c +36 -22
81 lines not on this page · raw message
include/linux/libata.h +2 -0
@@ -371,6 +371,8 @@ enum {/* return values for ->qc_defer */ ATA_DEFER_LINK =1 ,ATA_DEFER_PORT =2 , +ATA_DEFER_LINK_PMP_CBS =3 , +ATA_DEFER_PORT_PMP_CBS =4 ,/* desc_len for ata_eh_info and context */ ATA_EH_DESC_LEN = 80,
Signature · 1 line
2.54.0
Niklas Cassel
maintainerPATCH v3 2/2 ata: libata-scsi: do not needlessly defer commands when using PMP with FBS
In reply to Niklas CasselThe 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.
--- drivers/ata/libata-core.c | 16 +++++++++++----- drivers/ata/libata-eh.c | 8 ++++---- drivers/ata/libata-pmp.c | 5 ++++- drivers/ata/libata-scsi.c | 39 +++++++++++++++++++++++---------------- include/linux/libata.h | 6 +++--- 5 files changed, 45 insertions(+), 29 deletions(-)
drivers/ata/libata-core.c +11 -5
@@ -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 */
drivers/ata/libata-eh.c +4 -4
@@ -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 ) {
drivers/ata/libata-pmp.c +4 -1
@@ -588,8 +588,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 ;
drivers/ata/libata-scsi.c +23 -16
@@ -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 ; }
@@ -1819,7 +1826,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 ; }
include/linux/libata.h +3 -3
@@ -856,6 +856,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 */
@@ -901,9 +904,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() */
Signature · 1 line
2.54.0
Re: [PATCH v3 2/2] ata: libata-scsi: do not needlessly defer commands when using PMP with FBS
Quoted 12 lines · 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.
Quoted 8 lines
Therefore, move the
deferred_qcfrom structata_portto structata_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 <[email protected]>
Looks OK. However Tommy did see a warning, so I wonder if we still have something missing...
Nevertheless,
Signature · 2 lines
Damien Le Moal Western Digital Research
Re: [PATCH v3 2/2] ata: libata-scsi: do not needlessly defer commands when using PMP with FBS
Quoted 2 lines · Damien Le Moal
Looks OK. However Tommy did see a warning, so I wonder if we still have something missing...
Sorry if I'm causing confusion, for clarity:
I didn't see the warning with this v3 patchset applied.
I saw a warning with vanilla linux 6.19.12.
I just recently got FBS working so I was reporting any issues with old code + FBS for completeness.
With this v3 patchset + FBS, no issues or warnings.
Best,
Tommy
Damien Le Moal
maintainerRe: [PATCH v3 2/2] ata: libata-scsi: do not needlessly defer commands when using PMP with FBS
Quoted 9 lines · Tommy Kelly
Quoted 2 lines · Damien Le Moal
Looks OK. However Tommy did see a warning, so I wonder if we still have something missing...
Sorry if I'm causing confusion, for clarity:
I didn't see the warning with this v3 patchset applied.
I saw a warning with vanilla linux 6.19.12.
I just recently got FBS working so I was reporting any issues with old code + FBS for completeness.With this v3 patchset + FBS, no issues or warnings.
Thank you for clarifying. It would be interesting to test this v3 but without the FBS support. Could you try please ?
Quoted 3 lines
Best,
Tommy
Signature · 2 lines
Damien Le Moal Western Digital Research
Quoted 29 lines · Niklas Cassel
Hello all,
Tommy Kelly reported a regression with PMP that use CBS:
https://lore.kernel.org/linux-ide/[email protected]/It turns out that the
ap->excl_linklogic used for PMP that use CBS is incompatible with the deferred qc issuing via a workqueue. This is fixed in patch 1/2.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_portto structata_link. This is fixed in patch 2/2.Changes since v1:
(quoted diff) +0
-2-Avoid a local variable in sata_pmp_qc_defer_cmd_switch(), call ata_std_qc_defer(qc) inside the switch() directly instead. -Fix a typo, such that the the code actually compiles... Sorry for that.Niklas Cassel (2):
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
Quoted 7 lines
drivers/ata/libata-core.c | 16 ++++--- drivers/ata/libata-eh.c | 8 ++-- drivers/ata/libata-pmp.c | 26 +++++++++-- drivers/ata/libata-scsi.c | 95 ++++++++++++++++++++++++--------------- include/linux/libata.h | 8 ++-- 5 files changed, 101 insertions(+), 52 deletions(-)
Hi Niklas,
Interestingly, I got my first warning with the v6.19.12 code.
I run fsck on the PMP drives on boot and during the "check_alloc_info" part, I got
Quoted 18 lines
WARNING: drivers/ata/libata-scsi.c:1677 at ata_scsi_deferred_qc_work+0x84/0x98 CPU: 0 UID: 0 PID: 157 Comm: kworker/0:1H Tainted: G OE 6.19.12-200.fc43.aarch64 #1 PREEMPT(voluntary) Hardware name: hardkernel Hardkernel ODROID-M1/Hardkernel ODROID-M1, BIOS 2026.07-rc1-dirty 07/01/2026 Workqueue: events_highpri ata_scsi_deferred_qc_work pc : ata_scsi_deferred_qc_work+0x84/0x98 lr : ata_scsi_deferred_qc_work+0x50/0x98 Call trace: ata_scsi_deferred_qc_work+0x84/0x98 (P) process_one_work+0x17c/0x400 worker_thread+0x19c/0x328 kthread+0x10c/0x128 ret_from_fork+0x10/0x20 WARNING: drivers/ata/libata-core.c:5143 at ata_qc_issue+0x34/0x420, CPU#0: kworker/0:1H/157 CPU: 0 UID: 0 PID: 157 Comm: kworker/0:1H Tainted: G W OE 6.19.12-200.fc43.aarch64 #1 PREEMPT(voluntary) Hardware name: hardkernel Hardkernel ODROID-M1/Hardkernel ODROID-M1, BIOS 2026.07-rc1-dirty 07/01/2026 Workqueue: events_highpri ata_scsi_deferred_qc_work pc : ata_qc_issue+0x34/0x420
Quoted 8 lines
lr : ata_scsi_deferred_qc_work+0x68/0x98 Call trace: ata_qc_issue+0x34/0x420 (P) ata_scsi_deferred_qc_work+0x68/0x98 process_one_work+0x17c/0x400 worker_thread+0x19c/0x328 kthread+0x10c/0x128 ret_from_fork+0x10/0x20
Everything else on that boot seemed fine. No crashes or usability issues.
These are the lines that threw warnings:
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/ata/libata-scsi.c?h=v6.19.12#n1677
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/ata/libata-core.c?h=v6.19.12#n5143
I haven't been able to reproduce it though, it only happened once.
And now for the latest code: To build, I ran
Quoted 6 lines
git checkout -b fedora-7.0-libata kernel-ark/fedora-7.0 # linux v7.0.4 git merge libata/for-7.1
git cherry-pick --empty=drop v7.1-rc1..libata/for-7.1-fixes git cherry-pick --empty=drop v7.1-rc1..libata/for-next
git cherry-pick5918bf2a17f4689abfb94d341c5240088f8bd16e# FBS support b4 shazam [email protected] # v3 patch series
And the boot worked fine. No observable issues.
I haven't done any perf testing. If you'd instruct me how, I could run perf tests for you. Maybe not torture tests though because I need the drives to survive. :)
Thank you,
- Tommy
Damien Le Moal
maintainerQuoted 9 lines · Niklas Cassel
Hello all,
Tommy Kelly reported a regression with PMP that use CBS:
https://lore.kernel.org/linux-ide/[email protected]/It turns out that the
ap->excl_linklogic used for PMP that use CBS is incompatible with the deferred qc issuing via a workqueue. This is fixed in patch 1/2.
Quoted 9 lines
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_portto structata_link. This is fixed in patch 2/2.
What are the changes from v2 ?
Quoted 18 lines
Changes since v1:
(quoted diff) +0
-2-Avoid a local variable in sata_pmp_qc_defer_cmd_switch(), call ata_std_qc_defer(qc) inside the switch() directly instead. -Fix a typo, such that the the code actually compiles... Sorry for that.Niklas Cassel (2):
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
FBSdrivers/ata/libata-core.c | 16 ++++--- drivers/ata/libata-eh.c | 8 ++-- drivers/ata/libata-pmp.c | 26 +++++++++-- drivers/ata/libata-scsi.c | 95 ++++++++++++++++++++++++--------------- include/linux/libata.h | 8 ++-- 5 files changed, 101 insertions(+), 52 deletions(-)
Signature · 2 lines
Damien Le Moal Western Digital Research
Niklas Cassel
maintainerQuoted 20 lines · Damien Le Moal
Quoted 9 lines · Niklas Cassel
Hello all,
Tommy Kelly reported a regression with PMP that use CBS:
https://lore.kernel.org/linux-ide/[email protected]/It turns out that the
ap->excl_linklogic used for PMP that use CBS is incompatible with the deferred qc issuing via a workqueue. This is fixed in patch 1/2.Quoted 9 lines
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_portto structata_link. This is fixed in patch 2/2.
Quoted 1 line
What are the changes from v2 ?
There are the changes since v2, I simply forgot to change v1 to v2.
Quoted 24 lines
Quoted 19 lines
Changes since v1:
(quoted diff) +0
-2-Avoid a local variable in sata_pmp_qc_defer_cmd_switch(), call ata_std_qc_defer(qc) inside the switch() directly instead. -Fix a typo, such that the the code actually compiles... Sorry for that.Niklas Cassel (2):
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
FBSdrivers/ata/libata-core.c | 16 ++++--- drivers/ata/libata-eh.c | 8 ++-- drivers/ata/libata-pmp.c | 26 +++++++++-- drivers/ata/libata-scsi.c | 95 ++++++++++++++++++++++++--------------- include/linux/libata.h | 8 ++-- 5 files changed, 101 insertions(+), 52 deletions(-)Signature · 2 lines
Damien Le Moal Western Digital Research