Re: [PATCH v3] cxl/region: Unregister the pmem region when the bridge is unbound

[Dave Jiang <dave.jiang@intel.com>]

On 8/30/26 11:11 PM, Guixin Liu wrote:
> Removing cxl_acpi unbinds the nvdimm bridge before the regions it
> serves. A region probe can therefore register its cxl_pmem_region and
> only then find the bridge unbound, which leaves the new device with
> nothing to remove it.
> 
> The orphan outlives the teardown and pins its region and its memdevs, so
> a later cxl_acpi bind renumbers the root port, ports, endpoints,
> decoders and the memdev.
> 
> Unregister the cxl_pmem_region when the bridge is unbound.
> 
> Found by code inspection. Racing a cxl_region bind against a cxl_acpi
> unbind leaked a pmem_region device on 12 of 12 attempts, and none with
> this patch.
> 
> Fixes: f17b558d6663 ("cxl/pmem: Refactor nvdimm device registration, delete the workqueue")
> Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
> Reviewed-by: Richard Cheng <icheng@nvidia.com>
> Reviewed-by: Li Ming <ming.li@zohomail.com>
> ---

Applied to cxl/next:
ef06f230f533

> Found by inspecting the CXL error paths, then confirmed on a QEMU CXL
> topology (4 memdevs behind a CXL switch, one of them with 2G of
> persistent capacity).
> 
> Test procedure, with a dev_err() temporarily added on the unbound-bridge
> branch so that taking it could be observed:
> 
>   cxl create-region -d decoderX.Y -m memN -t pmem -s 2G -w 1 -g 256
>   echo regionN > /sys/bus/cxl/drivers/cxl_region/unbind
>   echo regionN > /sys/bus/cxl/drivers/cxl_region/bind &
>   echo ACPI0017:00 > /sys/bus/platform/drivers/cxl_acpi/unbind &
>   wait
> 
> Without the patch all 12 attempts took the unbound-bridge branch. The
> region bind failed with -ENXIO and pmem_regionN stayed registered on the
> cxl bus after the teardown had removed everything else, including the
> bridge and the region it hangs off, leaving its /sys/bus/cxl/devices/
> link dangling into the deleted path. Because the orphan keeps references
> on its region and on the mapped memdev, the ancestor chain up to the CXL
> root port stayed pinned, so rebinding cxl_acpi renumbered the topology:
> root0 became root1, port1 became port2 and mem3 became mem4.
> 
> With the patch the same 12 attempts still reach that branch and the bind
> still fails with -ENXIO, but nothing is left registered and the numbering
> is stable across repeated bind and unbind cycles.
> 
> No artificial delay was needed to land in the window. The teardown
> releases the bridge first, so it gets there while the probe is still
> inside device_add().
> 
> v2 -> v3:
> - Reframe the changelog around cxl_acpi teardown, which is the reachable
>   path, rather than cxl_pmem being unloaded or unbound. The bridge driver
>   sets suppress_bind_attrs and cxl_acpi now depends on cxl_pmem, so that
>   framing could not happen (Alison Schofield)
> - Drop the -EEXIST claim. The leaked device pins the region id, so the id
>   is never reused and no duplicate name occurs; the observable
>   consequence is the leak and the renumbering after a rebind
> - Cut the implementation narration and state what was tested
>   (Alison Schofield)
> - The code is unchanged from v2, so Richard Cheng's and Li Ming's
>   Reviewed-by tags are carried over. Note that the changelog has been
>   rewritten since they gave them, in particular the -EEXIST claim above
> 
> v1 -> v2:
> - rebase onto cxl/next
> - rewrite the commit message to describe the behaviour rather than narrate
>   the code change (Alison Schofield)
> 
> v1 was patch 8/8 of the "cxl: Assorted fixes" series [1]. Per review
> feedback that series is not being reworked as a whole; the fixes are
> resent individually instead. Patches 1, 2 and 7 of the series are
> dropped, as those issues are already fixed in cxl/next.
> 
> [1] https://lore.kernel.org/linux-cxl/20260811113608.2815625-1-kanie@linux.alibaba.com/
> v2: https://lore.kernel.org/linux-cxl/20260812061043.57319-1-kanie@linux.alibaba.com/
> 
>  drivers/cxl/core/region_pmem.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cxl/core/region_pmem.c b/drivers/cxl/core/region_pmem.c
> index 23d97e3d78b6..7ab1373a95e0 100644
> --- a/drivers/cxl/core/region_pmem.c
> +++ b/drivers/cxl/core/region_pmem.c
> @@ -168,12 +168,14 @@ int devm_cxl_add_pmem_region(struct cxl_region *cxlr)
>  		dev_name(dev));
>  
>  	scoped_guard(device, &cxl_nvb->dev) {
> -		if (cxl_nvb->dev.driver)
> +		if (cxl_nvb->dev.driver) {
>  			rc = devm_add_action_or_reset(&cxl_nvb->dev,
>  						      cxlr_pmem_unregister,
>  						      cxlr_pmem);
> -		else
> +		} else {
>  			rc = -ENXIO;
> +			cxlr_pmem_unregister(cxlr_pmem);
> +		}
>  	}
>  
>  	if (rc)
> 
> base-commit: 7098e9cd98a05c0c5de2fae0c2465f9d966fdd07