EDAC: mem_repair: reduce stack usage in edac_mem_repair_get_desc()

[PATCH] EDAC: mem_repair: reduce stack usage in edac_mem_repair_get_desc()

Posted by Arnd Bergmann 7 months, 3 weeks ago

From: Arnd Bergmann <arnd@arndb.de>

Constructing an array on the stack adds complexity and can exceed the
warning limit for per-function stack usage:

drivers/edac/mem_repair.c:361:5: error: stack frame size (1296) exceeds limit (1280) in 'edac_mem_repair_get_desc' [-Werror,-Wframe-larger-than]

Change this to have the actual attribute array allocated statically
and then just add the instance number on the per-instance copy.

Fixes: 699ea5219c4b ("EDAC: Add a memory repair control feature")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/edac/mem_repair.c | 56 +++++++++++++++------------------------
 1 file changed, 22 insertions(+), 34 deletions(-)

diff --git a/drivers/edac/mem_repair.c b/drivers/edac/mem_repair.c
index d1a8caa85369..2e4e790e0ffe 100755
--- a/drivers/edac/mem_repair.c
+++ b/drivers/edac/mem_repair.c
@@ -286,17 +286,26 @@ static umode_t mem_repair_attr_visible(struct kobject *kobj, struct attribute *a
 	return 0;
 }
 
-#define MR_ATTR_RO(_name, _instance)       \
-	((struct edac_mem_repair_dev_attr) { .dev_attr = __ATTR_RO(_name), \
-					     .instance = _instance })
-
-#define MR_ATTR_WO(_name, _instance)       \
-	((struct edac_mem_repair_dev_attr) { .dev_attr = __ATTR_WO(_name), \
-					     .instance = _instance })
-
-#define MR_ATTR_RW(_name, _instance)       \
-	((struct edac_mem_repair_dev_attr) { .dev_attr = __ATTR_RW(_name), \
-					     .instance = _instance })
+static const struct device_attribute mem_repair_dev_attr[] = {
+	[MR_TYPE]	  = __ATTR_RO(repair_type),
+	[MR_PERSIST_MODE] = __ATTR_RW(persist_mode),
+	[MR_SAFE_IN_USE]  = __ATTR_RO(repair_safe_when_in_use),
+	[MR_HPA]	  = __ATTR_RW(hpa),
+	[MR_MIN_HPA]	  = __ATTR_RO(min_hpa),
+	[MR_MAX_HPA]	  = __ATTR_RO(max_hpa),
+	[MR_DPA]	  = __ATTR_RW(dpa),
+	[MR_MIN_DPA]	  = __ATTR_RO(min_dpa),
+	[MR_MAX_DPA]	  = __ATTR_RO(max_dpa),
+	[MR_NIBBLE_MASK]  = __ATTR_RW(nibble_mask),
+	[MR_BANK_GROUP]   = __ATTR_RW(bank_group),
+	[MR_BANK]	  = __ATTR_RW(bank),
+	[MR_RANK]	  = __ATTR_RW(rank),
+	[MR_ROW]	  = __ATTR_RW(row),
+	[MR_COLUMN]	  = __ATTR_RW(column),
+	[MR_CHANNEL]	  = __ATTR_RW(channel),
+	[MR_SUB_CHANNEL]  = __ATTR_RW(sub_channel),
+	[MEM_DO_REPAIR]	  = __ATTR_WO(repair)
+};
 
 static int mem_repair_create_desc(struct device *dev,
 				  const struct attribute_group **attr_groups,
@@ -305,34 +314,13 @@ static int mem_repair_create_desc(struct device *dev,
 	struct edac_mem_repair_context *ctx;
 	struct attribute_group *group;
 	int i;
-	struct edac_mem_repair_dev_attr dev_attr[] = {
-		[MR_TYPE]	  = MR_ATTR_RO(repair_type, instance),
-		[MR_PERSIST_MODE] = MR_ATTR_RW(persist_mode, instance),
-		[MR_SAFE_IN_USE]  = MR_ATTR_RO(repair_safe_when_in_use, instance),
-		[MR_HPA]	  = MR_ATTR_RW(hpa, instance),
-		[MR_MIN_HPA]	  = MR_ATTR_RO(min_hpa, instance),
-		[MR_MAX_HPA]	  = MR_ATTR_RO(max_hpa, instance),
-		[MR_DPA]	  = MR_ATTR_RW(dpa, instance),
-		[MR_MIN_DPA]	  = MR_ATTR_RO(min_dpa, instance),
-		[MR_MAX_DPA]	  = MR_ATTR_RO(max_dpa, instance),
-		[MR_NIBBLE_MASK]  = MR_ATTR_RW(nibble_mask, instance),
-		[MR_BANK_GROUP]   = MR_ATTR_RW(bank_group, instance),
-		[MR_BANK]	  = MR_ATTR_RW(bank, instance),
-		[MR_RANK]	  = MR_ATTR_RW(rank, instance),
-		[MR_ROW]	  = MR_ATTR_RW(row, instance),
-		[MR_COLUMN]	  = MR_ATTR_RW(column, instance),
-		[MR_CHANNEL]	  = MR_ATTR_RW(channel, instance),
-		[MR_SUB_CHANNEL]  = MR_ATTR_RW(sub_channel, instance),
-		[MEM_DO_REPAIR]	  = MR_ATTR_WO(repair, instance)
-	};
-
 	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
 	if (!ctx)
 		return -ENOMEM;
 
 	for (i = 0; i < MR_MAX_ATTRS; i++) {
-		memcpy(&ctx->mem_repair_dev_attr[i],
-		       &dev_attr[i], sizeof(dev_attr[i]));
+		ctx->mem_repair_dev_attr[i].dev_attr = mem_repair_dev_attr[i];
+		ctx->mem_repair_dev_attr[i].instance = instance;
 		ctx->mem_repair_attrs[i] =
 			&ctx->mem_repair_dev_attr[i].dev_attr.attr;
 	}
-- 
2.39.5

RE: [PATCH] EDAC: mem_repair: reduce stack usage in edac_mem_repair_get_desc()

Posted by Shiju Jose 7 months, 1 week ago

>-----Original Message-----
>From: Arnd Bergmann <arnd@kernel.org>
>Sent: 20 June 2025 12:41
>To: Borislav Petkov <bp@alien8.de>; Tony Luck <tony.luck@intel.com>; Shiju
>Jose <shiju.jose@huawei.com>
>Cc: Arnd Bergmann <arnd@arndb.de>; James Morse <james.morse@arm.com>;
>Mauro Carvalho Chehab <mchehab@kernel.org>; Robert Richter
><rric@kernel.org>; Dave Jiang <dave.jiang@intel.com>; Alison Schofield
><alison.schofield@intel.com>; linux-edac@vger.kernel.org; linux-
>kernel@vger.kernel.org
>Subject: [PATCH] EDAC: mem_repair: reduce stack usage in
>edac_mem_repair_get_desc()
>
>From: Arnd Bergmann <arnd@arndb.de>
>
>Constructing an array on the stack adds complexity and can exceed the warning
>limit for per-function stack usage:
>
>drivers/edac/mem_repair.c:361:5: error: stack frame size (1296) exceeds limit
>(1280) in 'edac_mem_repair_get_desc' [-Werror,-Wframe-larger-than]
>
>Change this to have the actual attribute array allocated statically and then just
>add the instance number on the per-instance copy.
>
>Fixes: 699ea5219c4b ("EDAC: Add a memory repair control feature")
>Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Sorry, I missed this patch.
Tested-by: Shiju Jose <shiju.jose@huawei.com>
Reviewed-by: Shiju Jose <shiju.jose@huawei.com>
>---
> drivers/edac/mem_repair.c | 56 +++++++++++++++------------------------
> 1 file changed, 22 insertions(+), 34 deletions(-)
>
>diff --git a/drivers/edac/mem_repair.c b/drivers/edac/mem_repair.c index
>d1a8caa85369..2e4e790e0ffe 100755
>--- a/drivers/edac/mem_repair.c
>+++ b/drivers/edac/mem_repair.c
>@@ -286,17 +286,26 @@ static umode_t mem_repair_attr_visible(struct
>kobject *kobj, struct attribute *a
> 	return 0;
> }
>
>-#define MR_ATTR_RO(_name, _instance)       \
>-	((struct edac_mem_repair_dev_attr) { .dev_attr = __ATTR_RO(_name),
>\
>-					     .instance = _instance })
>-
>-#define MR_ATTR_WO(_name, _instance)       \
>-	((struct edac_mem_repair_dev_attr) { .dev_attr = __ATTR_WO(_name),
>\
>-					     .instance = _instance })
>-
>-#define MR_ATTR_RW(_name, _instance)       \
>-	((struct edac_mem_repair_dev_attr) { .dev_attr = __ATTR_RW(_name),
>\
>-					     .instance = _instance })
>+static const struct device_attribute mem_repair_dev_attr[] = {
>+	[MR_TYPE]	  = __ATTR_RO(repair_type),
>+	[MR_PERSIST_MODE] = __ATTR_RW(persist_mode),
>+	[MR_SAFE_IN_USE]  = __ATTR_RO(repair_safe_when_in_use),
>+	[MR_HPA]	  = __ATTR_RW(hpa),
>+	[MR_MIN_HPA]	  = __ATTR_RO(min_hpa),
>+	[MR_MAX_HPA]	  = __ATTR_RO(max_hpa),
>+	[MR_DPA]	  = __ATTR_RW(dpa),
>+	[MR_MIN_DPA]	  = __ATTR_RO(min_dpa),
>+	[MR_MAX_DPA]	  = __ATTR_RO(max_dpa),
>+	[MR_NIBBLE_MASK]  = __ATTR_RW(nibble_mask),
>+	[MR_BANK_GROUP]   = __ATTR_RW(bank_group),
>+	[MR_BANK]	  = __ATTR_RW(bank),
>+	[MR_RANK]	  = __ATTR_RW(rank),
>+	[MR_ROW]	  = __ATTR_RW(row),
>+	[MR_COLUMN]	  = __ATTR_RW(column),
>+	[MR_CHANNEL]	  = __ATTR_RW(channel),
>+	[MR_SUB_CHANNEL]  = __ATTR_RW(sub_channel),
>+	[MEM_DO_REPAIR]	  = __ATTR_WO(repair)
>+};
>
> static int mem_repair_create_desc(struct device *dev,
> 				  const struct attribute_group **attr_groups,
>@@ -305,34 +314,13 @@ static int mem_repair_create_desc(struct device
>*dev,
> 	struct edac_mem_repair_context *ctx;
> 	struct attribute_group *group;
> 	int i;
>-	struct edac_mem_repair_dev_attr dev_attr[] = {
>-		[MR_TYPE]	  = MR_ATTR_RO(repair_type, instance),
>-		[MR_PERSIST_MODE] = MR_ATTR_RW(persist_mode, instance),
>-		[MR_SAFE_IN_USE]  = MR_ATTR_RO(repair_safe_when_in_use,
>instance),
>-		[MR_HPA]	  = MR_ATTR_RW(hpa, instance),
>-		[MR_MIN_HPA]	  = MR_ATTR_RO(min_hpa, instance),
>-		[MR_MAX_HPA]	  = MR_ATTR_RO(max_hpa, instance),
>-		[MR_DPA]	  = MR_ATTR_RW(dpa, instance),
>-		[MR_MIN_DPA]	  = MR_ATTR_RO(min_dpa, instance),
>-		[MR_MAX_DPA]	  = MR_ATTR_RO(max_dpa, instance),
>-		[MR_NIBBLE_MASK]  = MR_ATTR_RW(nibble_mask, instance),
>-		[MR_BANK_GROUP]   = MR_ATTR_RW(bank_group, instance),
>-		[MR_BANK]	  = MR_ATTR_RW(bank, instance),
>-		[MR_RANK]	  = MR_ATTR_RW(rank, instance),
>-		[MR_ROW]	  = MR_ATTR_RW(row, instance),
>-		[MR_COLUMN]	  = MR_ATTR_RW(column, instance),
>-		[MR_CHANNEL]	  = MR_ATTR_RW(channel, instance),
>-		[MR_SUB_CHANNEL]  = MR_ATTR_RW(sub_channel, instance),
>-		[MEM_DO_REPAIR]	  = MR_ATTR_WO(repair, instance)
>-	};
>-
> 	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> 	if (!ctx)
> 		return -ENOMEM;
>
> 	for (i = 0; i < MR_MAX_ATTRS; i++) {
>-		memcpy(&ctx->mem_repair_dev_attr[i],
>-		       &dev_attr[i], sizeof(dev_attr[i]));
>+		ctx->mem_repair_dev_attr[i].dev_attr =
>mem_repair_dev_attr[i];
>+		ctx->mem_repair_dev_attr[i].instance = instance;
> 		ctx->mem_repair_attrs[i] =
> 			&ctx->mem_repair_dev_attr[i].dev_attr.attr;
> 	}
>--
>2.39.5
>

Re: [PATCH] EDAC: mem_repair: reduce stack usage in edac_mem_repair_get_desc()

Posted by Borislav Petkov 7 months, 2 weeks ago

On Fri, Jun 20, 2025 at 01:41:28PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> Constructing an array on the stack adds complexity and can exceed the
> warning limit for per-function stack usage:
> 
> drivers/edac/mem_repair.c:361:5: error: stack frame size (1296) exceeds limit (1280) in 'edac_mem_repair_get_desc' [-Werror,-Wframe-larger-than]
> 
> Change this to have the actual attribute array allocated statically
> and then just add the instance number on the per-instance copy.
> 
> Fixes: 699ea5219c4b ("EDAC: Add a memory repair control feature")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/edac/mem_repair.c | 56 +++++++++++++++------------------------
>  1 file changed, 22 insertions(+), 34 deletions(-)

Applied, thanks.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette