diff mbox series

[v4,16/17] cxl/pci: Simplify CDAT retrieval error path

Message ID 7a5c7104fb6a3016dbaec1c5d0ed34619ea11a0c.1678543498.git.lukas@wunner.de
State Accepted
Commit 7a877c923995b8257069209b1d757735af4f4ce0
Headers show
Series Collection of DOE material | expand

Commit Message

Lukas Wunner March 11, 2023, 2:40 p.m. UTC
From: Dave Jiang <dave.jiang@intel.com>

The cdat.table and cdat.length fields in struct cxl_port are set before
CDAT retrieval and must therefore be unset on failure.

Simplify by setting only on success.

Suggested-by: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
Link: https://lore.kernel.org/linux-cxl/20230209113422.00007017@Huawei.com/
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
[lukas: rebase and rephrase commit message]
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Reviewed-by: Ira Weiny <ira.weiny@intel.com>
---
Changes v3 -> v4:
 * Newly added patch in v4 on popular request (Jonathan, Dave)

 drivers/cxl/core/pci.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

Comments

Jonathan Cameron March 15, 2023, 4:48 p.m. UTC | #1
On Sat, 11 Mar 2023 15:40:16 +0100
Lukas Wunner <lukas@wunner.de> wrote:

> From: Dave Jiang <dave.jiang@intel.com>
> 
> The cdat.table and cdat.length fields in struct cxl_port are set before
> CDAT retrieval and must therefore be unset on failure.
> 
> Simplify by setting only on success.
> 
> Suggested-by: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
> Link: https://lore.kernel.org/linux-cxl/20230209113422.00007017@Huawei.com/
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> [lukas: rebase and rephrase commit message]
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> Reviewed-by: Ira Weiny <ira.weiny@intel.com>

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> ---
> Changes v3 -> v4:
>  * Newly added patch in v4 on popular request (Jonathan, Dave)
> 
>  drivers/cxl/core/pci.c | 23 ++++++++++++-----------
>  1 file changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> index c868f4a2f1de..0609bd629073 100644
> --- a/drivers/cxl/core/pci.c
> +++ b/drivers/cxl/core/pci.c
> @@ -475,10 +475,10 @@ static int cxl_cdat_get_length(struct device *dev,
>  
>  static int cxl_cdat_read_table(struct device *dev,
>  			       struct pci_doe_mb *cdat_doe,
> -			       struct cxl_cdat *cdat)
> +			       void *cdat_table, size_t *cdat_length)
>  {
> -	size_t length = cdat->length;
> -	__le32 *data = cdat->table;
> +	size_t length = *cdat_length;
> +	__le32 *data = cdat_table;
>  	int entry_handle = 0;
>  
>  	do {
> @@ -522,7 +522,7 @@ static int cxl_cdat_read_table(struct device *dev,
>  	} while (entry_handle != CXL_DOE_TABLE_ACCESS_LAST_ENTRY);
>  
>  	/* Length in CDAT header may exceed concatenation of CDAT entries */
> -	cdat->length -= length;
> +	*cdat_length -= length;
>  
>  	return 0;
>  }
> @@ -542,6 +542,7 @@ void read_cdat_data(struct cxl_port *port)
>  	struct cxl_dev_state *cxlds = cxlmd->cxlds;
>  	struct pci_dev *pdev = to_pci_dev(cxlds->dev);
>  	size_t cdat_length;
> +	void *cdat_table;
>  	int rc;
>  
>  	cdat_doe = pci_find_doe_mailbox(pdev, PCI_DVSEC_VENDOR_ID_CXL,
> @@ -558,19 +559,19 @@ void read_cdat_data(struct cxl_port *port)
>  		return;
>  	}
>  
> -	port->cdat.table = devm_kzalloc(dev, cdat_length, GFP_KERNEL);
> -	if (!port->cdat.table)
> +	cdat_table = devm_kzalloc(dev, cdat_length, GFP_KERNEL);
> +	if (!cdat_table)
>  		return;
>  
> -	port->cdat.length = cdat_length;
> -	rc = cxl_cdat_read_table(dev, cdat_doe, &port->cdat);
> +	rc = cxl_cdat_read_table(dev, cdat_doe, cdat_table, &cdat_length);
>  	if (rc) {
>  		/* Don't leave table data allocated on error */
> -		devm_kfree(dev, port->cdat.table);
> -		port->cdat.table = NULL;
> -		port->cdat.length = 0;
> +		devm_kfree(dev, cdat_table);
>  		dev_err(dev, "CDAT data read error\n");
>  	}
> +
> +	port->cdat.table = cdat_table;
> +	port->cdat.length = cdat_length;
>  }
>  EXPORT_SYMBOL_NS_GPL(read_cdat_data, CXL);
>
diff mbox series

Patch

diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
index c868f4a2f1de..0609bd629073 100644
--- a/drivers/cxl/core/pci.c
+++ b/drivers/cxl/core/pci.c
@@ -475,10 +475,10 @@  static int cxl_cdat_get_length(struct device *dev,
 
 static int cxl_cdat_read_table(struct device *dev,
 			       struct pci_doe_mb *cdat_doe,
-			       struct cxl_cdat *cdat)
+			       void *cdat_table, size_t *cdat_length)
 {
-	size_t length = cdat->length;
-	__le32 *data = cdat->table;
+	size_t length = *cdat_length;
+	__le32 *data = cdat_table;
 	int entry_handle = 0;
 
 	do {
@@ -522,7 +522,7 @@  static int cxl_cdat_read_table(struct device *dev,
 	} while (entry_handle != CXL_DOE_TABLE_ACCESS_LAST_ENTRY);
 
 	/* Length in CDAT header may exceed concatenation of CDAT entries */
-	cdat->length -= length;
+	*cdat_length -= length;
 
 	return 0;
 }
@@ -542,6 +542,7 @@  void read_cdat_data(struct cxl_port *port)
 	struct cxl_dev_state *cxlds = cxlmd->cxlds;
 	struct pci_dev *pdev = to_pci_dev(cxlds->dev);
 	size_t cdat_length;
+	void *cdat_table;
 	int rc;
 
 	cdat_doe = pci_find_doe_mailbox(pdev, PCI_DVSEC_VENDOR_ID_CXL,
@@ -558,19 +559,19 @@  void read_cdat_data(struct cxl_port *port)
 		return;
 	}
 
-	port->cdat.table = devm_kzalloc(dev, cdat_length, GFP_KERNEL);
-	if (!port->cdat.table)
+	cdat_table = devm_kzalloc(dev, cdat_length, GFP_KERNEL);
+	if (!cdat_table)
 		return;
 
-	port->cdat.length = cdat_length;
-	rc = cxl_cdat_read_table(dev, cdat_doe, &port->cdat);
+	rc = cxl_cdat_read_table(dev, cdat_doe, cdat_table, &cdat_length);
 	if (rc) {
 		/* Don't leave table data allocated on error */
-		devm_kfree(dev, port->cdat.table);
-		port->cdat.table = NULL;
-		port->cdat.length = 0;
+		devm_kfree(dev, cdat_table);
 		dev_err(dev, "CDAT data read error\n");
 	}
+
+	port->cdat.table = cdat_table;
+	port->cdat.length = cdat_length;
 }
 EXPORT_SYMBOL_NS_GPL(read_cdat_data, CXL);