linux-kernel - Re: [PATCH v9 8/9] cxl/port: Retry reading CDAT on failure

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 31 May 2022 12:30:46 -0700
From:   Davidlohr Bueso <dave@...olabs.net>
To:     "ira.weiny@...el.com" <ira.weiny@...el.com>
Cc:     Dan Williams <dan.j.williams@...el.com>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Jonathan Cameron <Jonathan.Cameron@...wei.com>,
        Alison Schofield <alison.schofield@...el.com>,
        Vishal Verma <vishal.l.verma@...el.com>,
        Dave Jiang <dave.jiang@...el.com>,
        Ben Widawsky <ben@...dawsk.net>, linux-kernel@...r.kernel.org,
        linux-cxl@...r.kernel.org, linux-pci@...r.kernel.org,
        a.manzanares@...sung.com
Subject: Re: [PATCH v9 8/9] cxl/port: Retry reading CDAT on failure

On Tue, 31 May 2022, ira.weiny@...el.com wrote:

>From: Ira Weiny <ira.weiny@...el.com>
>
>The CDAT read may fail for a number of reasons but mainly it is possible
>to get different parts of a valid state.  The checksum in the CDAT table
>protects against this.
>
>Now that the cdat data is validated issue a retries if the CDAT read
>fails.  For now 5 retries are implemented.
>
>Signed-off-by: Ira Weiny <ira.weiny@...el.com>
>---
>Changes from V8
>	Move code to cxl/core/pci.c
>
>Changes from V6
>	Move to pci.c
>	Fix retries count
>	Change to 5 retries
>
>Changes from V5:
>	New patch -- easy to push off or drop.
>---
> drivers/cxl/core/pci.c | 32 +++++++++++++++++++++-----------
> 1 file changed, 21 insertions(+), 11 deletions(-)
>
>diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
>index bb370df1cb6c..7f7dac9030a6 100644
>--- a/drivers/cxl/core/pci.c
>+++ b/drivers/cxl/core/pci.c
>@@ -620,20 +620,18 @@ static int cxl_cdat_read_table(struct cxl_port *port,
> 	return rc;
> }
>
>-void read_cdat_data(struct cxl_port *port)
>+static int __read_cdat_data(struct cxl_port *port)
> {
> 	struct device *dev = &port->dev;
> 	size_t cdat_length;
> 	int ret;
>
> 	if (cxl_cdat_get_length(port, &cdat_length))
>-		return;
>+		return 0;
>
> 	port->cdat.table = devm_kzalloc(dev, cdat_length, GFP_KERNEL);
>-	if (!port->cdat.table) {
>-		ret = -ENOMEM;
>-		goto error;
>-	}
>+	if (!port->cdat.table)
>+		return -ENOMEM;
>
> 	port->cdat.length = cdat_length;
> 	ret = cxl_cdat_read_table(port, &port->cdat);
>@@ -641,12 +639,24 @@ void read_cdat_data(struct cxl_port *port)
> 		devm_kfree(dev, port->cdat.table);
> 		port->cdat.table = NULL;
> 		port->cdat.length = 0;
>-		ret = -EIO;
>-		goto error;
>+		return -EIO;
> 	}
>
>-	return;
>-error:
>-	dev_err(dev, "CDAT data read error (%d)\n", ret);
>+	return 0;
>+}
>+
>+void read_cdat_data(struct cxl_port *port)
>+{
>+	int retries = 5;
>+	int rc;
>+
>+	while (retries--) {
>+		rc = __read_cdat_data(port);
>+		if (!rc)
>+			break;
>+		dev_err(&port->dev,
>+			"CDAT data read error rc=%d (retries %d)\n",
>+			rc, retries);
>+	}
> }
> EXPORT_SYMBOL_NS_GPL(read_cdat_data, CXL);


Better a single final error message after exhausting all retries
instead of each time.

Thanks,
Davidlohr