superrob1500
/
mirror-linux
mirror of https://github.com/torvalds/linux
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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

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

Commit 0ea84089db ("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. This is in line with how PMPs with FBS worked before
commit 0ea84089db ("ata: libata-scsi: avoid Non-NCQ command starvation").

Fixes: 0ea84089db ("ata: libata-scsi: avoid Non-NCQ command starvation")
Tested-by: Tommy Kelly <linux@tkel.ly>
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Signed-off-by: Niklas Cassel <cassel@kernel.org>
master
Niklas Cassel 2026-05-14 09:39:02 +02:00
parent f233124fb3
commit 759e8756da
5 changed files with 42 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
drivers/ata/libata-core.c
View File

@ -5584,6 +5584,7 @@ void ata_link_init(struct ata_port *ap, struct ata_link *link, int pmp)
link->pmp = pmp; link->pmp = pmp;
link->active_tag = ATA_TAG_POISON; link->active_tag = ATA_TAG_POISON;
link->hw_sata_spd_limit = UINT_MAX; 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 */ /* can't use iterator, ap isn't initialized yet */
for (i = 0; i < ATA_MAX_DEVICES; i++) { 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); mutex_init(&ap->scsi_scan_mutex);
INIT_DELAYED_WORK(&ap->hotplug_task, ata_scsi_hotplug); INIT_DELAYED_WORK(&ap->hotplug_task, ata_scsi_hotplug);
INIT_DELAYED_WORK(&ap->scsi_rescan_task, ata_scsi_dev_rescan); 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_LIST_HEAD(&ap->eh_done_q);
init_waitqueue_head(&ap->eh_wait_q); init_waitqueue_head(&ap->eh_wait_q);
init_completion(&ap->park_req_pending); 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. */ /* It better be dead now and not have any remaining deferred qc. */
WARN_ON(!(ap->pflags & ATA_PFLAG_UNLOADED)); 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->hotplug_task);
cancel_delayed_work_sync(&ap->scsi_rescan_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 */ /* Delete port multiplier link transport devices */
if (ap->pmp_link) { if (ap->pmp_link) {
int i; int i;

8
drivers/ata/libata-eh.c
View File

@ -651,11 +651,11 @@ int ata_scsi_cmd_error_handler(struct Scsi_Host *host, struct ata_port *ap,
if (qc->scsicmd != scmd) if (qc->scsicmd != scmd)
continue; continue;
if ((qc->flags & ATA_QCFLAG_ACTIVE) || if ((qc->flags & ATA_QCFLAG_ACTIVE) ||
qc == ap->deferred_qc) qc == qc->dev->link->deferred_qc)
break; 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 * This is a deferred command that timed out while
* waiting for the command queue to drain. Since the qc * 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. * deferred qc work from issuing this qc.
*/ */
WARN_ON_ONCE(qc->flags & ATA_QCFLAG_ACTIVE); WARN_ON_ONCE(qc->flags & ATA_QCFLAG_ACTIVE);
ap->deferred_qc = NULL; qc->dev->link->deferred_qc = NULL;
cancel_work(&ap->deferred_qc_work); cancel_work(&qc->dev->link->deferred_qc_work);
set_host_byte(scmd, DID_TIME_OUT); set_host_byte(scmd, DID_TIME_OUT);
scsi_eh_finish_cmd(scmd, &ap->eh_done_q); scsi_eh_finish_cmd(scmd, &ap->eh_done_q);
} else if (i < ATA_MAX_QUEUE) { } else if (i < ATA_MAX_QUEUE) {

5
drivers/ata/libata-pmp.c
View File

@ -582,8 +582,11 @@ static void sata_pmp_detach(struct ata_device *dev)
if (ap->ops->pmp_detach) if (ap->ops->pmp_detach)
ap->ops->pmp_detach(ap); 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); ata_eh_detach_dev(tlink->device);
}
spin_lock_irqsave(ap->lock, flags); spin_lock_irqsave(ap->lock, flags);
ap->nr_pmp_links = 0; ap->nr_pmp_links = 0;

43
drivers/ata/libata-scsi.c
View File

@ -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) void ata_scsi_deferred_qc_work(struct work_struct *work)
{ {
struct ata_port *ap = struct ata_link *link =
container_of(work, struct ata_port, deferred_qc_work); container_of(work, struct ata_link, deferred_qc_work);
struct ata_port *ap = link->ap;
struct ata_queued_cmd *qc; struct ata_queued_cmd *qc;
unsigned long flags; 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 * such case, we should not need any more deferring the qc, so warn if
* qc_defer() says otherwise. * qc_defer() says otherwise.
*/ */
qc = ap->deferred_qc; qc = link->deferred_qc;
if (qc && !ata_port_eh_scheduled(ap)) { if (qc && !ata_port_eh_scheduled(ap)) {
WARN_ON_ONCE(ap->ops->qc_defer(qc)); WARN_ON_ONCE(ap->ops->qc_defer(qc));
ap->deferred_qc = NULL; link->deferred_qc = NULL;
ata_qc_issue(qc); 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) 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); 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 * do not try to be smart about what to do with this deferred command
* and simply requeue it by completing it with DID_REQUEUE. * and simply requeue it by completing it with DID_REQUEUE.
*/ */
if (qc) { ata_for_each_link(link, ap, PMP_FIRST) {
ap->deferred_qc = NULL; struct ata_queued_cmd *qc = link->deferred_qc;
cancel_work(&ap->deferred_qc_work);
ata_scsi_qc_done(qc, true, DID_REQUEUE << 16); 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); lockdep_assert_held(ap->lock);
@ -1723,12 +1729,12 @@ static void ata_scsi_schedule_deferred_qc(struct ata_port *ap)
return; return;
} }
if (!ap->ops->qc_defer(qc)) 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) 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; struct scsi_cmnd *cmd = qc->scsicmd;
u8 *cdb = cmd->cmnd; u8 *cdb = cmd->cmnd;
bool have_sense = qc->flags & ATA_QCFLAG_SENSE_VALID; 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_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) static int ata_scsi_qc_issue(struct ata_port *ap, struct ata_queued_cmd *qc)
{ {
struct ata_link *link = qc->dev->link;
int ret; int ret;
if (!ap->ops->qc_defer) 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 * requeue and defer all incoming commands until the deferred qc is
* processed, once all on-going commands complete. * processed, once all on-going commands complete.
*/ */
if (ap->deferred_qc) { if (link->deferred_qc) {
ata_qc_free(qc); ata_qc_free(qc);
return SCSI_MLQUEUE_DEVICE_BUSY; 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: case ATA_DEFER_LINK_EXCL:
/* /*
* Drivers making use of ap->excl_link cannot store the QC in * Drivers making use of ap->excl_link cannot store the QC in
* ap->deferred_qc, because the ap->excl_link handling is * link->deferred_qc, because the ap->excl_link handling is
* incompatible with the ap->deferred_qc workqueue handling. * incompatible with the link->deferred_qc workqueue handling.
*/ */
ret = SCSI_MLQUEUE_DEVICE_BUSY; ret = SCSI_MLQUEUE_DEVICE_BUSY;
goto free_qc; goto free_qc;
@ -1817,7 +1824,7 @@ defer_qc:
* commands complete. * commands complete.
*/ */
if (!ata_is_ncq(qc->tf.protocol)) { if (!ata_is_ncq(qc->tf.protocol)) {
ap->deferred_qc = qc; link->deferred_qc = qc;
return 0; return 0;
} }

6
include/linux/libata.h
View File

@ -855,6 +855,9 @@ struct ata_link {
unsigned int sata_spd; /* current SATA PHY speed */ unsigned int sata_spd; /* current SATA PHY speed */
enum ata_lpm_policy lpm_policy; 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 */ /* record runtime error info, protected by host_set lock */
struct ata_eh_info eh_info; struct ata_eh_info eh_info;
/* EH context */ /* EH context */
@ -900,9 +903,6 @@ struct ata_port {
u64 qc_active; u64 qc_active;
int nr_active_links; /* #links with active qcs */ 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 link; /* host default link */
struct ata_link *slave_link; /* see ata_slave_link_init() */ struct ata_link *slave_link; /* see ata_slave_link_init() */