[PATCH v2] cxl/region: Unregister the pmem region bridge on setup failure

Guixin Liu <[email protected]>
Newsgroups org.kernel.vger.linux-cxl
Message-ID <[email protected]>
devm_cxl_add_pmem_region() publishes the cxl_pmem_region with device_add()
and only afterwards, under the nvdimm bridge's device lock, arranges for
its removal - and only if the bridge has a driver bound. If it does not,
the function sets -ENXIO and leaves through err_bridge, which drops the
reference this function took on the bridge and returns. The device that
was just added has no owner at that point: no device_del(), no
put_device(), and no devm action to do either later. The sibling failure,
a devm_add_action_or_reset() that cannot allocate, is already covered,
because devm_add_action_or_reset() runs cxlr_pmem_unregister() itself on
that path.

An unbound bridge is a normal state, not an error state. The bridge is
unbound whenever cxl_pmem is unloaded or its device is detached through
sysfs, and a region can be probed in that window.

The added device then stays in sysfs, along with the reference it holds on
the region, until the module is unloaded. cxlr->cxlr_pmem still points at
it, and worse, the name is still taken: a later probe of the same region
allocates a second cxl_pmem_region and fails in device_add() on the
duplicate "pmem_region%d", so once this has happened the region can no
longer be brought up at all.

Call cxlr_pmem_unregister() on that branch. It is invoked from inside the
scoped_guard() that holds the bridge's device lock, which is what its
device_lock_assert() requires, and it performs the same teardown the devm
action would have performed, including clearing cxlr->cxlr_pmem, so
err_bridge is left with only the bridge reference to drop.

Fixes: f17b558d6663 ("cxl/pmem: Refactor nvdimm device registration, delete the workqueue")
Signed-off-by: Guixin Liu <[email protected]>
---
This 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.

v1->v2:
- rebase onto cxl/next
- rewrite the commit message to describe the behaviour rather than narrate
  the code change (Alison Schofield)

[1] https://lore.kernel.org/linux-cxl/[email protected]/

 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
-- 
2.43.7
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.