Re: [PATCH v7 08/11] cxl: Coordinate sibling functions for CXL reset

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] AB-BA deadlock between `device_lock` and `pci_bus_sem` during CXL reset
- [Low] Mixed usage of goto-based error handling and scope-based cleanup helpers (`scoped_guard`)
--

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

cxl: Coordinate sibling functions for CXL reset

CXL Device Reset affects all CXL.cache and CXL.mem functions in the reset
scope. Lock same-scope siblings with pci_dev_trylock(), save/disable them,
drain pending transactions, and hold IOMMU reset blocks until recovery.

> diff --git a/drivers/cxl/core/reset.c b/drivers/cxl/core/reset.c
> index 1ae714a3595cb..69bcfab898589 100644
> --- a/drivers/cxl/core/reset.c
> +++ b/drivers/cxl/core/reset.c

[ ... ]

> @@ -944,11 +1045,24 @@ int cxl_reset_function(struct pci_dev *pdev, bool probe)

[Severity: High]
Could this lead to an AB-BA deadlock between the device lock and pci_bus_sem?

When the PCI core invokes a device-specific reset method, it holds the
target device's lock (device_lock(&pdev->dev)). If cxl_reset_function()
subsequently calls cxl_reset_collect_siblings() which uses pci_walk_bus(),
that will attempt to acquire a read lock on pci_bus_sem.

The standard PCI lock acquisition order is pci_bus_sem followed by
device_lock (for example, in pcie_do_recovery()). This inversion could
create a deadlock if another thread (like hotplug) requests a write lock
on pci_bus_sem, queuing the read lock request and blocking all involved
threads.

>  	if (rc)
>  		goto out;
>  
> +	rc = cxl_pci_functions_lock(&ctx);
> +	if (rc)
> +		goto out_unlock;
> +
> +	rc = cxl_pci_functions_reset_prepare(&ctx);
> +	if (rc)
> +		goto out_functions_done;
> +
>  	scoped_guard(rwsem_write, &cxl_rwsem.region) {
> -		rc = cxl_hdm_ranges_prepare(&range_ctx, pdev);
> +		rc = cxl_hdm_ranges_prepare(&range_ctx, &ctx);
>  		if (!rc)
>  			rc = cxl_reset_execute(pdev, dvsec);
>  	}
> +
> +out_functions_done:

[Severity: Low]
Does this mix goto-based error handling with scope-based cleanup helpers?

According to the cleanup guidelines, functions should not mix goto-based
cleanup labels (like out_functions_done) with scope-based helpers like
scoped_guard(). All resources needing a cleanup should either be converted
to scope-based cleanup, or none of them should use it.

> +	cxl_pci_functions_reset_done(&ctx);
> +out_unlock:
> +	cxl_pci_functions_unlock(&ctx);
>  out:
>  	cxl_hdm_range_context_destroy(&range_ctx);
>  	cxl_reset_context_destroy(&ctx);

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