Commit 9fb9ce39 authored by Borislav Petkov's avatar Borislav Petkov
Browse files

EDAC/device: Get rid of the silly one-shot memory allocation in edac_device_alloc_ctl_info() 



Use boring kzalloc() instead. Add pointers to the different allocated
members in struct edac_device_ctl_info for easier freeing later.

One of the reasons, perhaps, why it was done this way is to be able to
do a single kfree(ctl_info) without having to kfree() the other parts of
the struct too but that is not nearly a sensible reason as to why there
should be this obscure pointer alignment.

There should be no functional changes resulting from this.

Signed-off-by: default avatarBorislav Petkov <bp@suse.de>
Link: https://lore.kernel.org/r/20220310095254.1510-4-bp@alien8.de
parent fb8cd45c

drivers/edac/edac_device.c

+42 −67
Original line number Diff line number Diff line
@@ -59,77 +59,52 @@ struct edac_device_ctl_info *edac_device_alloc_ctl_info( 
	struct edac_device_instance *dev_inst, *inst;

	struct edac_device_block *dev_blk, *blk_p, *blk;

	struct edac_dev_sysfs_block_attribute *dev_attrib, *attrib_p, *attrib;

	unsigned total_size;

	unsigned count;

	unsigned instance, block, attr;

	void *pvt, *p;

	void *pvt;

	int err;



	edac_dbg(4, "instances=%d blocks=%d\n", nr_instances, nr_blocks);



	/* Calculate the size of memory we need to allocate AND

	 * determine the offsets of the various item arrays

	 * (instance,block,attrib) from the start of an  allocated structure.

	 * We want the alignment of each item  (instance,block,attrib)

	 * to be at least as stringent as what the compiler would

	 * provide if we could simply hardcode everything into a single struct.

	 */

	p = NULL;

	dev_ctl = edac_align_ptr(&p, sizeof(*dev_ctl), 1);

	dev_ctl = kzalloc(sizeof(struct edac_device_ctl_info), GFP_KERNEL);

	if (!dev_ctl)

		return NULL;



	/* Calc the 'end' offset past end of ONE ctl_info structure

	 * which will become the start of the 'instance' array

	 */

	dev_inst = edac_align_ptr(&p, sizeof(*dev_inst), nr_instances);

	dev_inst = kmalloc_array(nr_instances,

				 sizeof(struct edac_device_instance),

				 GFP_KERNEL | __GFP_ZERO);

	if (!dev_inst)

		goto free;



	/* Calc the 'end' offset past the instance array within the ctl_info

	 * which will become the start of the block array

	 */

	count = nr_instances * nr_blocks;

	dev_blk = edac_align_ptr(&p, sizeof(*dev_blk), count);

	dev_ctl->instances = dev_inst;



	/* Calc the 'end' offset past the dev_blk array

	 * which will become the start of the attrib array, if any.

	 */

	/* calc how many nr_attrib we need */

	if (nr_attrib > 0)

		count *= nr_attrib;

	dev_attrib = edac_align_ptr(&p, sizeof(*dev_attrib), count);

	dev_blk = kmalloc_array(nr_instances * nr_blocks,

				sizeof(struct edac_device_block),

				GFP_KERNEL | __GFP_ZERO);

	if (!dev_blk)

		goto free;



	/* Calc the 'end' offset past the attributes array */

	pvt = edac_align_ptr(&p, sz_private, 1);

	dev_ctl->blocks = dev_blk;



	/* 'pvt' now points to where the private data area is.

	 * At this point 'pvt' (like dev_inst,dev_blk and dev_attrib)

	 * is baselined at ZERO

	 */

	total_size = ((unsigned long)pvt) + sz_private;

	if (nr_attrib) {

		dev_attrib = kmalloc_array(nr_attrib,

					   sizeof(struct edac_dev_sysfs_block_attribute),

					   GFP_KERNEL | __GFP_ZERO);

		if (!dev_attrib)

			goto free;



	/* Allocate the amount of memory for the set of control structures */

	dev_ctl = kzalloc(total_size, GFP_KERNEL);

	if (dev_ctl == NULL)

		return NULL;

		dev_ctl->attribs = dev_attrib;

	}



	if (sz_private) {

		pvt = kzalloc(sz_private, GFP_KERNEL);

		if (!pvt)

			goto free;



		dev_ctl->pvt_info = pvt;

	}



	/* Adjust pointers so they point within the actual memory we

	 * just allocated rather than an imaginary chunk of memory

	 * located at address 0.

	 * 'dev_ctl' points to REAL memory, while the others are

	 * ZERO based and thus need to be adjusted to point within

	 * the allocated memory.

	 */

	dev_inst = (struct edac_device_instance *)

		(((char *)dev_ctl) + ((unsigned long)dev_inst));

	dev_blk = (struct edac_device_block *)

		(((char *)dev_ctl) + ((unsigned long)dev_blk));

	dev_attrib = (struct edac_dev_sysfs_block_attribute *)

		(((char *)dev_ctl) + ((unsigned long)dev_attrib));

	pvt = sz_private ? (((char *)dev_ctl) + ((unsigned long)pvt)) : NULL;



	/* Begin storing the information into the control info structure */

	dev_ctl->dev_idx	= device_index;

	dev_ctl->nr_instances	= nr_instances;

	dev_ctl->instances = dev_inst;

	dev_ctl->pvt_info = pvt;



	/* Default logging of CEs and UEs */

	dev_ctl->log_ce = 1;
@@ -138,9 +113,6 @@ struct edac_device_ctl_info *edac_device_alloc_ctl_info( 
	/* Name of this edac device */

	snprintf(dev_ctl->name, sizeof(dev_ctl->name),"%s", edac_device_name);



	edac_dbg(4, "edac_dev=%p next after end=%p\n",

		 dev_ctl, pvt + sz_private);



	/* Initialize every Instance */

	for (instance = 0; instance < nr_instances; instance++) {

		inst = &dev_inst[instance];
@@ -210,10 +182,8 @@ struct edac_device_ctl_info *edac_device_alloc_ctl_info( 
	 * Initialize the 'root' kobj for the edac_device controller

	 */

	err = edac_device_register_sysfs_main_kobj(dev_ctl);

	if (err) {

		kfree(dev_ctl);

		return NULL;

	}

	if (err)

		goto free;



	/* at this point, the root kobj is valid, and in order to

	 * 'free' the object, then the function:
@@ -223,6 +193,11 @@ struct edac_device_ctl_info *edac_device_alloc_ctl_info( 
	 */



	return dev_ctl;



free:

	__edac_device_free_ctl_info(dev_ctl);



	return NULL;

}

EXPORT_SYMBOL_GPL(edac_device_alloc_ctl_info);

drivers/edac/edac_device.h

+14 −0
Original line number Diff line number Diff line
@@ -216,6 +216,8 @@ struct edac_device_ctl_info { 
	 */

	u32 nr_instances;

	struct edac_device_instance *instances;

	struct edac_device_block *blocks;

	struct edac_dev_sysfs_block_attribute *attribs;



	/* Event counters for the this whole EDAC Device */

	struct edac_device_counter counters;
@@ -348,4 +350,16 @@ edac_device_handle_ue(struct edac_device_ctl_info *edac_dev, int inst_nr, 
 */

extern int edac_device_alloc_index(void);

extern const char *edac_layer_name[];



/* Free the actual struct */

static inline void __edac_device_free_ctl_info(struct edac_device_ctl_info *ci)

{

	if (ci) {

		kfree(ci->pvt_info);

		kfree(ci->attribs);

		kfree(ci->blocks);

		kfree(ci->instances);

		kfree(ci);

	}

}

#endif

drivers/edac/edac_device_sysfs.c

+1 −4
Original line number Diff line number Diff line
@@ -208,10 +208,7 @@ static void edac_device_ctrl_master_release(struct kobject *kobj) 
	/* decrement the EDAC CORE module ref count */

	module_put(edac_dev->owner);



	/* free the control struct containing the 'main' kobj

	 * passed in to this routine

	 */

	kfree(edac_dev);

	__edac_device_free_ctl_info(edac_dev);

}



/* ktype for the main (master) kobject */