Commit d357dd8a authored by Dan Williams's avatar Dan Williams Committed by Dave Jiang
Browse files

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: default avatarJonathan 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: default avatarDan Williams <dan.j.williams@intel.com>
Reviewed-by: default avatarJonathan Cameron <Jonathan.Cameron@huawei.com>
Link: https://lore.kernel.org/r/171451430965.1147997.15782562063090960666.stgit@dwillia2-xfh.jf.intel.com


Signed-off-by: default avatarDave Jiang <dave.jiang@intel.com>
parent e4ff70a8

drivers/cxl/core/region.c

+17 −26
Original line number Diff line number Diff line
@@ -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;