Re: [PATCH v16 02/18] cxl: Share CXL port upstream PCI device lookup

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] cxl: mix of __free() and goto cleanup in read_cdat_data()
--

commit e44dda37d2191c0dc43154c400e85f444104b3ba
Author: Srirangan Madhavan <smadhavan@nvidia.com>

cxl: Share CXL port upstream PCI device lookup

This commit factors the lookup of a CXL port's upstream PCI device out of
read_cdat_data() into cxl_port_get_uport_pci_dev(), returning a referenced
pci_dev. It also updates read_cdat_data() to release the PCI device reference
using __free(pci_dev_put).

> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> index 9d807c1a002ca..c3c41efae5fde 100644
> --- a/drivers/cxl/core/pci.c
> +++ b/drivers/cxl/core/pci.c
> @@ -564,26 +564,14 @@ static unsigned char cdat_checksum(void *buf, size_t size)
>   */
>  void read_cdat_data(struct cxl_port *port)
>  {
> -	struct device *uport = port->uport_dev;
> +	struct pci_dev *pdev __free(pci_dev_put) =
> +		cxl_port_get_uport_pci_dev(port);

[Severity: Low]
In drivers/cxl/core/pci.c, read_cdat_data() now uses __free(pci_dev_put) for
pdev, while the remainder of the function still uses goto err on failure of
devm_kzalloc(), cxl_cdat_read_table(), or cdat_checksum() to free buf:

	buf = devm_kzalloc(dev, sizeof(*buf) + length, GFP_KERNEL);
	if (!buf)
		goto err;

	table_length = length;

	rc = cxl_cdat_read_table(dev, doe_mb, buf, &length);
	if (rc)
		goto err;
	...
	if (cdat_checksum(buf->data, length))
		goto err;
	...
err:
	/* Don't leave table data allocated on error */
	devm_kfree(dev, buf);
	dev_err(dev, "Failed to read/validate CDAT.\n");

Per include/linux/cleanup.h, goto-based cleanup and __free() scope-based
cleanup helpers should not be mixed within the same function:

 * Lastly, given that the benefit of cleanup helpers is removal of
 * "goto", and that the "goto" statement can jump between scopes, the
 * expectation is that usage of "goto" and cleanup helpers is never
 * mixed in the same function. I.e. for a given routine, convert all
 * resources that need a "goto" cleanup to scope-based cleanup, or
 * convert none of them.

Should read_cdat_data() either convert buf cleanup to avoid the err goto
label as well, or avoid mixing __free() with goto-based error handling here?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261029145703.2185231-1-smadhavan@nvidia.com?part=2