From patchwork Thu Dec 29 16:59:57 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Niklas Cassel X-Patchwork-Id: 638331 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id BE810C46467 for ; Thu, 29 Dec 2022 17:00:23 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233600AbiL2RAV (ORCPT ); Thu, 29 Dec 2022 12:00:21 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42662 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233575AbiL2RAQ (ORCPT ); Thu, 29 Dec 2022 12:00:16 -0500 Received: from esa2.hgst.iphmx.com (esa2.hgst.iphmx.com [68.232.143.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 22896101D6; Thu, 29 Dec 2022 09:00:15 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=wdc.com; i=@wdc.com; q=dns/txt; s=dkim.wdc.com; t=1672333215; x=1703869215; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=D/lRENqbiIXYinNFSFDpkw7ukUGgZpe7PO2Ug2lf2+Y=; b=fJGYlUS34HXTu0OV3lwyOVFYALS8CSJbqxxSFQrgr+zjHggAmTc6iv/e f7xhl3gro1EdeOgKm6hKIKLoBrMzAFBUQO9+SaMSaUG0/LlmS/vcpcmfF s0+AvRw6CybeWzMnRN49Z4irvbUH0LO15BGJ8494uehqiZij1qWN/6s9G OkU+rSOKaBRVLVZQSbTceRaptl7MwBSJ5hMGtjKW8yE5gsBm4ZlNWkrIS +GEIoSbyJiG4FkL+x/xnCfjPFRdJ/9OtS2WDPjbJ7xTfuU2LZVOo+rSOa cqaBtIPYAfl5ASQSwvv9ZLKK3sAq8j4d58AY4+Ya/wdO7PaYCqyzbFFGV g==; X-IronPort-AV: E=Sophos;i="5.96,284,1665417600"; d="scan'208";a="323990970" Received: from h199-255-45-14.hgst.com (HELO uls-op-cesaep01.wdc.com) ([199.255.45.14]) by ob1.hgst.iphmx.com with ESMTP; 30 Dec 2022 01:00:14 +0800 IronPort-SDR: 0tYZJ5z1+I89gDbk2FC3ZRO2ANHqnLr0R9tiyyvx6CN1Q5hHLm7S0NmIGjW+SpVXoI3ZOv5HGa 2odASXOrRj5BsZXsQQsSCJ6lumI7uDM97zcl75AIpPObleAdZM46CccnX2FHducBshU11AXYgr y+m6QRjRJqRpgZXO74koQ8//fd5XXhku5sTEiuZGY6onOUHG9TkXEjuWKQ9knNbJmm+2f/wDDF 367lMw7Rqnms+afRQOMGnIjsHLOhgBTZAH9483WK8zzcsvNv05MOs09zxiWyLWyJipWTNSb+uo Ruk= Received: from uls-op-cesaip02.wdc.com ([10.248.3.37]) by uls-op-cesaep01.wdc.com with ESMTP/TLS/ECDHE-RSA-AES128-GCM-SHA256; 29 Dec 2022 08:18:18 -0800 IronPort-SDR: RDHxQnMcxfI+YK8zcqsPauQoTLLtulHqsKibYaLOE7XdO712vtmF6EhdhlRq7fkBxvkUWGfCZc wLB5A23mrr9GBqOvruon+IVXq/BnLrdsIevuU+yDwbhjOP/SuwUjTVnZXgGz5F0LTL/O8UsACn OVk7IRkorKtLaw/cMY9MAIhf73sE3hR72Nq2f72ULA5oAtKMOW6hHLgQpL6I7duRQT0rIJaqw4 lG1rpEGwQftDvNnL2AQWXd/99k/o4bB7wvWRQgB4WXGJ4RvowoOQSvHQqXEhRb3ZcKHFWe5gXC ajo= WDCIronportException: Internal Received: from unknown (HELO x1-carbon.lan) ([10.225.164.9]) by uls-op-cesaip02.wdc.com with ESMTP; 29 Dec 2022 09:00:13 -0800 From: Niklas Cassel To: Damien Le Moal , Mikael Pettersson , Brian King , "James E.J. Bottomley" , "Martin K. Petersen" Cc: Niklas Cassel , John Garry , linux-ide@vger.kernel.org, linux-scsi@vger.kernel.org Subject: [PATCH v2 1/7] ata: scsi: rename flag ATA_QCFLAG_FAILED to ATA_QCFLAG_EH Date: Thu, 29 Dec 2022 17:59:57 +0100 Message-Id: <20221229170005.49118-2-niklas.cassel@wdc.com> X-Mailer: git-send-email 2.38.1 In-Reply-To: <20221229170005.49118-1-niklas.cassel@wdc.com> References: <20221229170005.49118-1-niklas.cassel@wdc.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org The name ATA_QCFLAG_FAILED is misleading since it does not mean that a QC completed in error, or that it didn't complete at all. It means that libata decided to schedule EH for the QC, so the QC is now owned by the libata error handler (EH). The normal execution path is responsible for not accessing a QC owned by EH. libata core enforces the rule by returning NULL from ata_qc_from_tag() for QCs owned by EH. It is quite easy to mistake that a QC marked with ATA_QCFLAG_FAILED was an error. However, a QC that was actually an error is instead indicated by having qc->err_mask set. E.g. when we have a NCQ error, we abort all QCs, which currently will mark all QCs as ATA_QCFLAG_FAILED. However, it will only be a single QC that is an error (i.e. has qc->err_mask set). Rename ATA_QCFLAG_FAILED to ATA_QCFLAG_EH to more clearly highlight that this flag simply means that a QC is now owned by EH. This new name will not mislead to think that the QC was an error (which is instead indicated by having qc->err_mask set). This also makes it more obvious that the EH code skips all QCs that do not have ATA_QCFLAG_EH set (rather than ATA_QCFLAG_FAILED), since the EH code should simply only care about QCs that are owned by EH itself. Signed-off-by: Niklas Cassel Reviewed-by: John Garry --- drivers/ata/acard-ahci.c | 2 +- drivers/ata/libahci.c | 4 ++-- drivers/ata/libata-core.c | 12 ++++++------ drivers/ata/libata-eh.c | 22 +++++++++++----------- drivers/ata/libata-sata.c | 4 ++-- drivers/ata/libata-sff.c | 4 ++-- drivers/ata/libata-trace.c | 2 +- drivers/ata/sata_fsl.c | 2 +- drivers/ata/sata_inic162x.c | 2 +- drivers/ata/sata_promise.c | 2 +- drivers/ata/sata_sil24.c | 2 +- drivers/ata/sata_sx4.c | 2 +- drivers/scsi/ipr.c | 4 ++-- drivers/scsi/libsas/sas_ata.c | 8 ++++---- include/linux/libata.h | 4 ++-- 15 files changed, 38 insertions(+), 38 deletions(-) diff --git a/drivers/ata/acard-ahci.c b/drivers/ata/acard-ahci.c index 7654a40c12b4..da74a86b70ba 100644 --- a/drivers/ata/acard-ahci.c +++ b/drivers/ata/acard-ahci.c @@ -263,7 +263,7 @@ static bool acard_ahci_qc_fill_rtf(struct ata_queued_cmd *qc) * Setup FIS. */ if (qc->tf.protocol == ATA_PROT_PIO && qc->dma_dir == DMA_FROM_DEVICE && - !(qc->flags & ATA_QCFLAG_FAILED)) { + !(qc->flags & ATA_QCFLAG_EH)) { ata_tf_from_fis(rx_fis + RX_FIS_PIO_SETUP, &qc->result_tf); qc->result_tf.status = (rx_fis + RX_FIS_PIO_SETUP)[15]; } else diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c index 29acc35bf4a6..03aa9eb415d3 100644 --- a/drivers/ata/libahci.c +++ b/drivers/ata/libahci.c @@ -2068,7 +2068,7 @@ static bool ahci_qc_fill_rtf(struct ata_queued_cmd *qc) * Setup FIS. */ if (qc->tf.protocol == ATA_PROT_PIO && qc->dma_dir == DMA_FROM_DEVICE && - !(qc->flags & ATA_QCFLAG_FAILED)) { + !(qc->flags & ATA_QCFLAG_EH)) { ata_tf_from_fis(rx_fis + RX_FIS_PIO_SETUP, &qc->result_tf); qc->result_tf.status = (rx_fis + RX_FIS_PIO_SETUP)[15]; @@ -2138,7 +2138,7 @@ static void ahci_post_internal_cmd(struct ata_queued_cmd *qc) struct ata_port *ap = qc->ap; /* make DMA engine forget about the failed command */ - if (qc->flags & ATA_QCFLAG_FAILED) + if (qc->flags & ATA_QCFLAG_EH) ahci_kick_engine(ap); } diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 884ae73b11ea..6b03bebcde50 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -1590,7 +1590,7 @@ static unsigned ata_exec_internal_sg(struct ata_device *dev, ap->ops->post_internal_cmd(qc); /* perform minimal error analysis */ - if (qc->flags & ATA_QCFLAG_FAILED) { + if (qc->flags & ATA_QCFLAG_EH) { if (qc->result_tf.status & (ATA_ERR | ATA_DF)) qc->err_mask |= AC_ERR_DEV; @@ -4683,10 +4683,10 @@ void ata_qc_complete(struct ata_queued_cmd *qc) /* XXX: New EH and old EH use different mechanisms to * synchronize EH with regular execution path. * - * In new EH, a failed qc is marked with ATA_QCFLAG_FAILED. + * In new EH, a qc owned by EH is marked with ATA_QCFLAG_EH. * Normal execution path is responsible for not accessing a - * failed qc. libata core enforces the rule by returning NULL - * from ata_qc_from_tag() for failed qcs. + * qc owned by EH. libata core enforces the rule by returning NULL + * from ata_qc_from_tag() for qcs owned by EH. * * Old EH depends on ata_qc_complete() nullifying completion * requests if ATA_QCFLAG_EH_SCHEDULED is set. Old EH does @@ -4698,7 +4698,7 @@ void ata_qc_complete(struct ata_queued_cmd *qc) struct ata_eh_info *ehi = &dev->link->eh_info; if (unlikely(qc->err_mask)) - qc->flags |= ATA_QCFLAG_FAILED; + qc->flags |= ATA_QCFLAG_EH; /* * Finish internal commands without any further processing @@ -4715,7 +4715,7 @@ void ata_qc_complete(struct ata_queued_cmd *qc) * Non-internal qc has failed. Fill the result TF and * summon EH. */ - if (unlikely(qc->flags & ATA_QCFLAG_FAILED)) { + if (unlikely(qc->flags & ATA_QCFLAG_EH)) { fill_result_tf(qc); trace_ata_qc_complete_failed(qc); ata_qc_schedule_eh(qc); diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c index 34303ce67c14..8cb250930c48 100644 --- a/drivers/ata/libata-eh.c +++ b/drivers/ata/libata-eh.c @@ -575,7 +575,7 @@ void ata_scsi_cmd_error_handler(struct Scsi_Host *host, struct ata_port *ap, * normal completion, error completion, and SCSI timeout. * Both completions can race against SCSI timeout. When normal * completion wins, the qc never reaches EH. When error - * completion wins, the qc has ATA_QCFLAG_FAILED set. + * completion wins, the qc has ATA_QCFLAG_EH set. * * When SCSI timeout wins, things are a bit more complex. * Normal or error completion can occur after the timeout but @@ -611,10 +611,10 @@ void ata_scsi_cmd_error_handler(struct Scsi_Host *host, struct ata_port *ap, if (i < ATA_MAX_QUEUE) { /* the scmd has an associated qc */ - if (!(qc->flags & ATA_QCFLAG_FAILED)) { + if (!(qc->flags & ATA_QCFLAG_EH)) { /* which hasn't failed yet, timeout */ qc->err_mask |= AC_ERR_TIMEOUT; - qc->flags |= ATA_QCFLAG_FAILED; + qc->flags |= ATA_QCFLAG_EH; nr_timedout++; } } else { @@ -631,7 +631,7 @@ void ata_scsi_cmd_error_handler(struct Scsi_Host *host, struct ata_port *ap, * this point but the state of the controller is * unknown. Freeze the port to make sure the IRQ * handler doesn't diddle with those qcs. This must - * be done atomically w.r.t. setting QCFLAG_FAILED. + * be done atomically w.r.t. setting ATA_QCFLAG_EH. */ if (nr_timedout) __ata_port_freeze(ap); @@ -911,12 +911,12 @@ void ata_qc_schedule_eh(struct ata_queued_cmd *qc) WARN_ON(!ap->ops->error_handler); - qc->flags |= ATA_QCFLAG_FAILED; + qc->flags |= ATA_QCFLAG_EH; ata_eh_set_pending(ap, 1); /* The following will fail if timeout has already expired. * ata_scsi_error() takes care of such scmds on EH entry. - * Note that ATA_QCFLAG_FAILED is unconditionally set after + * Note that ATA_QCFLAG_EH is unconditionally set after * this function completes. */ blk_abort_request(scsi_cmd_to_rq(qc->scsicmd)); @@ -994,7 +994,7 @@ static int ata_do_link_abort(struct ata_port *ap, struct ata_link *link) /* include internal tag in iteration */ ata_qc_for_each_with_internal(ap, qc, tag) { if (qc && (!link || qc->dev->link == link)) { - qc->flags |= ATA_QCFLAG_FAILED; + qc->flags |= ATA_QCFLAG_EH; ata_qc_complete(qc); nr_aborted++; } @@ -1954,7 +1954,7 @@ static void ata_eh_link_autopsy(struct ata_link *link) all_err_mask |= ehc->i.err_mask; ata_qc_for_each_raw(ap, qc, tag) { - if (!(qc->flags & ATA_QCFLAG_FAILED) || + if (!(qc->flags & ATA_QCFLAG_EH) || qc->flags & ATA_QCFLAG_RETRY || ata_dev_phys_link(qc->dev) != link) continue; @@ -2232,7 +2232,7 @@ static void ata_eh_link_report(struct ata_link *link) desc = ehc->i.desc; ata_qc_for_each_raw(ap, qc, tag) { - if (!(qc->flags & ATA_QCFLAG_FAILED) || + if (!(qc->flags & ATA_QCFLAG_EH) || ata_dev_phys_link(qc->dev) != link || ((qc->flags & ATA_QCFLAG_QUIET) && qc->err_mask == AC_ERR_DEV)) @@ -2298,7 +2298,7 @@ static void ata_eh_link_report(struct ata_link *link) char data_buf[20] = ""; char cdb_buf[70] = ""; - if (!(qc->flags & ATA_QCFLAG_FAILED) || + if (!(qc->flags & ATA_QCFLAG_EH) || ata_dev_phys_link(qc->dev) != link || !qc->err_mask) continue; @@ -3802,7 +3802,7 @@ void ata_eh_finish(struct ata_port *ap) /* retry or finish qcs */ ata_qc_for_each_raw(ap, qc, tag) { - if (!(qc->flags & ATA_QCFLAG_FAILED)) + if (!(qc->flags & ATA_QCFLAG_EH)) continue; if (qc->err_mask) { diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c index 18ef14e749a0..908f35acee1e 100644 --- a/drivers/ata/libata-sata.c +++ b/drivers/ata/libata-sata.c @@ -1429,7 +1429,7 @@ void ata_eh_analyze_ncq_error(struct ata_link *link) /* has LLDD analyzed already? */ ata_qc_for_each_raw(ap, qc, tag) { - if (!(qc->flags & ATA_QCFLAG_FAILED)) + if (!(qc->flags & ATA_QCFLAG_EH)) continue; if (qc->err_mask) @@ -1477,7 +1477,7 @@ void ata_eh_analyze_ncq_error(struct ata_link *link) } ata_qc_for_each_raw(ap, qc, tag) { - if (!(qc->flags & ATA_QCFLAG_FAILED) || + if (!(qc->flags & ATA_QCFLAG_EH) || ata_dev_phys_link(qc->dev) != link) continue; diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c index 153f49e00713..34beda28e712 100644 --- a/drivers/ata/libata-sff.c +++ b/drivers/ata/libata-sff.c @@ -2073,7 +2073,7 @@ void ata_sff_error_handler(struct ata_port *ap) unsigned long flags; qc = __ata_qc_from_tag(ap, ap->link.active_tag); - if (qc && !(qc->flags & ATA_QCFLAG_FAILED)) + if (qc && !(qc->flags & ATA_QCFLAG_EH)) qc = NULL; spin_lock_irqsave(ap->lock, flags); @@ -2796,7 +2796,7 @@ void ata_bmdma_error_handler(struct ata_port *ap) bool thaw = false; qc = __ata_qc_from_tag(ap, ap->link.active_tag); - if (qc && !(qc->flags & ATA_QCFLAG_FAILED)) + if (qc && !(qc->flags & ATA_QCFLAG_EH)) qc = NULL; /* reset PIO HSM and stop DMA engine */ diff --git a/drivers/ata/libata-trace.c b/drivers/ata/libata-trace.c index e0e4d0d5a100..9b5363fd0ab0 100644 --- a/drivers/ata/libata-trace.c +++ b/drivers/ata/libata-trace.c @@ -142,7 +142,7 @@ libata_trace_parse_qc_flags(struct trace_seq *p, unsigned int qc_flags) trace_seq_printf(p, "QUIET "); if (qc_flags & ATA_QCFLAG_RETRY) trace_seq_printf(p, "RETRY "); - if (qc_flags & ATA_QCFLAG_FAILED) + if (qc_flags & ATA_QCFLAG_EH) trace_seq_printf(p, "FAILED "); if (qc_flags & ATA_QCFLAG_SENSE_VALID) trace_seq_printf(p, "SENSE_VALID "); diff --git a/drivers/ata/sata_fsl.c b/drivers/ata/sata_fsl.c index b9a4f68b371d..7eab9c4e1473 100644 --- a/drivers/ata/sata_fsl.c +++ b/drivers/ata/sata_fsl.c @@ -1042,7 +1042,7 @@ static void sata_fsl_error_handler(struct ata_port *ap) static void sata_fsl_post_internal_cmd(struct ata_queued_cmd *qc) { - if (qc->flags & ATA_QCFLAG_FAILED) + if (qc->flags & ATA_QCFLAG_EH) qc->err_mask |= AC_ERR_OTHER; if (qc->err_mask) { diff --git a/drivers/ata/sata_inic162x.c b/drivers/ata/sata_inic162x.c index 11e518f0111c..f480ff456190 100644 --- a/drivers/ata/sata_inic162x.c +++ b/drivers/ata/sata_inic162x.c @@ -672,7 +672,7 @@ static void inic_error_handler(struct ata_port *ap) static void inic_post_internal_cmd(struct ata_queued_cmd *qc) { /* make DMA engine forget about the failed command */ - if (qc->flags & ATA_QCFLAG_FAILED) + if (qc->flags & ATA_QCFLAG_EH) inic_reset_port(inic_port_base(qc->ap)); } diff --git a/drivers/ata/sata_promise.c b/drivers/ata/sata_promise.c index 9cd7d8b71361..4e60e6c4c35a 100644 --- a/drivers/ata/sata_promise.c +++ b/drivers/ata/sata_promise.c @@ -828,7 +828,7 @@ static void pdc_post_internal_cmd(struct ata_queued_cmd *qc) struct ata_port *ap = qc->ap; /* make DMA engine forget about the failed command */ - if (qc->flags & ATA_QCFLAG_FAILED) + if (qc->flags & ATA_QCFLAG_EH) pdc_reset_port(ap); } diff --git a/drivers/ata/sata_sil24.c b/drivers/ata/sata_sil24.c index 2fef6ce93f07..0a01518a8d97 100644 --- a/drivers/ata/sata_sil24.c +++ b/drivers/ata/sata_sil24.c @@ -1185,7 +1185,7 @@ static void sil24_post_internal_cmd(struct ata_queued_cmd *qc) struct ata_port *ap = qc->ap; /* make DMA engine forget about the failed command */ - if ((qc->flags & ATA_QCFLAG_FAILED) && sil24_init_port(ap)) + if ((qc->flags & ATA_QCFLAG_EH) && sil24_init_port(ap)) ata_eh_freeze_port(ap); } diff --git a/drivers/ata/sata_sx4.c b/drivers/ata/sata_sx4.c index ab70cbc78f96..a92c60455b1d 100644 --- a/drivers/ata/sata_sx4.c +++ b/drivers/ata/sata_sx4.c @@ -866,7 +866,7 @@ static void pdc_post_internal_cmd(struct ata_queued_cmd *qc) struct ata_port *ap = qc->ap; /* make DMA engine forget about the failed command */ - if (qc->flags & ATA_QCFLAG_FAILED) + if (qc->flags & ATA_QCFLAG_EH) pdc_reset_port(ap); } diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c index 2022ffb45041..c68ca2218a05 100644 --- a/drivers/scsi/ipr.c +++ b/drivers/scsi/ipr.c @@ -5370,9 +5370,9 @@ static int __ipr_eh_dev_reset(struct scsi_cmnd *scsi_cmd) continue; ipr_cmd->done = ipr_sata_eh_done; - if (!(ipr_cmd->qc->flags & ATA_QCFLAG_FAILED)) { + if (!(ipr_cmd->qc->flags & ATA_QCFLAG_EH)) { ipr_cmd->qc->err_mask |= AC_ERR_TIMEOUT; - ipr_cmd->qc->flags |= ATA_QCFLAG_FAILED; + ipr_cmd->qc->flags |= ATA_QCFLAG_EH; } } } diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c index 1ccce706167a..14da33a3b6a6 100644 --- a/drivers/scsi/libsas/sas_ata.c +++ b/drivers/scsi/libsas/sas_ata.c @@ -125,7 +125,7 @@ static void sas_ata_task_done(struct sas_task *task) } else { link->eh_info.err_mask |= ac_err_mask(dev->sata_dev.fis[2]); if (unlikely(link->eh_info.err_mask)) - qc->flags |= ATA_QCFLAG_FAILED; + qc->flags |= ATA_QCFLAG_EH; } } else { ac = sas_to_ata_err(stat); @@ -136,7 +136,7 @@ static void sas_ata_task_done(struct sas_task *task) qc->err_mask = ac; } else { link->eh_info.err_mask |= AC_ERR_DEV; - qc->flags |= ATA_QCFLAG_FAILED; + qc->flags |= ATA_QCFLAG_EH; } dev->sata_dev.fis[2] = ATA_ERR | ATA_DRDY; /* tf status */ @@ -476,7 +476,7 @@ static void sas_ata_internal_abort(struct sas_task *task) static void sas_ata_post_internal(struct ata_queued_cmd *qc) { - if (qc->flags & ATA_QCFLAG_FAILED) + if (qc->flags & ATA_QCFLAG_EH) qc->err_mask |= AC_ERR_OTHER; if (qc->err_mask) { @@ -631,7 +631,7 @@ void sas_ata_task_abort(struct sas_task *task) /* Internal command, fake a timeout and complete. */ qc->flags &= ~ATA_QCFLAG_ACTIVE; - qc->flags |= ATA_QCFLAG_FAILED; + qc->flags |= ATA_QCFLAG_EH; qc->err_mask |= AC_ERR_TIMEOUT; waiting = qc->private_data; complete(waiting); diff --git a/include/linux/libata.h b/include/linux/libata.h index c9149ebe7423..7985e6e2ae0e 100644 --- a/include/linux/libata.h +++ b/include/linux/libata.h @@ -206,7 +206,7 @@ enum { ATA_QCFLAG_QUIET = (1 << 6), /* don't report device error */ ATA_QCFLAG_RETRY = (1 << 7), /* retry after failure */ - ATA_QCFLAG_FAILED = (1 << 16), /* cmd failed and is owned by EH */ + ATA_QCFLAG_EH = (1 << 16), /* cmd aborted and owned by EH */ ATA_QCFLAG_SENSE_VALID = (1 << 17), /* sense data valid */ ATA_QCFLAG_EH_SCHEDULED = (1 << 18), /* EH scheduled (obsolete) */ @@ -1756,7 +1756,7 @@ static inline struct ata_queued_cmd *ata_qc_from_tag(struct ata_port *ap, return qc; if ((qc->flags & (ATA_QCFLAG_ACTIVE | - ATA_QCFLAG_FAILED)) == ATA_QCFLAG_ACTIVE) + ATA_QCFLAG_EH)) == ATA_QCFLAG_ACTIVE) return qc; return NULL; From patchwork Thu Dec 29 16:59:58 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Niklas Cassel X-Patchwork-Id: 637789 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0AD34C4708E for ; Thu, 29 Dec 2022 17:00:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233614AbiL2RAX (ORCPT ); Thu, 29 Dec 2022 12:00:23 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42668 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233597AbiL2RAU (ORCPT ); Thu, 29 Dec 2022 12:00:20 -0500 Received: from esa2.hgst.iphmx.com (esa2.hgst.iphmx.com [68.232.143.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D7333F5A7; Thu, 29 Dec 2022 09:00:18 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=wdc.com; i=@wdc.com; q=dns/txt; s=dkim.wdc.com; t=1672333218; x=1703869218; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=pli0p8qWBG/fw32ZyJYrgU57uHIz5Ggmoyz9LzzBTXc=; b=VESXedALYS9FYESpX7wcexybsUJxJ2Yp3HVEw/kzmQNjmsHbKvYBOsV9 hpwqMKvtcvy9SdOxX0V5a6qVFhzaJgCdUTHias5xqxmUdahEjRXuIBJVp R65xoYuv8L3rVU2FRdM//ZEX47eM8AB+Nbmx5taiCihEBhaVCX8jKDZnD mHSTpzFIC08kNviu+0Ae4GNzenWwWhPUXyC92RNZQ5JRqLI4xynDFJR3+ rnMxbzbismthKsbu/FggN7KqI61KiufWMHt3d8xItKbg78W2xScez1oQw tcZ2tgvfXw8ibdRiIMLcTWpYQgz9IG71Zk3GlUtBRQJYMUwloGt4A1BLZ A==; X-IronPort-AV: E=Sophos;i="5.96,284,1665417600"; d="scan'208";a="323990980" Received: from h199-255-45-14.hgst.com (HELO uls-op-cesaep01.wdc.com) ([199.255.45.14]) by ob1.hgst.iphmx.com with ESMTP; 30 Dec 2022 01:00:18 +0800 IronPort-SDR: F6XP4QRhK1P7ZM62Hg5w6tW2M6Gf7H/g0LZf08aS8ZIapnZ+EOGyw3/aGvI8Gp4HUShFnNhP7g X+GAB9E4iY3LzlGWa0rlriaVBzIHNcD0PMB93vvsxHRU9V6KGSWUXKn64/aaipetjLc9rRIMf0 h9LAnMREiQiPHfbb14VE7TgP9b5ArSp7HSIhPjNvkk+YHcjueIFV2oMATdgcCQo/M0ihq0trLM EVrqXuZg6WF4nryZZGmb/UFTN78eTHi1HZXkoow2qSyLZQziMAhtH8vLH9CkixKbUeBRH/oX7H GpE= Received: from uls-op-cesaip02.wdc.com ([10.248.3.37]) by uls-op-cesaep01.wdc.com with ESMTP/TLS/ECDHE-RSA-AES128-GCM-SHA256; 29 Dec 2022 08:18:22 -0800 IronPort-SDR: Nru65MeL5cdfhQVCSSwAEpfCvFsyvIDy/MoJ4kTlDq7w6Ou6rG/lX1RwzA87u2t49TGRszVRCn ZC1oldwvfGUv1a0bFyLbyk1o4k2fS1Jmd7q/0C327WLEAkU/hIDPcyGau6m/XZMggO3CooufCT hXpyFlvK8TxqxQKMDX+r5kihjBfApGxXxLYhcyfcWG0ej+UYwYILJ/9WocOmgTYetZ6zBq0RNF SQK6XUJSU7x8wZt5/sbkgSO20UNYxHJI8e7AZunhIQIkA1jVpS/gY2iwWSaZHfVKqwvomoCudF m00= WDCIronportException: Internal Received: from unknown (HELO x1-carbon.lan) ([10.225.164.9]) by uls-op-cesaip02.wdc.com with ESMTP; 29 Dec 2022 09:00:17 -0800 From: Niklas Cassel To: Damien Le Moal , Brian King , "James E.J. Bottomley" , "Martin K. Petersen" Cc: Niklas Cassel , John Garry , linux-ide@vger.kernel.org, linux-scsi@vger.kernel.org Subject: [PATCH v2 2/7] ata: libata: simplify qc_fill_rtf port operation interface Date: Thu, 29 Dec 2022 17:59:58 +0100 Message-Id: <20221229170005.49118-3-niklas.cassel@wdc.com> X-Mailer: git-send-email 2.38.1 In-Reply-To: <20221229170005.49118-1-niklas.cassel@wdc.com> References: <20221229170005.49118-1-niklas.cassel@wdc.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org From: Damien Le Moal The boolean return value of the qc_fill_rtf operation is used nowhere. Simplify this operation interface by making it a void function. All drivers defining this operation are also updated. Signed-off-by: Damien Le Moal Signed-off-by: Niklas Cassel Reviewed-by: John Garry --- drivers/ata/acard-ahci.c | 6 ++---- drivers/ata/libahci.c | 6 ++---- drivers/ata/libata-sff.c | 6 +----- drivers/ata/sata_fsl.c | 3 +-- drivers/ata/sata_inic162x.c | 12 +++++------- drivers/ata/sata_sil24.c | 5 ++--- drivers/scsi/ipr.c | 7 +------ drivers/scsi/libsas/sas_ata.c | 3 +-- include/linux/libata.h | 4 ++-- 9 files changed, 17 insertions(+), 35 deletions(-) diff --git a/drivers/ata/acard-ahci.c b/drivers/ata/acard-ahci.c index da74a86b70ba..993eadd173da 100644 --- a/drivers/ata/acard-ahci.c +++ b/drivers/ata/acard-ahci.c @@ -57,7 +57,7 @@ struct acard_sg { }; static enum ata_completion_errors acard_ahci_qc_prep(struct ata_queued_cmd *qc); -static bool acard_ahci_qc_fill_rtf(struct ata_queued_cmd *qc); +static void acard_ahci_qc_fill_rtf(struct ata_queued_cmd *qc); static int acard_ahci_port_start(struct ata_port *ap); static int acard_ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent); @@ -248,7 +248,7 @@ static enum ata_completion_errors acard_ahci_qc_prep(struct ata_queued_cmd *qc) return AC_ERR_OK; } -static bool acard_ahci_qc_fill_rtf(struct ata_queued_cmd *qc) +static void acard_ahci_qc_fill_rtf(struct ata_queued_cmd *qc) { struct ahci_port_priv *pp = qc->ap->private_data; u8 *rx_fis = pp->rx_fis; @@ -268,8 +268,6 @@ static bool acard_ahci_qc_fill_rtf(struct ata_queued_cmd *qc) qc->result_tf.status = (rx_fis + RX_FIS_PIO_SETUP)[15]; } else ata_tf_from_fis(rx_fis + RX_FIS_D2H_REG, &qc->result_tf); - - return true; } static int acard_ahci_port_start(struct ata_port *ap) diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c index 03aa9eb415d3..0167aac25c34 100644 --- a/drivers/ata/libahci.c +++ b/drivers/ata/libahci.c @@ -55,7 +55,7 @@ static ssize_t ahci_transmit_led_message(struct ata_port *ap, u32 state, static int ahci_scr_read(struct ata_link *link, unsigned int sc_reg, u32 *val); static int ahci_scr_write(struct ata_link *link, unsigned int sc_reg, u32 val); -static bool ahci_qc_fill_rtf(struct ata_queued_cmd *qc); +static void ahci_qc_fill_rtf(struct ata_queued_cmd *qc); static int ahci_port_start(struct ata_port *ap); static void ahci_port_stop(struct ata_port *ap); static enum ata_completion_errors ahci_qc_prep(struct ata_queued_cmd *qc); @@ -2053,7 +2053,7 @@ unsigned int ahci_qc_issue(struct ata_queued_cmd *qc) } EXPORT_SYMBOL_GPL(ahci_qc_issue); -static bool ahci_qc_fill_rtf(struct ata_queued_cmd *qc) +static void ahci_qc_fill_rtf(struct ata_queued_cmd *qc) { struct ahci_port_priv *pp = qc->ap->private_data; u8 *rx_fis = pp->rx_fis; @@ -2087,8 +2087,6 @@ static bool ahci_qc_fill_rtf(struct ata_queued_cmd *qc) qc->result_tf.error = fis[3]; } else ata_tf_from_fis(rx_fis + RX_FIS_D2H_REG, &qc->result_tf); - - return true; } static void ahci_freeze(struct ata_port *ap) diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c index 34beda28e712..cd82d3b5ed14 100644 --- a/drivers/ata/libata-sff.c +++ b/drivers/ata/libata-sff.c @@ -1377,14 +1377,10 @@ EXPORT_SYMBOL_GPL(ata_sff_qc_issue); * * LOCKING: * spin_lock_irqsave(host lock) - * - * RETURNS: - * true indicating that result TF is successfully filled. */ -bool ata_sff_qc_fill_rtf(struct ata_queued_cmd *qc) +void ata_sff_qc_fill_rtf(struct ata_queued_cmd *qc) { qc->ap->ops->sff_tf_read(qc->ap, &qc->result_tf); - return true; } EXPORT_SYMBOL_GPL(ata_sff_qc_fill_rtf); diff --git a/drivers/ata/sata_fsl.c b/drivers/ata/sata_fsl.c index 7eab9c4e1473..b052c5a65c17 100644 --- a/drivers/ata/sata_fsl.c +++ b/drivers/ata/sata_fsl.c @@ -566,7 +566,7 @@ static unsigned int sata_fsl_qc_issue(struct ata_queued_cmd *qc) return 0; } -static bool sata_fsl_qc_fill_rtf(struct ata_queued_cmd *qc) +static void sata_fsl_qc_fill_rtf(struct ata_queued_cmd *qc) { struct sata_fsl_port_priv *pp = qc->ap->private_data; struct sata_fsl_host_priv *host_priv = qc->ap->host->private_data; @@ -577,7 +577,6 @@ static bool sata_fsl_qc_fill_rtf(struct ata_queued_cmd *qc) cd = pp->cmdentry + tag; ata_tf_from_fis(cd->sfis, &qc->result_tf); - return true; } static int sata_fsl_scr_write(struct ata_link *link, diff --git a/drivers/ata/sata_inic162x.c b/drivers/ata/sata_inic162x.c index f480ff456190..2833c722118d 100644 --- a/drivers/ata/sata_inic162x.c +++ b/drivers/ata/sata_inic162x.c @@ -566,7 +566,7 @@ static void inic_tf_read(struct ata_port *ap, struct ata_taskfile *tf) tf->status = readb(port_base + PORT_TF_COMMAND); } -static bool inic_qc_fill_rtf(struct ata_queued_cmd *qc) +static void inic_qc_fill_rtf(struct ata_queued_cmd *qc) { struct ata_taskfile *rtf = &qc->result_tf; struct ata_taskfile tf; @@ -580,12 +580,10 @@ static bool inic_qc_fill_rtf(struct ata_queued_cmd *qc) */ inic_tf_read(qc->ap, &tf); - if (!(tf.status & ATA_ERR)) - return false; - - rtf->status = tf.status; - rtf->error = tf.error; - return true; + if (tf.status & ATA_ERR) { + rtf->status = tf.status; + rtf->error = tf.error; + } } static void inic_freeze(struct ata_port *ap) diff --git a/drivers/ata/sata_sil24.c b/drivers/ata/sata_sil24.c index 0a01518a8d97..22cc9e9789dd 100644 --- a/drivers/ata/sata_sil24.c +++ b/drivers/ata/sata_sil24.c @@ -328,7 +328,7 @@ static int sil24_scr_write(struct ata_link *link, unsigned sc_reg, u32 val); static int sil24_qc_defer(struct ata_queued_cmd *qc); static enum ata_completion_errors sil24_qc_prep(struct ata_queued_cmd *qc); static unsigned int sil24_qc_issue(struct ata_queued_cmd *qc); -static bool sil24_qc_fill_rtf(struct ata_queued_cmd *qc); +static void sil24_qc_fill_rtf(struct ata_queued_cmd *qc); static void sil24_pmp_attach(struct ata_port *ap); static void sil24_pmp_detach(struct ata_port *ap); static void sil24_freeze(struct ata_port *ap); @@ -901,10 +901,9 @@ static unsigned int sil24_qc_issue(struct ata_queued_cmd *qc) return 0; } -static bool sil24_qc_fill_rtf(struct ata_queued_cmd *qc) +static void sil24_qc_fill_rtf(struct ata_queued_cmd *qc) { sil24_read_tf(qc->ap, qc->hw_tag, &qc->result_tf); - return true; } static void sil24_pmp_attach(struct ata_port *ap) diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c index c68ca2218a05..1c8040d250ea 100644 --- a/drivers/scsi/ipr.c +++ b/drivers/scsi/ipr.c @@ -7142,11 +7142,8 @@ static unsigned int ipr_qc_issue(struct ata_queued_cmd *qc) /** * ipr_qc_fill_rtf - Read result TF * @qc: ATA queued command - * - * Return value: - * true **/ -static bool ipr_qc_fill_rtf(struct ata_queued_cmd *qc) +static void ipr_qc_fill_rtf(struct ata_queued_cmd *qc) { struct ipr_sata_port *sata_port = qc->ap->private_data; struct ipr_ioasa_gata *g = &sata_port->ioasa; @@ -7163,8 +7160,6 @@ static bool ipr_qc_fill_rtf(struct ata_queued_cmd *qc) tf->hob_lbal = g->hob_lbal; tf->hob_lbam = g->hob_lbam; tf->hob_lbah = g->hob_lbah; - - return true; } static struct ata_port_operations ipr_sata_ops = { diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c index 14da33a3b6a6..ac8576e7f0b7 100644 --- a/drivers/scsi/libsas/sas_ata.c +++ b/drivers/scsi/libsas/sas_ata.c @@ -226,12 +226,11 @@ static unsigned int sas_ata_qc_issue(struct ata_queued_cmd *qc) return ret; } -static bool sas_ata_qc_fill_rtf(struct ata_queued_cmd *qc) +static void sas_ata_qc_fill_rtf(struct ata_queued_cmd *qc) { struct domain_device *dev = qc->ap->private_data; ata_tf_from_fis(dev->sata_dev.fis, &qc->result_tf); - return true; } static struct sas_internal *dev_to_sas_internal(struct domain_device *dev) diff --git a/include/linux/libata.h b/include/linux/libata.h index 7985e6e2ae0e..8483d8300ea3 100644 --- a/include/linux/libata.h +++ b/include/linux/libata.h @@ -876,7 +876,7 @@ struct ata_port_operations { int (*check_atapi_dma)(struct ata_queued_cmd *qc); enum ata_completion_errors (*qc_prep)(struct ata_queued_cmd *qc); unsigned int (*qc_issue)(struct ata_queued_cmd *qc); - bool (*qc_fill_rtf)(struct ata_queued_cmd *qc); + void (*qc_fill_rtf)(struct ata_queued_cmd *qc); /* * Configuration and exception handling @@ -1936,7 +1936,7 @@ extern void ata_sff_queue_delayed_work(struct delayed_work *dwork, unsigned long delay); extern void ata_sff_queue_pio_task(struct ata_link *link, unsigned long delay); extern unsigned int ata_sff_qc_issue(struct ata_queued_cmd *qc); -extern bool ata_sff_qc_fill_rtf(struct ata_queued_cmd *qc); +extern void ata_sff_qc_fill_rtf(struct ata_queued_cmd *qc); extern unsigned int ata_sff_port_intr(struct ata_port *ap, struct ata_queued_cmd *qc); extern irqreturn_t ata_sff_interrupt(int irq, void *dev_instance);