From 4dd86ca99ffcc413cbf79063fd9956ef54e0ca91 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Tue, 19 May 2026 14:01:55 -0700 Subject: [PATCH] cxl/region: Resolve region deletion races Sungwoo noticed that the sysfs trigger to delete a region may try to delete a region multiple times. It also has no exclusion relative to the kernel releasing the region via CXL root device teardown. Instead of installing new cxl root devres actions per region, use the existing root decoder unregistration event to remove all remaining regions. An xarray of regions replaces a devres list of regions. This handles 3 separate issues with the old approach: 1/ sysfs users racing to delete the same region: no longer possible now that the regions_lock is held over the lookup and deletion. 2/ multiple actions triggering deletion of the same region: solved by erasing regions while holding @regions_lock, and only proceeding on successful erasure. 3/ userspace racing devres_release_all() to trigger the devres not found warning: solved by sysfs unregistration not requiring a release action Fixes: 779dd20cfb56 ("cxl/region: Add region creation support") Reported-by: Sungwoo Kim Closes: http://lore.kernel.org/20260427032010.916681-2-iam@sung-woo.kim Signed-off-by: Dan Williams Reviewed-by: Alejandro Lucero Tested-by: ALejandro Lucero Reviewed-by: Dave Jiang Link: https://patch.msgid.link/20260519210158.1499795-3-djbw@kernel.org Signed-off-by: Dave Jiang --- drivers/cxl/core/core.h | 2 ++ drivers/cxl/core/port.c | 5 ++++ drivers/cxl/core/region.c | 60 +++++++++++++++++++++------------------ drivers/cxl/cxl.h | 4 +++ 4 files changed, 44 insertions(+), 27 deletions(-) diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h index 82ca3a4767080..07555ae638594 100644 --- a/drivers/cxl/core/core.h +++ b/drivers/cxl/core/core.h @@ -52,6 +52,7 @@ u64 cxl_dpa_to_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd, u64 dpa); int devm_cxl_add_dax_region(struct cxl_region *cxlr); int devm_cxl_add_pmem_region(struct cxl_region *cxlr); +void kill_regions(struct cxl_root_decoder *cxlrd); #else static inline u64 cxl_dpa_to_hpa(struct cxl_region *cxlr, @@ -81,6 +82,7 @@ static inline int cxl_region_init(void) static inline void cxl_region_exit(void) { } +static inline void kill_regions(struct cxl_root_decoder *cxlrd) { }; #define CXL_REGION_ATTR(x) NULL #define CXL_REGION_TYPE(x) NULL #define SET_CXL_REGION_ATTR(x) diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c index 6e7a70d51cfe4..1215ee4f40351 100644 --- a/drivers/cxl/core/port.c +++ b/drivers/cxl/core/port.c @@ -458,6 +458,8 @@ static void cxl_root_decoder_release(struct device *dev) if (atomic_read(&cxlrd->region_id) >= 0) memregion_free(atomic_read(&cxlrd->region_id)); + mutex_destroy(&cxlrd->regions_lock); + xa_destroy(&cxlrd->regions); __cxl_decoder_release(&cxlrd->cxlsd.cxld); kfree(cxlrd); } @@ -2017,6 +2019,7 @@ struct cxl_root_decoder *cxl_root_decoder_alloc(struct cxl_port *port, } mutex_init(&cxlrd->regions_lock); + xa_init(&cxlrd->regions); cxld = &cxlsd->cxld; cxld->dev.type = &cxl_decoder_root_type; @@ -2192,6 +2195,8 @@ static void cxld_unregister(void *dev) if (is_endpoint_decoder(dev)) cxl_decoder_detach(NULL, to_cxl_endpoint_decoder(dev), -1, DETACH_INVALIDATE); + if (is_root_decoder(dev)) + kill_regions(to_cxl_root_decoder(dev)); device_unregister(dev); } diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c index b5601e89e302e..faf9785c05096 100644 --- a/drivers/cxl/core/region.c +++ b/drivers/cxl/core/region.c @@ -2537,12 +2537,13 @@ static struct cxl_region *to_cxl_region(struct device *dev) return container_of(dev, struct cxl_region, dev); } -static void unregister_region(void *_cxlr) +static void unregister_region(struct cxl_region *cxlr) { - struct cxl_region *cxlr = _cxlr; + struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(cxlr->dev.parent); struct cxl_region_params *p = &cxlr->params; int i; + xa_erase(&cxlrd->regions, cxlr->id); device_del(&cxlr->dev); /* @@ -2673,6 +2674,19 @@ static int cxl_region_calculate_adistance(struct notifier_block *nb, return NOTIFY_STOP; } +/* unwind all remaining regions */ +void kill_regions(struct cxl_root_decoder *cxlrd) +{ + unsigned long index; + struct cxl_region *cxlr; + + guard(mutex)(&cxlrd->regions_lock); + /* no more region creation */ + cxlrd->dead = true; + xa_for_each(&cxlrd->regions, index, cxlr) + unregister_region(cxlr); +} + /** * devm_cxl_add_region - Adds a region to a decoder * @cxlrd: root decoder @@ -2711,14 +2725,15 @@ static struct cxl_region *devm_cxl_add_region(struct cxl_root_decoder *cxlrd, if (rc) goto err; - rc = devm_add_action_or_reset(port->uport_dev, unregister_region, cxlr); - if (rc) + rc = xa_insert(&cxlrd->regions, cxlr->id, cxlr, GFP_KERNEL); + if (rc) { + unregister_region(cxlr); return ERR_PTR(rc); + } dev_dbg(port->uport_dev, "%s: created %s\n", dev_name(&cxlrd->cxlsd.cxld.dev), dev_name(dev)); return cxlr; - err: put_device(dev); return ERR_PTR(rc); @@ -2747,6 +2762,9 @@ static struct cxl_region *__create_region(struct cxl_root_decoder *cxlrd, { int rc; + if (cxlrd->dead) + return ERR_PTR(-ENXIO); + switch (mode) { case CXL_PARTMODE_RAM: case CXL_PARTMODE_PMEM: @@ -2822,38 +2840,27 @@ static ssize_t region_show(struct device *dev, struct device_attribute *attr, } DEVICE_ATTR_RO(region); -static struct cxl_region * -cxl_find_region_by_name(struct cxl_root_decoder *cxlrd, const char *name) -{ - struct cxl_decoder *cxld = &cxlrd->cxlsd.cxld; - struct device *region_dev; - - region_dev = device_find_child_by_name(&cxld->dev, name); - if (!region_dev) - return ERR_PTR(-ENODEV); - - return to_cxl_region(region_dev); -} - static ssize_t delete_region_store(struct device *dev, struct device_attribute *attr, const char *buf, size_t len) { struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(dev); - struct cxl_port *port = to_cxl_port(dev->parent); struct cxl_region *cxlr; - int rc; + int rc, id; ACQUIRE(mutex_intr, regions_lock)(&cxlrd->regions_lock); if ((rc = ACQUIRE_ERR(mutex_intr, ®ions_lock))) return rc; - cxlr = cxl_find_region_by_name(cxlrd, buf); - if (IS_ERR(cxlr)) - return PTR_ERR(cxlr); + rc = sscanf(buf, "region%d\n", &id); + if (rc != 1) + return -EINVAL; - devm_release_action(port->uport_dev, unregister_region, cxlr); - put_device(&cxlr->dev); + cxlr = xa_load(&cxlrd->regions, id); + if (!cxlr || !sysfs_streq(buf, dev_name(&cxlr->dev))) + return -ENODEV; + + unregister_region(cxlr); return len; } @@ -3718,7 +3725,6 @@ static struct cxl_region *construct_region(struct cxl_root_decoder *cxlrd, { struct cxl_endpoint_decoder *cxled = ctx->cxled; struct cxl_memdev *cxlmd = cxled_to_memdev(cxled); - struct cxl_port *port = cxlrd_to_port(cxlrd); struct cxl_dev_state *cxlds = cxlmd->cxlds; int rc, part = READ_ONCE(cxled->part); struct cxl_region *cxlr; @@ -3739,7 +3745,7 @@ static struct cxl_region *construct_region(struct cxl_root_decoder *cxlrd, rc = __construct_region(cxlr, ctx); if (rc) { - devm_release_action(port->uport_dev, unregister_region, cxlr); + unregister_region(cxlr); return ERR_PTR(rc); } diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h index 3900a0778571d..f43abd1903ce9 100644 --- a/drivers/cxl/cxl.h +++ b/drivers/cxl/cxl.h @@ -360,6 +360,8 @@ struct cxl_rd_ops { * @region_id: region id for next region provisioning event * @platform_data: platform specific configuration data * @regions_lock: sync region discovery, construction, and deletion + * @regions: regions to remove at root decoder destruct time + * @dead: root decoder dead to region creation * @qos_class: QoS performance class cookie * @ops: CXL root decoder operations * @cxlsd: base cxl switch decoder @@ -370,6 +372,8 @@ struct cxl_root_decoder { atomic_t region_id; void *platform_data; struct mutex regions_lock; + struct xarray regions; + bool dead; int qos_class; struct cxl_rd_ops ops; struct cxl_switch_decoder cxlsd; -- 2.47.3