cxl/region: Convert cxl_pmem_region_alloc to scope-based resource management

A recent bugfix to cxl_pmem_region_alloc() to fix an
error-unwind-memleak [1], highlighted a use case for scope-based resource
management.

Delete the goto for releasing @cxl_region_rwsem, and return error codes
directly from error condition paths.

The caller, devm_cxl_add_pmem_region(), is no longer given @cxlr_pmem
directly it must retrieve it from @cxlr->cxlr_pmem. This retrieval from
@cxlr was already in place for @cxlr->cxl_nvb, and converting
cxl_pmem_region_alloc() to return an int makes it less awkward to handle
no_free_ptr().

Cc: Li Zhijian <lizhijian@fujitsu.com>
Reported-by: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
Closes: http://lore.kernel.org/r/20240430174540.000039ce@Huawei.com
Link: http://lore.kernel.org/r/20240428030748.318985-1-lizhijian@fujitsu.com
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Link: https://lore.kernel.org/r/171451430965.1147997.15782562063090960666.stgit@dwillia2-xfh.jf.intel.com
Signed-off-by: Dave Jiang <dave.jiang@intel.com>

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 29b0b8a52d3d830895417b22a83e456cda733e45..00a9f0eef8dd2c6292b7772038cd7af6835d5dd3 100644 (file)
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -2816,26 +2816,21 @@ u64 cxl_trace_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd,
 
 static struct lock_class_key cxl_pmem_region_key;
 
-static struct cxl_pmem_region *cxl_pmem_region_alloc(struct cxl_region *cxlr)
+static int cxl_pmem_region_alloc(struct cxl_region *cxlr)
 {
        struct cxl_region_params *p = &cxlr->params;
        struct cxl_nvdimm_bridge *cxl_nvb;
-       struct cxl_pmem_region *cxlr_pmem;
        struct device *dev;
        int i;
 
-       down_read(&cxl_region_rwsem);
-       if (p->state != CXL_CONFIG_COMMIT) {
-               cxlr_pmem = ERR_PTR(-ENXIO);
-               goto out;
-       }
+       guard(rwsem_read)(&cxl_region_rwsem);
+       if (p->state != CXL_CONFIG_COMMIT)
+               return -ENXIO;
 
-       cxlr_pmem = kzalloc(struct_size(cxlr_pmem, mapping, p->nr_targets),
-                           GFP_KERNEL);
-       if (!cxlr_pmem) {
-               cxlr_pmem = ERR_PTR(-ENOMEM);
-               goto out;
-       }
+       struct cxl_pmem_region *cxlr_pmem __free(kfree) =
+               kzalloc(struct_size(cxlr_pmem, mapping, p->nr_targets), GFP_KERNEL);
+       if (!cxlr_pmem)
+               return -ENOMEM;
 
        cxlr_pmem->hpa_range.start = p->res->start;
        cxlr_pmem->hpa_range.end = p->res->end;
@@ -2853,11 +2848,8 @@ static struct cxl_pmem_region *cxl_pmem_region_alloc(struct cxl_region *cxlr)
                 */
                if (i == 0) {
                        cxl_nvb = cxl_find_nvdimm_bridge(cxlmd);
-                       if (!cxl_nvb) {
-                               kfree(cxlr_pmem);
-                               cxlr_pmem = ERR_PTR(-ENODEV);
-                               goto out;
-                       }
+                       if (!cxl_nvb)
+                               return -ENODEV;
                        cxlr->cxl_nvb = cxl_nvb;
                }
                m->cxlmd = cxlmd;
@@ -2868,18 +2860,16 @@ static struct cxl_pmem_region *cxl_pmem_region_alloc(struct cxl_region *cxlr)
        }
 
        dev = &cxlr_pmem->dev;
-       cxlr_pmem->cxlr = cxlr;
-       cxlr->cxlr_pmem = cxlr_pmem;
        device_initialize(dev);
        lockdep_set_class(&dev->mutex, &cxl_pmem_region_key);
        device_set_pm_not_required(dev);
        dev->parent = &cxlr->dev;
        dev->bus = &cxl_bus_type;
        dev->type = &cxl_pmem_region_type;
-out:
-       up_read(&cxl_region_rwsem);
+       cxlr_pmem->cxlr = cxlr;
+       cxlr->cxlr_pmem = no_free_ptr(cxlr_pmem);
 
-       return cxlr_pmem;
+       return 0;
 }
 
 static void cxl_dax_region_release(struct device *dev)
@@ -2996,9 +2986,10 @@ static int devm_cxl_add_pmem_region(struct cxl_region *cxlr)
        struct device *dev;
        int rc;
 
-       cxlr_pmem = cxl_pmem_region_alloc(cxlr);
-       if (IS_ERR(cxlr_pmem))
-               return PTR_ERR(cxlr_pmem);
+       rc = cxl_pmem_region_alloc(cxlr);
+       if (rc)
+               return rc;
+       cxlr_pmem = cxlr->cxlr_pmem;
        cxl_nvb = cxlr->cxl_nvb;
 
        dev = &cxlr_pmem->dev;