EDAC/device: Get rid of the silly one-shot memory allocation in edac_device_alloc_ctl...

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: Borislav Petkov <bp@suse.de>
Link: https://lore.kernel.org/r/20220310095254.1510-4-bp@alien8.de

diff --git a/drivers/edac/edac_device.c b/drivers/edac/edac_device.c
index 8c4d947fb8486bab37ab82b2a4937c97d52e4043..3c91156bbd2741437cb5b0fe22422be3bdae997b 100644 (file)
--- a/drivers/edac/edac_device.c
+++ b/drivers/edac/edac_device.c
@@ -59,87 +59,59 @@ 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;
+       }
 
-       /* 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;
+       if (sz_private) {
+               pvt = kzalloc(sz_private, GFP_KERNEL);
+               if (!pvt)
+                       goto free;
+
+               dev_ctl->pvt_info = pvt;
+       }
+
+       dev_ctl->dev_idx        = device_index;
+       dev_ctl->nr_instances   = nr_instances;
 
        /* Default logging of CEs and UEs */
        dev_ctl->log_ce = 1;
        dev_ctl->log_ue = 1;
 
        /* 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);
+       snprintf(dev_ctl->name, sizeof(dev_ctl->name),"%s", edac_device_name);
 
        /* Initialize every Instance */
        for (instance = 0; instance < nr_instances; 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);
 

diff --git a/drivers/edac/edac_device.h b/drivers/edac/edac_device.h
index fc2d2c2180649d239ad3befdb75a27782249e769..3f44e6b9d387f961d615db4a435edcfdc2faef11 100644 (file)
--- a/drivers/edac/edac_device.h
+++ b/drivers/edac/edac_device.h
@@ -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

diff --git a/drivers/edac/edac_device_sysfs.c b/drivers/edac/edac_device_sysfs.c
index 9a61d92bdf42045e6d02d62ee10b4febc1864e45..ac678b4a21fcbfd4d933d6be602594a80be81151 100644 (file)
--- a/drivers/edac/edac_device_sysfs.c
+++ b/drivers/edac/edac_device_sysfs.c
@@ -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 */