public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] EDAC/device: simplify allocation
@ 2026-04-30 22:00 Rosen Penev
  2026-04-30 22:00 ` [PATCH 1/2] EDAC/device: simplify info allocation Rosen Penev
  2026-04-30 22:00 ` [PATCH 2/2] EDAC/device: 3 to 1 allocations in edac_dev_feat_ctx Rosen Penev
  0 siblings, 2 replies; 5+ messages in thread
From: Rosen Penev @ 2026-04-30 22:00 UTC (permalink / raw)
  To: linux-edac
  Cc: Borislav Petkov, Tony Luck, Kees Cook, Gustavo A. R. Silva,
	open list,
	open list:KERNEL HARDENING (not covered by other areas):Keyword:b__counted_by(_le|_be)?b

Use a single allocation to remove a bunch of kfree calls.

Rosen Penev (2):
  EDAC/device: simplify info allocation
  EDAC/device: 3 to 1 allocations in edac_dev_feat_ctx

 drivers/edac/edac_device.c | 63 +++++++++++++-------------------------
 drivers/edac/edac_device.h |  4 +--
 include/linux/edac.h       |  2 +-
 3 files changed, 24 insertions(+), 45 deletions(-)

--
2.54.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] EDAC/device: simplify info allocation
  2026-04-30 22:00 [PATCH 0/2] EDAC/device: simplify allocation Rosen Penev
@ 2026-04-30 22:00 ` Rosen Penev
  2026-05-01 14:47   ` Zhuo, Qiuxu
  2026-04-30 22:00 ` [PATCH 2/2] EDAC/device: 3 to 1 allocations in edac_dev_feat_ctx Rosen Penev
  1 sibling, 1 reply; 5+ messages in thread
From: Rosen Penev @ 2026-04-30 22:00 UTC (permalink / raw)
  To: linux-edac
  Cc: Borislav Petkov, Tony Luck, Kees Cook, Gustavo A. R. Silva,
	open list,
	open list:KERNEL HARDENING (not covered by other areas):Keyword:b__counted_by(_le|_be)?b

Use a flexible array member to combine info and instance allocations.
Simplifies allocation slightly.

Add __counted_by for extra runtime analysis. Move counting variable
assignment to after allocation as kzalloc_flex already does with GCC >=
15.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/edac/edac_device.c | 23 ++++++++---------------
 drivers/edac/edac_device.h |  4 +---
 2 files changed, 9 insertions(+), 18 deletions(-)

diff --git a/drivers/edac/edac_device.c b/drivers/edac/edac_device.c
index cf0d3c2dfc04..9c21997a50e0 100644
--- a/drivers/edac/edac_device.c
+++ b/drivers/edac/edac_device.c
@@ -58,31 +58,25 @@ edac_device_alloc_ctl_info(unsigned pvt_sz, char *dev_name, unsigned nr_instance
 			   char *blk_name, unsigned nr_blocks, unsigned off_val,
 			   int device_index)
 {
-	struct edac_device_block *dev_blk, *blk_p, *blk;
+	struct edac_device_block *blk_p, *blk;
 	struct edac_device_instance *dev_inst, *inst;
 	struct edac_device_ctl_info *dev_ctl;
 	unsigned instance, block;
+	size_t alloc_size;
 	void *pvt;
 	int err;
 
 	edac_dbg(4, "instances=%d blocks=%d\n", nr_instances, nr_blocks);
 
-	dev_ctl = kzalloc_obj(struct edac_device_ctl_info);
+	alloc_size = struct_size(dev_ctl, instances, nr_instances);
+	alloc_size += sizeof(*dev_ctl->blocks) * nr_instances * nr_blocks;
+	dev_ctl = kzalloc(alloc_size, GFP_KERNEL);
 	if (!dev_ctl)
 		return NULL;
 
-	dev_inst = kzalloc_objs(struct edac_device_instance, nr_instances);
-	if (!dev_inst)
-		goto free;
-
-	dev_ctl->instances = dev_inst;
-
-	dev_blk = kzalloc_objs(struct edac_device_block,
-			       nr_instances * nr_blocks);
-	if (!dev_blk)
-		goto free;
+	dev_ctl->nr_instances = nr_instances;
 
-	dev_ctl->blocks = dev_blk;
+	dev_ctl->blocks = (struct edac_device_block *)(dev_ctl->instances + nr_instances);
 
 	if (pvt_sz) {
 		pvt = kzalloc(pvt_sz, GFP_KERNEL);
@@ -93,7 +87,6 @@ edac_device_alloc_ctl_info(unsigned pvt_sz, char *dev_name, unsigned nr_instance
 	}
 
 	dev_ctl->dev_idx	= device_index;
-	dev_ctl->nr_instances	= nr_instances;
 
 	/* Default logging of CEs and UEs */
 	dev_ctl->log_ce = 1;
@@ -107,7 +100,7 @@ edac_device_alloc_ctl_info(unsigned pvt_sz, char *dev_name, unsigned nr_instance
 		inst = &dev_inst[instance];
 		inst->ctl = dev_ctl;
 		inst->nr_blocks = nr_blocks;
-		blk_p = &dev_blk[instance * nr_blocks];
+		blk_p = &dev_ctl->blocks[instance * nr_blocks];
 		inst->blocks = blk_p;
 
 		/* name of this instance */
diff --git a/drivers/edac/edac_device.h b/drivers/edac/edac_device.h
index 24c1921aa490..4562461a3e8f 100644
--- a/drivers/edac/edac_device.h
+++ b/drivers/edac/edac_device.h
@@ -203,7 +203,6 @@ struct edac_device_ctl_info {
 	 * and the array of those instances
 	 */
 	u32 nr_instances;
-	struct edac_device_instance *instances;
 	struct edac_device_block *blocks;
 
 	/* Event counters for the this whole EDAC Device */
@@ -213,6 +212,7 @@ struct edac_device_ctl_info {
 	 * device this structure controls
 	 */
 	struct kobject kobj;
+	struct edac_device_instance instances[] __counted_by(nr_instances);
 };
 
 /* To get from the instance's wq to the beginning of the ctl structure */
@@ -341,8 +341,6 @@ static inline void __edac_device_free_ctl_info(struct edac_device_ctl_info *ci)
 {
 	if (ci) {
 		kfree(ci->pvt_info);
-		kfree(ci->blocks);
-		kfree(ci->instances);
 		kfree(ci);
 	}
 }
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] EDAC/device: 3 to 1 allocations in edac_dev_feat_ctx
  2026-04-30 22:00 [PATCH 0/2] EDAC/device: simplify allocation Rosen Penev
  2026-04-30 22:00 ` [PATCH 1/2] EDAC/device: simplify info allocation Rosen Penev
@ 2026-04-30 22:00 ` Rosen Penev
  2026-05-01 14:54   ` Zhuo, Qiuxu
  1 sibling, 1 reply; 5+ messages in thread
From: Rosen Penev @ 2026-04-30 22:00 UTC (permalink / raw)
  To: linux-edac
  Cc: Borislav Petkov, Tony Luck, Kees Cook, Gustavo A. R. Silva,
	open list,
	open list:KERNEL HARDENING (not covered by other areas):Keyword:b__counted_by(_le|_be)?b

Simplifies memory handling slightly by using a flexible array member.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/edac/edac_device.c | 40 +++++++++++++-------------------------
 include/linux/edac.h       |  2 +-
 2 files changed, 15 insertions(+), 27 deletions(-)

diff --git a/drivers/edac/edac_device.c b/drivers/edac/edac_device.c
index 9c21997a50e0..a6c41ce68f13 100644
--- a/drivers/edac/edac_device.c
+++ b/drivers/edac/edac_device.c
@@ -569,8 +569,6 @@ static void edac_dev_release(struct device *dev)
 {
 	struct edac_dev_feat_ctx *ctx = container_of(dev, struct edac_dev_feat_ctx, dev);
 
-	kfree(ctx->mem_repair);
-	kfree(ctx->scrub);
 	kfree(ctx->dev.groups);
 	kfree(ctx);
 }
@@ -612,6 +610,7 @@ int edac_dev_register(struct device *parent, char *name,
 	int attr_gcnt = 0;
 	int ret = -ENOMEM;
 	int scrub_cnt = 0;
+	size_t alloc_size;
 	int feat;
 
 	if (!parent || !name || !num_features || !ras_features)
@@ -636,26 +635,18 @@ int edac_dev_register(struct device *parent, char *name,
 		}
 	}
 
-	ctx = kzalloc_obj(*ctx);
+	alloc_size = struct_size(ctx, scrub, scrub_cnt);
+	alloc_size += sizeof(*ctx->mem_repair) * mem_repair_cnt;
+	ctx = kzalloc(alloc_size, GFP_KERNEL);
 	if (!ctx)
 		return -ENOMEM;
 
+	ctx->mem_repair = ctx->scrub + scrub_cnt;
+
 	ras_attr_groups = kzalloc_objs(*ras_attr_groups, attr_gcnt + 1);
 	if (!ras_attr_groups)
 		goto ctx_free;
 
-	if (scrub_cnt) {
-		ctx->scrub = kzalloc_objs(*ctx->scrub, scrub_cnt);
-		if (!ctx->scrub)
-			goto groups_free;
-	}
-
-	if (mem_repair_cnt) {
-		ctx->mem_repair = kzalloc_objs(*ctx->mem_repair, mem_repair_cnt);
-		if (!ctx->mem_repair)
-			goto data_mem_free;
-	}
-
 	attr_gcnt = 0;
 	scrub_cnt = 0;
 	mem_repair_cnt = 0;
@@ -664,7 +655,7 @@ int edac_dev_register(struct device *parent, char *name,
 		case RAS_FEAT_SCRUB:
 			if (!ras_features->scrub_ops || scrub_cnt != ras_features->instance) {
 				ret = -EINVAL;
-				goto data_mem_free;
+				goto groups_free;
 			}
 
 			dev_data = &ctx->scrub[scrub_cnt];
@@ -674,7 +665,7 @@ int edac_dev_register(struct device *parent, char *name,
 			ret = edac_scrub_get_desc(parent, &ras_attr_groups[attr_gcnt],
 						  ras_features->instance);
 			if (ret)
-				goto data_mem_free;
+				goto groups_free;
 
 			scrub_cnt++;
 			attr_gcnt++;
@@ -682,7 +673,7 @@ int edac_dev_register(struct device *parent, char *name,
 		case RAS_FEAT_ECS:
 			if (!ras_features->ecs_ops) {
 				ret = -EINVAL;
-				goto data_mem_free;
+				goto groups_free;
 			}
 
 			dev_data = &ctx->ecs;
@@ -691,7 +682,7 @@ int edac_dev_register(struct device *parent, char *name,
 			ret = edac_ecs_get_desc(parent, &ras_attr_groups[attr_gcnt],
 						ras_features->ecs_info.num_media_frus);
 			if (ret)
-				goto data_mem_free;
+				goto groups_free;
 
 			attr_gcnt += ras_features->ecs_info.num_media_frus;
 			break;
@@ -699,7 +690,7 @@ int edac_dev_register(struct device *parent, char *name,
 			if (!ras_features->mem_repair_ops ||
 			    mem_repair_cnt != ras_features->instance) {
 				ret = -EINVAL;
-				goto data_mem_free;
+				goto groups_free;
 			}
 
 			dev_data = &ctx->mem_repair[mem_repair_cnt];
@@ -709,14 +700,14 @@ int edac_dev_register(struct device *parent, char *name,
 			ret = edac_mem_repair_get_desc(parent, &ras_attr_groups[attr_gcnt],
 						       ras_features->instance);
 			if (ret)
-				goto data_mem_free;
+				goto groups_free;
 
 			mem_repair_cnt++;
 			attr_gcnt++;
 			break;
 		default:
 			ret = -EINVAL;
-			goto data_mem_free;
+			goto groups_free;
 		}
 	}
 
@@ -729,7 +720,7 @@ int edac_dev_register(struct device *parent, char *name,
 
 	ret = dev_set_name(&ctx->dev, "%s", name);
 	if (ret)
-		goto data_mem_free;
+		goto groups_free;
 
 	ret = device_register(&ctx->dev);
 	if (ret) {
@@ -739,9 +730,6 @@ int edac_dev_register(struct device *parent, char *name,
 
 	return devm_add_action_or_reset(parent, edac_dev_unreg, &ctx->dev);
 
-data_mem_free:
-	kfree(ctx->mem_repair);
-	kfree(ctx->scrub);
 groups_free:
 	kfree(ras_attr_groups);
 ctx_free:
diff --git a/include/linux/edac.h b/include/linux/edac.h
index 7a3c1c60dea7..b3ca9e3d0521 100644
--- a/include/linux/edac.h
+++ b/include/linux/edac.h
@@ -864,9 +864,9 @@ struct edac_dev_data {
 struct edac_dev_feat_ctx {
 	struct device dev;
 	void *private;
-	struct edac_dev_data *scrub;
 	struct edac_dev_data ecs;
 	struct edac_dev_data *mem_repair;
+	struct edac_dev_data scrub[];
 };
 
 struct edac_dev_feature {
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* RE: [PATCH 1/2] EDAC/device: simplify info allocation
  2026-04-30 22:00 ` [PATCH 1/2] EDAC/device: simplify info allocation Rosen Penev
@ 2026-05-01 14:47   ` Zhuo, Qiuxu
  0 siblings, 0 replies; 5+ messages in thread
From: Zhuo, Qiuxu @ 2026-05-01 14:47 UTC (permalink / raw)
  To: Rosen Penev, linux-edac@vger.kernel.org
  Cc: Borislav Petkov, Luck, Tony, Kees Cook, Gustavo A. R. Silva,
	open list,
	open list:KERNEL HARDENING (not covered by other areas):Keyword:\b__counted_by(_le|_be)?\b

> From: Rosen Penev <rosenp@gmail.com>
> Sent: Friday, May 1, 2026 6:01 AM
> To: linux-edac@vger.kernel.org
> Cc: Borislav Petkov <bp@alien8.de>; Luck, Tony <tony.luck@intel.com>; Kees
> Cook <kees@kernel.org>; Gustavo A. R. Silva <gustavoars@kernel.org>; open
> list <linux-kernel@vger.kernel.org>; open list:KERNEL HARDENING (not
> covered by other areas):Keyword:\b__counted_by(_le|_be)?\b <linux-
> hardening@vger.kernel.org>
> Subject: [PATCH 1/2] EDAC/device: simplify info allocation
> 
> Use a flexible array member to combine info and instance allocations.
> Simplifies allocation slightly.
> 
> Add __counted_by for extra runtime analysis. Move counting variable
> assignment to after allocation as kzalloc_flex already does with GCC >= 15.

I'm confused - I don't see kzalloc_flex()used in this patch.

> 
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  drivers/edac/edac_device.c | 23 ++++++++---------------
> drivers/edac/edac_device.h |  4 +---
>  2 files changed, 9 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/edac/edac_device.c b/drivers/edac/edac_device.c index
> cf0d3c2dfc04..9c21997a50e0 100644
> --- a/drivers/edac/edac_device.c
> +++ b/drivers/edac/edac_device.c
> @@ -58,31 +58,25 @@ edac_device_alloc_ctl_info(unsigned pvt_sz, char
> *dev_name, unsigned nr_instance
>  			   char *blk_name, unsigned nr_blocks, unsigned
> off_val,
>  			   int device_index)
>  {
> -	struct edac_device_block *dev_blk, *blk_p, *blk;
> +	struct edac_device_block *blk_p, *blk;
>  	struct edac_device_instance *dev_inst, *inst;
>  	struct edac_device_ctl_info *dev_ctl;
>  	unsigned instance, block;
> +	size_t alloc_size;
>  	void *pvt;
>  	int err;
> 
>  	edac_dbg(4, "instances=%d blocks=%d\n", nr_instances, nr_blocks);
> 
> -	dev_ctl = kzalloc_obj(struct edac_device_ctl_info);
> +	alloc_size = struct_size(dev_ctl, instances, nr_instances);
> +	alloc_size += sizeof(*dev_ctl->blocks) * nr_instances * nr_blocks;
> +	dev_ctl = kzalloc(alloc_size, GFP_KERNEL);
>  	if (!dev_ctl)
>  		return NULL;
> 
> -	dev_inst = kzalloc_objs(struct edac_device_instance, nr_instances);
> -	if (!dev_inst)
> -		goto free;
> -
> -	dev_ctl->instances = dev_inst;
> -
> -	dev_blk = kzalloc_objs(struct edac_device_block,
> -			       nr_instances * nr_blocks);
> -	if (!dev_blk)
> -		goto free;
> +	dev_ctl->nr_instances = nr_instances;
> 
> -	dev_ctl->blocks = dev_blk;
> +	dev_ctl->blocks = (struct edac_device_block *)(dev_ctl->instances +
> +nr_instances);
> 

1. This assumes that the address 
'dev_ctl->instances +nr_instances' is properly aligned for struct edac_device_block,
but that is not guaranteed.

2. Using a flexible array member for a single structure member can be reasonable and easy to read.
But this patch effectively turns two members into flexible arrays and requires manual address calculation for
one of them (and there is alignment risk).  This makes code less readable to me.
I'd prefer old code, as it's easier to follow ownership and no pointer alignment risk.

>  	if (pvt_sz) {
>  		pvt = kzalloc(pvt_sz, GFP_KERNEL);
> @@ -93,7 +87,6 @@ edac_device_alloc_ctl_info(unsigned pvt_sz, char
> *dev_name, unsigned nr_instance
>  	}
> 
>  	dev_ctl->dev_idx	= device_index;
> -	dev_ctl->nr_instances	= nr_instances;
> 
>  	/* Default logging of CEs and UEs */
>  	dev_ctl->log_ce = 1;
> @@ -107,7 +100,7 @@ edac_device_alloc_ctl_info(unsigned pvt_sz, char
> *dev_name, unsigned nr_instance
>  		inst = &dev_inst[instance];

BUG:  'dev_inst' is used w/o initialization. 

[...]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH 2/2] EDAC/device: 3 to 1 allocations in edac_dev_feat_ctx
  2026-04-30 22:00 ` [PATCH 2/2] EDAC/device: 3 to 1 allocations in edac_dev_feat_ctx Rosen Penev
@ 2026-05-01 14:54   ` Zhuo, Qiuxu
  0 siblings, 0 replies; 5+ messages in thread
From: Zhuo, Qiuxu @ 2026-05-01 14:54 UTC (permalink / raw)
  To: Rosen Penev, linux-edac@vger.kernel.org
  Cc: Borislav Petkov, Luck, Tony, Kees Cook, Gustavo A. R. Silva,
	open list,
	open list:KERNEL HARDENING (not covered by other areas):Keyword:\b__counted_by(_le|_be)?\b

> From: Rosen Penev <rosenp@gmail.com>
> Sent: Friday, May 1, 2026 6:01 AM
> To: linux-edac@vger.kernel.org
> Cc: Borislav Petkov <bp@alien8.de>; Luck, Tony <tony.luck@intel.com>; Kees
> Cook <kees@kernel.org>; Gustavo A. R. Silva <gustavoars@kernel.org>; open
> list <linux-kernel@vger.kernel.org>; open list:KERNEL HARDENING (not
> covered by other areas):Keyword:\b__counted_by(_le|_be)?\b <linux-
> hardening@vger.kernel.org>
> Subject: [PATCH 2/2] EDAC/device: 3 to 1 allocations in edac_dev_feat_ctx
> 
> Simplifies memory handling slightly by using a flexible array member.
> 
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  drivers/edac/edac_device.c | 40 +++++++++++++-------------------------
>  include/linux/edac.h       |  2 +-
>  2 files changed, 15 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/edac/edac_device.c b/drivers/edac/edac_device.c index
> 9c21997a50e0..a6c41ce68f13 100644
> --- a/drivers/edac/edac_device.c
> +++ b/drivers/edac/edac_device.c
> @@ -569,8 +569,6 @@ static void edac_dev_release(struct device *dev)  {
>  	struct edac_dev_feat_ctx *ctx = container_of(dev, struct
> edac_dev_feat_ctx, dev);
> 
> -	kfree(ctx->mem_repair);
> -	kfree(ctx->scrub);
>  	kfree(ctx->dev.groups);
>  	kfree(ctx);
>  }
> @@ -612,6 +610,7 @@ int edac_dev_register(struct device *parent, char
> *name,
>  	int attr_gcnt = 0;
>  	int ret = -ENOMEM;
>  	int scrub_cnt = 0;
> +	size_t alloc_size;
>  	int feat;
> 
>  	if (!parent || !name || !num_features || !ras_features) @@ -636,26
> +635,18 @@ int edac_dev_register(struct device *parent, char *name,
>  		}
>  	}
> 
> -	ctx = kzalloc_obj(*ctx);
> +	alloc_size = struct_size(ctx, scrub, scrub_cnt);
> +	alloc_size += sizeof(*ctx->mem_repair) * mem_repair_cnt;
> +	ctx = kzalloc(alloc_size, GFP_KERNEL);
>  	if (!ctx)
>  		return -ENOMEM;
> 
> +	ctx->mem_repair = ctx->scrub + scrub_cnt;

The same concerns about pointer alignment risk and code readability as in: 
https://lore.kernel.org/all/CY8PR11MB7134BF3800324EFA1B3172AA89322@CY8PR11MB7134.namprd11.prod.outlook.com/

-Qiuxu

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-05-01 14:55 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-30 22:00 [PATCH 0/2] EDAC/device: simplify allocation Rosen Penev
2026-04-30 22:00 ` [PATCH 1/2] EDAC/device: simplify info allocation Rosen Penev
2026-05-01 14:47   ` Zhuo, Qiuxu
2026-04-30 22:00 ` [PATCH 2/2] EDAC/device: 3 to 1 allocations in edac_dev_feat_ctx Rosen Penev
2026-05-01 14:54   ` Zhuo, Qiuxu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox