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

cxl/mem: Fix devm_cxl_memdev_edac_release() confusion 



A device release method is only for undoing allocations on the path to
preparing the device for device_add(). In contrast, devm allocations are
post device_add(), are acquired during / after ->probe() and are released
synchronous with ->remove().

So, a "devm" helper in a "release" method is a clear anti-pattern.

Move this devm release action where it belongs, an action created at edac
object creation time. Otherwise, this leaks resources until
cxl_memdev_release() time which may be long after these xarray and error
record caches have gone idle.

Note, this also fixes up the type of @cxlmd->err_rec_array which needlessly
dropped type-safety.

Fixes: 0b5ccb0d ("cxl/edac: Support for finding memory operation attributes from the current boot")
Cc: Dave Jiang <dave.jiang@intel.com>
Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Shiju Jose <shiju.jose@huawei.com>
Cc: Alison Schofield <alison.schofield@intel.com>
Reviewed-by: default avatarAlison Schofield <alison.schofield@intel.com>
Reviewed-by: default avatarBen Cheatham <benjamin.cheatham@amd.com>
Reviewed-by: default avatarDave Jiang <dave.jiang@intel.com>
Reviewed-by: default avatarJonathan Cameron <jonathan.cameron@huawei.com>
Tested-by: default avatarShiju Jose <shiju.jose@huawei.com>
Reviewed-by: default avatarShiju Jose <shiju.jose@huawei.com>
Tested-by: default avatarAlejandro Lucero <alucerop@amd.com>
Link: https://patch.msgid.link/20251216005616.3090129-2-dan.j.williams@intel.com


Signed-off-by: default avatarDan Williams <dan.j.williams@intel.com>
Signed-off-by: default avatarDave Jiang <dave.jiang@intel.com>
parent 9ace4753

drivers/cxl/core/edac.c

+37 −27
Original line number Diff line number Diff line
@@ -1988,6 +1988,40 @@ static int cxl_memdev_soft_ppr_init(struct cxl_memdev *cxlmd, 
	return 0;

}



static void err_rec_free(void *_cxlmd)

{

	struct cxl_memdev *cxlmd = _cxlmd;

	struct cxl_mem_err_rec *array_rec = cxlmd->err_rec_array;

	struct cxl_event_gen_media *rec_gen_media;

	struct cxl_event_dram *rec_dram;

	unsigned long index;



	cxlmd->err_rec_array = NULL;

	xa_for_each(&array_rec->rec_dram, index, rec_dram)

		kfree(rec_dram);

	xa_destroy(&array_rec->rec_dram);



	xa_for_each(&array_rec->rec_gen_media, index, rec_gen_media)

		kfree(rec_gen_media);

	xa_destroy(&array_rec->rec_gen_media);

	kfree(array_rec);

}



static int devm_cxl_memdev_setup_err_rec(struct cxl_memdev *cxlmd)

{

	struct cxl_mem_err_rec *array_rec =

		kzalloc(sizeof(*array_rec), GFP_KERNEL);



	if (!array_rec)

		return -ENOMEM;



	xa_init(&array_rec->rec_gen_media);

	xa_init(&array_rec->rec_dram);

	cxlmd->err_rec_array = array_rec;



	return devm_add_action_or_reset(&cxlmd->dev, err_rec_free, cxlmd);

}



int devm_cxl_memdev_edac_register(struct cxl_memdev *cxlmd)

{

	struct edac_dev_feature ras_features[CXL_NR_EDAC_DEV_FEATURES];
@@ -2038,15 +2072,9 @@ int devm_cxl_memdev_edac_register(struct cxl_memdev *cxlmd) 
		}



		if (repair_inst) {

			struct cxl_mem_err_rec *array_rec =

				devm_kzalloc(&cxlmd->dev, sizeof(*array_rec),

					     GFP_KERNEL);

			if (!array_rec)

				return -ENOMEM;



			xa_init(&array_rec->rec_gen_media);

			xa_init(&array_rec->rec_dram);

			cxlmd->err_rec_array = array_rec;

			rc = devm_cxl_memdev_setup_err_rec(cxlmd);

			if (rc)

				return rc;

		}

	}
@@ -2088,22 +2116,4 @@ int devm_cxl_region_edac_register(struct cxl_region *cxlr) 
}

EXPORT_SYMBOL_NS_GPL(devm_cxl_region_edac_register, "CXL");



void devm_cxl_memdev_edac_release(struct cxl_memdev *cxlmd)

{

	struct cxl_mem_err_rec *array_rec = cxlmd->err_rec_array;

	struct cxl_event_gen_media *rec_gen_media;

	struct cxl_event_dram *rec_dram;

	unsigned long index;



	if (!IS_ENABLED(CONFIG_CXL_EDAC_MEM_REPAIR) || !array_rec)

		return;

	xa_for_each(&array_rec->rec_dram, index, rec_dram)

		kfree(rec_dram);

	xa_destroy(&array_rec->rec_dram);



	xa_for_each(&array_rec->rec_gen_media, index, rec_gen_media)

		kfree(rec_gen_media);

	xa_destroy(&array_rec->rec_gen_media);

}

EXPORT_SYMBOL_NS_GPL(devm_cxl_memdev_edac_release, "CXL");

drivers/cxl/core/memdev.c

+0 −1
Original line number Diff line number Diff line
@@ -27,7 +27,6 @@ static void cxl_memdev_release(struct device *dev) 
	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);



	ida_free(&cxl_memdev_ida, cxlmd->id);

	devm_cxl_memdev_edac_release(cxlmd);

	kfree(cxlmd);

}

drivers/cxl/cxlmem.h

+1 −4
Original line number Diff line number Diff line
@@ -63,7 +63,7 @@ struct cxl_memdev { 
	int depth;

	u8 scrub_cycle;

	int scrub_region_id;

	void *err_rec_array;

	struct cxl_mem_err_rec *err_rec_array;

};



static inline struct cxl_memdev *to_cxl_memdev(struct device *dev)
@@ -877,7 +877,6 @@ int devm_cxl_memdev_edac_register(struct cxl_memdev *cxlmd); 
int devm_cxl_region_edac_register(struct cxl_region *cxlr);

int cxl_store_rec_gen_media(struct cxl_memdev *cxlmd, union cxl_event *evt);

int cxl_store_rec_dram(struct cxl_memdev *cxlmd, union cxl_event *evt);

void devm_cxl_memdev_edac_release(struct cxl_memdev *cxlmd);

#else

static inline int devm_cxl_memdev_edac_register(struct cxl_memdev *cxlmd)

{ return 0; }
@@ -889,8 +888,6 @@ static inline int cxl_store_rec_gen_media(struct cxl_memdev *cxlmd, 
static inline int cxl_store_rec_dram(struct cxl_memdev *cxlmd,

				     union cxl_event *evt)

{ return 0; }

static inline void devm_cxl_memdev_edac_release(struct cxl_memdev *cxlmd)

{ return; }

#endif



#ifdef CONFIG_CXL_SUSPEND