{"id":2231872,"url":"http://patchwork.ozlabs.org/api/1.2/patches/2231872/?format=json","web_url":"http://patchwork.ozlabs.org/project/linux-ide/patch/20260501125410.1204490-8-cassel@kernel.org/","project":{"id":13,"url":"http://patchwork.ozlabs.org/api/1.2/projects/13/?format=json","name":"Linux IDE development","link_name":"linux-ide","list_id":"linux-ide.vger.kernel.org","list_email":"linux-ide@vger.kernel.org","web_url":null,"scm_url":null,"webscm_url":null,"list_archive_url":"","list_archive_url_format":"","commit_url_format":""},"msgid":"<20260501125410.1204490-8-cassel@kernel.org>","list_archive_url":null,"date":"2026-05-01T12:54:13","name":"[3/3] ata: libata-scsi: do not needlessly defer commands when using PMP with FBS","commit_ref":null,"pull_url":null,"state":"new","archived":false,"hash":"3bb4e30ff581c61dab489f7af97713740d816514","submitter":{"id":87751,"url":"http://patchwork.ozlabs.org/api/1.2/people/87751/?format=json","name":"Niklas Cassel","email":"cassel@kernel.org"},"delegate":null,"mbox":"http://patchwork.ozlabs.org/project/linux-ide/patch/20260501125410.1204490-8-cassel@kernel.org/mbox/","series":[{"id":502457,"url":"http://patchwork.ozlabs.org/api/1.2/series/502457/?format=json","web_url":"http://patchwork.ozlabs.org/project/linux-ide/list/?series=502457","date":"2026-05-01T12:54:13","name":"ata: fix deferred QC handling for port multipliers","version":1,"mbox":"http://patchwork.ozlabs.org/series/502457/mbox/"}],"comments":"http://patchwork.ozlabs.org/api/patches/2231872/comments/","check":"pending","checks":"http://patchwork.ozlabs.org/api/patches/2231872/checks/","tags":{},"related":[],"headers":{"Return-Path":"\n ","X-Original-To":["incoming@patchwork.ozlabs.org","linux-ide@vger.kernel.org"],"Delivered-To":"patchwork-incoming@legolas.ozlabs.org","Authentication-Results":["legolas.ozlabs.org;\n\tdkim=pass (2048-bit key;\n unprotected) header.d=kernel.org header.i=@kernel.org header.a=rsa-sha256\n header.s=k20201202 header.b=umDFMyss;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=2600:3c09:e001:a7::12fc:5321; helo=sto.lore.kernel.org;\n envelope-from=linux-ide+bounces-5619-incoming=patchwork.ozlabs.org@vger.kernel.org;\n receiver=patchwork.ozlabs.org)","smtp.subspace.kernel.org;\n\tdkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org\n header.b=\"umDFMyss\"","smtp.subspace.kernel.org;\n arc=none smtp.client-ip=10.30.226.201"],"Received":["from sto.lore.kernel.org (sto.lore.kernel.org\n [IPv6:2600:3c09:e001:a7::12fc:5321])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange x25519 server-signature ECDSA (secp384r1) server-digest SHA384)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4g6WFy58cfz1yHZ\n\tfor ; Fri, 01 May 2026 22:54:30 +1000 (AEST)","from smtp.subspace.kernel.org (conduit.subspace.kernel.org\n [100.90.174.1])\n\tby sto.lore.kernel.org (Postfix) with ESMTP id 0BA33300AB29\n\tfor ; Fri, 1 May 2026 12:54:27 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id 743B43A5451;\n\tFri, 1 May 2026 12:54:25 +0000 (UTC)","from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org\n [10.30.226.201])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby smtp.subspace.kernel.org (Postfix) with ESMTPS id 519763A5420\n\tfor ; Fri, 1 May 2026 12:54:25 +0000 (UTC)","by smtp.kernel.org (Postfix) with ESMTPSA id 74799C2BCB8;\n\tFri, 1 May 2026 12:54:23 +0000 (UTC)"],"ARC-Seal":"i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1777640065; cv=none;\n b=RJQnvKhuIcFJekGNbVYb+Xztl2K4zGelLBEivBn+vUakO5+VtZl4XKuDm2fAvLDkrZmQFyOTkOYINgZ6iG6sKSMSR/nxzhhWzG5rzcGbquBHReWFlHFOAcnrPsreKfa1TDReRhKwftEIHJtEede8/c32P4D3Fg9ZOSwbY80uCoY=","ARC-Message-Signature":"i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1777640065; c=relaxed/simple;\n\tbh=j62k58F05RiR/yX9yyHmoEuIKRNt1X3OmKoxfTu/mx8=;\n\th=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References:\n\t MIME-Version;\n b=DL76O5BGLrII8qRjRK9bupMohJGA8ozs70z06q76OZkSc+rmqjLTcC5OuzNLKMl9UPgzjlVv+GlI2AvNZdJsZox61QVdg9mF34+AeYmr8A28BNKmNONRpeBEsVunONSS1rZo6PAjipehsnSL9ki3a0mzezLM5DGqNoxyqlos38E=","ARC-Authentication-Results":"i=1; smtp.subspace.kernel.org;\n dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org\n header.b=umDFMyss; arc=none smtp.client-ip=10.30.226.201","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org;\n\ts=k20201202; t=1777640065;\n\tbh=j62k58F05RiR/yX9yyHmoEuIKRNt1X3OmKoxfTu/mx8=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=umDFMyssGH8kXNiJHLuXsY9YceC+xLH331fJwrNYgSXUC22WLhxgvv+kR40Bp1eou\n\t 73GZQvXC8pyGp8u9rLQoiIxg3hI5SbQa6mBnexF703FD++WmA/T5gPDkx6QHjAY3WD\n\t d0re2HcgAO+yfsFsP2K/kXIM+ioQNH5O5eJbBOHvnLlLFYaVFH6wwFeuB3k+Noe0uB\n\t Ud5GVq5zi2DBudeVkgTaxmGcO5fHrPUGwZ+ONfwM64ogfjD7QiA+reGRUj4WsCKD/6\n\t qjXykiGMFJ/YYdyv9s1/enHLLxq3L7vfHQm58HjbZsnklOadvhKcjS3xSCxNr5MVKf\n\t N0HJYLeHKRplQ==","From":"Niklas Cassel ","To":"Tommy Kelly ,\n\tDamien Le Moal ,\n\tNiklas Cassel ,\n\tJohn Garry ,\n\t\"Martin K. Petersen\" ","Cc":"linux-ide@vger.kernel.org","Subject":"[PATCH 3/3] ata: libata-scsi: do not needlessly defer commands when\n using PMP with FBS","Date":"Fri, 1 May 2026 14:54:13 +0200","Message-ID":"<20260501125410.1204490-8-cassel@kernel.org>","X-Mailer":"git-send-email 2.54.0","In-Reply-To":"<20260501125410.1204490-5-cassel@kernel.org>","References":"<20260501125410.1204490-5-cassel@kernel.org>","Precedence":"bulk","X-Mailing-List":"linux-ide@vger.kernel.org","List-Id":"","List-Subscribe":"","List-Unsubscribe":"","MIME-Version":"1.0","X-Developer-Signature":"v=1; a=openpgp-sha256; l=9483; i=cassel@kernel.org;\n h=from:subject; bh=j62k58F05RiR/yX9yyHmoEuIKRNt1X3OmKoxfTu/mx8=;\n b=owGbwMvMwCV2MsVw8cxjvkWMp9WSGDK/LCopPhpW88v25IHVIR0LH/+7OlH19As/K5P/C17N9\n YyfutTpTEcpC4MYF4OsmCKL7w+X/cXd7lOOK96xgZnDygQyhIGLUwAmMl+LkaHH+PoHf3MVWe65\n 9jE/N2nEct9Z+aPHa4fUX44Ja6Y92qTB8E/zQnTmwbNHS3hPXd/ry5Bpd36Te8qEaxV5rsftNj7\n vPcAOAA==","X-Developer-Key":"i=cassel@kernel.org; a=openpgp;\n fpr=5ADE635C0E631CBBD5BE065A352FE6582ED9B5DA","Content-Transfer-Encoding":"8bit"},"content":"The SATA specification does not allow a non-NCQ command to be issued while\nan NCQ command is outstanding.\n\nCommit 0ea84089dbf6 (\"ata: libata-scsi: avoid Non-NCQ command starvation\")\nintroduced a feature where a deferred non-NCQ command gets issued from a\nworkqueue. The design stores a single non-NCQ command per port.\n\nHowever, when using Port Multipliers (PMPs), specifically PMPs that\nsupport FIS-Based Switching (FBS), non-NCQ and NCQ commands can be mixed\non the same port, just not for the same link, see e.g. ata_std_qc_defer()\nwhich is, and always has operated on a per-link basis.\n\nTherefore, move the deferred_qc from struct ata_port to struct ata_link.\nThis way, when using a PMP with FBS, we will not needlessly defer commands\nto all other links, just because one link issued a non-NCQ command while\nhaving an NCQ command outstanding. Only commands for that specific link\nwill be deferred.\n\nFixes: 0ea84089dbf6 (\"ata: libata-scsi: avoid Non-NCQ command starvation\")\nSigned-off-by: Niklas Cassel \n---\n drivers/ata/libata-core.c | 16 +++++++++++-----\n drivers/ata/libata-eh.c | 8 ++++----\n drivers/ata/libata-pmp.c | 4 +++-\n drivers/ata/libata-scsi.c | 39 +++++++++++++++++++++++----------------\n include/linux/libata.h | 6 +++---\n 5 files changed, 44 insertions(+), 29 deletions(-)","diff":"diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c\nindex e76d15411e2a..c3a10e85a19c 100644\n--- a/drivers/ata/libata-core.c\n+++ b/drivers/ata/libata-core.c\n@@ -5584,6 +5584,7 @@ void ata_link_init(struct ata_port *ap, struct ata_link *link, int pmp)\n \tlink->pmp = pmp;\n \tlink->active_tag = ATA_TAG_POISON;\n \tlink->hw_sata_spd_limit = UINT_MAX;\n+\tINIT_WORK(&link->deferred_qc_work, ata_scsi_deferred_qc_work);\n \n \t/* can't use iterator, ap isn't initialized yet */\n \tfor (i = 0; i < ATA_MAX_DEVICES; i++) {\n@@ -5666,7 +5667,6 @@ struct ata_port *ata_port_alloc(struct ata_host *host)\n \tmutex_init(&ap->scsi_scan_mutex);\n \tINIT_DELAYED_WORK(&ap->hotplug_task, ata_scsi_hotplug);\n \tINIT_DELAYED_WORK(&ap->scsi_rescan_task, ata_scsi_dev_rescan);\n-\tINIT_WORK(&ap->deferred_qc_work, ata_scsi_deferred_qc_work);\n \tINIT_LIST_HEAD(&ap->eh_done_q);\n \tinit_waitqueue_head(&ap->eh_wait_q);\n \tinit_completion(&ap->park_req_pending);\n@@ -6291,9 +6291,9 @@ static void ata_port_detach(struct ata_port *ap)\n \n \t/* It better be dead now and not have any remaining deferred qc. */\n \tWARN_ON(!(ap->pflags & ATA_PFLAG_UNLOADED));\n-\tWARN_ON(ap->deferred_qc);\n+\tWARN_ON(ap->link.deferred_qc);\n \n-\tcancel_work_sync(&ap->deferred_qc_work);\n+\tcancel_work_sync(&ap->link.deferred_qc_work);\n \tcancel_delayed_work_sync(&ap->hotplug_task);\n \tcancel_delayed_work_sync(&ap->scsi_rescan_task);\n \n@@ -6301,8 +6301,14 @@ static void ata_port_detach(struct ata_port *ap)\n \tif (ap->pmp_link) {\n \t\tint i;\n \n-\t\tfor (i = 0; i < SATA_PMP_MAX_PORTS; i++)\n-\t\t\tata_tlink_delete(&ap->pmp_link[i]);\n+\t\tfor (i = 0; i < SATA_PMP_MAX_PORTS; i++) {\n+\t\t\tstruct ata_link *pmp_link = &ap->pmp_link[i];\n+\n+\t\t\tWARN_ON(pmp_link->deferred_qc);\n+\t\t\tcancel_work_sync(&pmp_link->deferred_qc_work);\n+\n+\t\t\tata_tlink_delete(pmp_link);\n+\t\t}\n \t}\n \n \t/* Remove the associated SCSI host */\ndiff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c\nindex 9a4b67b90b17..d623eb32ed8b 100644\n--- a/drivers/ata/libata-eh.c\n+++ b/drivers/ata/libata-eh.c\n@@ -651,11 +651,11 @@ int ata_scsi_cmd_error_handler(struct Scsi_Host *host, struct ata_port *ap,\n \t\t\tif (qc->scsicmd != scmd)\n \t\t\t\tcontinue;\n \t\t\tif ((qc->flags & ATA_QCFLAG_ACTIVE) ||\n-\t\t\t qc == ap->deferred_qc)\n+\t\t\t qc == qc->dev->link->deferred_qc)\n \t\t\t\tbreak;\n \t\t}\n \n-\t\tif (i < ATA_MAX_QUEUE && qc == ap->deferred_qc) {\n+\t\tif (i < ATA_MAX_QUEUE && qc == qc->dev->link->deferred_qc) {\n \t\t\t/*\n \t\t\t * This is a deferred command that timed out while\n \t\t\t * waiting for the command queue to drain. Since the qc\n@@ -666,8 +666,8 @@ int ata_scsi_cmd_error_handler(struct Scsi_Host *host, struct ata_port *ap,\n \t\t\t * deferred qc work from issuing this qc.\n \t\t\t */\n \t\t\tWARN_ON_ONCE(qc->flags & ATA_QCFLAG_ACTIVE);\n-\t\t\tap->deferred_qc = NULL;\n-\t\t\tcancel_work(&ap->deferred_qc_work);\n+\t\t\tqc->dev->link->deferred_qc = NULL;\n+\t\t\tcancel_work(&qc->dev->link->deferred_qc_work);\n \t\t\tset_host_byte(scmd, DID_TIME_OUT);\n \t\t\tscsi_eh_finish_cmd(scmd, &ap->eh_done_q);\n \t\t} else if (i < ATA_MAX_QUEUE) {\ndiff --git a/drivers/ata/libata-pmp.c b/drivers/ata/libata-pmp.c\nindex 0575e1e283d8..b0c484a591b8 100644\n--- a/drivers/ata/libata-pmp.c\n+++ b/drivers/ata/libata-pmp.c\n@@ -574,8 +574,10 @@ static void sata_pmp_detach(struct ata_device *dev)\n \tif (ap->ops->pmp_detach)\n \t\tap->ops->pmp_detach(ap);\n \n-\tata_for_each_link(tlink, ap, EDGE)\n+\tata_for_each_link(tlink, ap, EDGE) {\n+\t\tcancel_work_sync(&tlink->deferred_qc_work);\n \t\tata_eh_detach_dev(tlink->device);\n+\t}\n \n \tspin_lock_irqsave(ap->lock, flags);\n \tap->nr_pmp_links = 0;\ndiff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c\nindex dca7ea7e6315..201dac46d0a3 100644\n--- a/drivers/ata/libata-scsi.c\n+++ b/drivers/ata/libata-scsi.c\n@@ -1664,8 +1664,9 @@ static void ata_scsi_qc_done(struct ata_queued_cmd *qc, bool set_result,\n \n void ata_scsi_deferred_qc_work(struct work_struct *work)\n {\n-\tstruct ata_port *ap =\n-\t\tcontainer_of(work, struct ata_port, deferred_qc_work);\n+\tstruct ata_link *link =\n+\t\tcontainer_of(work, struct ata_link, deferred_qc_work);\n+\tstruct ata_port *ap = link->ap;\n \tstruct ata_queued_cmd *qc;\n \tunsigned long flags;\n \n@@ -1676,10 +1677,10 @@ void ata_scsi_deferred_qc_work(struct work_struct *work)\n \t * such case, we should not need any more deferring the qc, so warn if\n \t * qc_defer() says otherwise.\n \t */\n-\tqc = ap->deferred_qc;\n+\tqc = link->deferred_qc;\n \tif (qc && !ata_port_eh_scheduled(ap)) {\n \t\tWARN_ON_ONCE(ap->ops->qc_defer(qc));\n-\t\tap->deferred_qc = NULL;\n+\t\tlink->deferred_qc = NULL;\n \t\tata_qc_issue(qc);\n \t}\n \n@@ -1688,7 +1689,7 @@ void ata_scsi_deferred_qc_work(struct work_struct *work)\n \n void ata_scsi_requeue_deferred_qc(struct ata_port *ap)\n {\n-\tstruct ata_queued_cmd *qc = ap->deferred_qc;\n+\tstruct ata_link *link;\n \n \tlockdep_assert_held(ap->lock);\n \n@@ -1697,16 +1698,21 @@ void ata_scsi_requeue_deferred_qc(struct ata_port *ap)\n \t * do not try to be smart about what to do with this deferred command\n \t * and simply requeue it by completing it with DID_REQUEUE.\n \t */\n-\tif (qc) {\n-\t\tap->deferred_qc = NULL;\n-\t\tcancel_work(&ap->deferred_qc_work);\n-\t\tata_scsi_qc_done(qc, true, DID_REQUEUE << 16);\n+\tata_for_each_link(link, ap, EDGE) {\n+\t\tstruct ata_queued_cmd *qc = link->deferred_qc;\n+\n+\t\tif (qc) {\n+\t\t\tlink->deferred_qc = NULL;\n+\t\t\tcancel_work(&link->deferred_qc_work);\n+\t\t\tata_scsi_qc_done(qc, true, DID_REQUEUE << 16);\n+\t\t}\n \t}\n }\n \n-static void ata_scsi_schedule_deferred_qc(struct ata_port *ap)\n+static void ata_scsi_schedule_deferred_qc(struct ata_link *link)\n {\n-\tstruct ata_queued_cmd *qc = ap->deferred_qc;\n+\tstruct ata_queued_cmd *qc = link->deferred_qc;\n+\tstruct ata_port *ap = link->ap;\n \n \tlockdep_assert_held(ap->lock);\n \n@@ -1723,12 +1729,12 @@ static void ata_scsi_schedule_deferred_qc(struct ata_port *ap)\n \t\treturn;\n \t}\n \tif (!ap->ops->qc_defer(qc))\n-\t\tqueue_work(system_highpri_wq, &ap->deferred_qc_work);\n+\t\tqueue_work(system_highpri_wq, &link->deferred_qc_work);\n }\n \n static void ata_scsi_qc_complete(struct ata_queued_cmd *qc)\n {\n-\tstruct ata_port *ap = qc->ap;\n+\tstruct ata_link *link = qc->dev->link;\n \tstruct scsi_cmnd *cmd = qc->scsicmd;\n \tu8 *cdb = cmd->cmnd;\n \tbool have_sense = qc->flags & ATA_QCFLAG_SENSE_VALID;\n@@ -1759,11 +1765,12 @@ static void ata_scsi_qc_complete(struct ata_queued_cmd *qc)\n \n \tata_scsi_qc_done(qc, false, 0);\n \n-\tata_scsi_schedule_deferred_qc(ap);\n+\tata_scsi_schedule_deferred_qc(link);\n }\n \n static int ata_scsi_qc_issue(struct ata_port *ap, struct ata_queued_cmd *qc)\n {\n+\tstruct ata_link *link = qc->dev->link;\n \tint ret;\n \n \tif (!ap->ops->qc_defer)\n@@ -1774,7 +1781,7 @@ static int ata_scsi_qc_issue(struct ata_port *ap, struct ata_queued_cmd *qc)\n \t * requeue and defer all incoming commands until the deferred qc is\n \t * processed, once all on-going commands complete.\n \t */\n-\tif (ap->deferred_qc) {\n+\tif (link->deferred_qc) {\n \t\tata_qc_free(qc);\n \t\treturn SCSI_MLQUEUE_DEVICE_BUSY;\n \t}\n@@ -1793,7 +1800,7 @@ static int ata_scsi_qc_issue(struct ata_port *ap, struct ata_queued_cmd *qc)\n \t\t * commands complete.\n \t\t */\n \t\tif (!ata_is_ncq(qc->tf.protocol)) {\n-\t\t\tap->deferred_qc = qc;\n+\t\t\tlink->deferred_qc = qc;\n \t\t\treturn 0;\n \t\t}\n \ndiff --git a/include/linux/libata.h b/include/linux/libata.h\nindex 5c085ef4eda7..a04384e67b84 100644\n--- a/include/linux/libata.h\n+++ b/include/linux/libata.h\n@@ -854,6 +854,9 @@ struct ata_link {\n \tunsigned int\t\tsata_spd;\t/* current SATA PHY speed */\n \tenum ata_lpm_policy\tlpm_policy;\n \n+\tstruct work_struct\tdeferred_qc_work;\n+\tstruct ata_queued_cmd\t*deferred_qc;\n+\n \t/* record runtime error info, protected by host_set lock */\n \tstruct ata_eh_info\teh_info;\n \t/* EH context */\n@@ -899,9 +902,6 @@ struct ata_port {\n \tu64\t\t\tqc_active;\n \tint\t\t\tnr_active_links; /* #links with active qcs */\n \n-\tstruct work_struct\tdeferred_qc_work;\n-\tstruct ata_queued_cmd\t*deferred_qc;\n-\n \tstruct ata_link\t\tlink;\t\t/* host default link */\n \tstruct ata_link\t\t*slave_link;\t/* see ata_slave_link_init() */\n \n","prefixes":["3/3"]}