Re: [PATCH] hns3: work around stack size warning

[Jijie Shao <shaojijie@huawei.com>]

[-- Attachment #1: Type: text/plain, Size: 6531 bytes --]


on 2025/6/10 17:21, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> The hns3 debugfs functions all use an extra on-stack buffer to store
> temporary text output before copying that to the debugfs file.
>
> In some configurations with clang, this can trigger the warning limit
> for the total stack size:
>
>   drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c:788:12: error: stack frame size (1456) exceeds limit (1280) in 'hns3_dbg_tx_queue_info' [-Werror,-Wframe-larger-than]
>
> The problem here is that both hns3_dbg_tx_spare_info() and
> hns3_dbg_tx_queue_info() have a large on-stack buffer, and clang decides
> to inline them into a single function.

Hi Arnd:

Thank you for your report.

>
> Annotate hns3_dbg_tx_spare_info() as noinline_for_stack to force the
> behavior that gcc has, regardless of the compiler.
>
> Ideally all the functions in here would be changed to avoid on-stack
> output buffers.

Would you please help test whether the following changes have solved your problem,
And I'm not sure if this patch should be sent to net or net-next...


 From d8d1ec419d45411762dd1c8ba24510e5a40bad08 Mon Sep 17 00:00:00 2001
From: Jian Shen <shenjian15@huawei.com>
Date: Tue, 10 Jun 2025 19:35:19 +0800
Subject: [PATCH] net: hns3: clean up compile warning in debugfs

Arnd reported that there are two build warning for on-stasck
buffer oversize[1]. So use kmalloc instead of on-stack buffer.

1: https://lore.kernel.org/all/20250610092113.2639248-1-arnd@kernel.org/
Reported-by: Arnd Bergmann <arnd@kernel.org>
Signed-off-by: Jian Shen <shenjian15@huawei.com>
---
  .../ethernet/hisilicon/hns3/hns3_debugfs.c    | 37 ++++++++++++-------
  1 file changed, 24 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c b/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
index dd86a3f66040..0246d9ef26ab 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
@@ -592,10 +592,11 @@ static const struct hns3_dbg_item tx_spare_info_items[] = {
  static void hns3_dbg_tx_spare_info(struct hns3_enet_ring *ring, char *buf,
  				   int len, u32 ring_num, int *pos)
  {
-	char data_str[ARRAY_SIZE(tx_spare_info_items)][HNS3_DBG_DATA_STR_LEN];
+	size_t item_num = ARRAY_SIZE(tx_spare_info_items);
  	struct hns3_tx_spare *tx_spare = ring->tx_spare;
  	char *result[ARRAY_SIZE(tx_spare_info_items)];
  	char content[HNS3_DBG_INFO_LEN];
+	char *data_str;
  	u32 i, j;
  
  	if (!tx_spare) {
@@ -604,12 +605,16 @@ static void hns3_dbg_tx_spare_info(struct hns3_enet_ring *ring, char *buf,
  		return;
  	}
  
-	for (i = 0; i < ARRAY_SIZE(tx_spare_info_items); i++)
-		result[i] = &data_str[i][0];
+	data_str = kzalloc(item_num * HNS3_DBG_DATA_STR_LEN, GFP_KERNEL);
+	if (!data_str)
+		return;
+
+	for (i = 0; i < item_num; i++)
+		result[i] = &data_str[i * HNS3_DBG_DATA_STR_LEN];
  
  	*pos += scnprintf(buf + *pos, len - *pos, "tx spare buffer info\n");
  	hns3_dbg_fill_content(content, sizeof(content), tx_spare_info_items,
-			      NULL, ARRAY_SIZE(tx_spare_info_items));
+			      NULL, item_num);
  	*pos += scnprintf(buf + *pos, len - *pos, "%s", content);
  
  	for (i = 0; i < ring_num; i++) {
@@ -623,10 +628,11 @@ static void hns3_dbg_tx_spare_info(struct hns3_enet_ring *ring, char *buf,
  		sprintf(result[j++], "%pad", &tx_spare->dma);
  		hns3_dbg_fill_content(content, sizeof(content),
  				      tx_spare_info_items,
-				      (const char **)result,
-				      ARRAY_SIZE(tx_spare_info_items));
+				      (const char **)result, item_num);
  		*pos += scnprintf(buf + *pos, len - *pos, "%s", content);
  	}
+
+	kfree(data_str);
  }
  
  static const struct hns3_dbg_item rx_queue_info_items[] = {
@@ -793,12 +799,13 @@ static void hns3_dump_tx_queue_info(struct hns3_enet_ring *ring,
  static int hns3_dbg_tx_queue_info(struct hnae3_handle *h,
  				  char *buf, int len)
  {
-	char data_str[ARRAY_SIZE(tx_queue_info_items)][HNS3_DBG_DATA_STR_LEN];
+	size_t item_num = ARRAY_SIZE(tx_queue_info_items);
  	struct hnae3_ae_dev *ae_dev = hns3_get_ae_dev(h);
  	char *result[ARRAY_SIZE(tx_queue_info_items)];
  	struct hns3_nic_priv *priv = h->priv;
  	char content[HNS3_DBG_INFO_LEN];
  	struct hns3_enet_ring *ring;
+	char *data_str;
  	int pos = 0;
  	u32 i;
  
@@ -807,11 +814,15 @@ static int hns3_dbg_tx_queue_info(struct hnae3_handle *h,
  		return -EFAULT;
  	}
  
-	for (i = 0; i < ARRAY_SIZE(tx_queue_info_items); i++)
-		result[i] = &data_str[i][0];
+	data_str = kzalloc(item_num * HNS3_DBG_DATA_STR_LEN, GFP_KERNEL);
+	if (!data_str)
+		return -ENOMEM;
+
+	for (i = 0; i < item_num; i++)
+		result[i] = &data_str[i * HNS3_DBG_DATA_STR_LEN];
  
  	hns3_dbg_fill_content(content, sizeof(content), tx_queue_info_items,
-			      NULL, ARRAY_SIZE(tx_queue_info_items));
+			      NULL, item_num);
  	pos += scnprintf(buf + pos, len - pos, "%s", content);
  
  	for (i = 0; i < h->kinfo.num_tqps; i++) {
@@ -827,13 +838,13 @@ static int hns3_dbg_tx_queue_info(struct hnae3_handle *h,
  		hns3_dump_tx_queue_info(ring, ae_dev, result, i);
  		hns3_dbg_fill_content(content, sizeof(content),
  				      tx_queue_info_items,
-				      (const char **)result,
-				      ARRAY_SIZE(tx_queue_info_items));
+				      (const char **)result, item_num);
  		pos += scnprintf(buf + pos, len - pos, "%s", content);
  	}
  
-	hns3_dbg_tx_spare_info(ring, buf, len, h->kinfo.num_tqps, &pos);
+	kfree(data_str);
  
+	hns3_dbg_tx_spare_info(ring, buf, len, h->kinfo.num_tqps, &pos);
  	return 0;
  }
  
-- 
2.33.0


Thanks very much!

>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>   drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c b/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
> index 4e5d8bc39a1b..97dc47eeb44c 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
> @@ -580,8 +580,9 @@ static const struct hns3_dbg_item tx_spare_info_items[] = {
>   	{ "DMA", 17 },
>   };
>   
> -static void hns3_dbg_tx_spare_info(struct hns3_enet_ring *ring, char *buf,
> -				   int len, u32 ring_num, int *pos)
> +static noinline_for_stack void
> +hns3_dbg_tx_spare_info(struct hns3_enet_ring *ring, char *buf,
> +			int len, u32 ring_num, int *pos)
>   {
>   	char data_str[ARRAY_SIZE(tx_spare_info_items)][HNS3_DBG_DATA_STR_LEN];
>   	struct hns3_tx_spare *tx_spare = ring->tx_spare;

[-- Attachment #2: 0001-net-hns3-clean-up-compile-w.patch --]
[-- Type: text/plain, Size: 4407 bytes --]

From d8d1ec419d45411762dd1c8ba24510e5a40bad08 Mon Sep 17 00:00:00 2001
From: Jian Shen <shenjian15@huawei.com>
Date: Tue, 10 Jun 2025 19:35:19 +0800
Subject: [PATCH] net: hns3: clean up compile warning in debugfs

Arnd reported that there are two build warning for on-stasck
buffer oversize[1]. So use kmalloc instead of on-stack buffer.

1: https://lore.kernel.org/all/20250610092113.2639248-1-arnd@kernel.org/
Reported-by: Arnd Bergmann <arnd@kernel.org>
Signed-off-by: Jian Shen <shenjian15@huawei.com>
---
 .../ethernet/hisilicon/hns3/hns3_debugfs.c    | 37 ++++++++++++-------
 1 file changed, 24 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c b/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
index dd86a3f66040..0246d9ef26ab 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
@@ -592,10 +592,11 @@ static const struct hns3_dbg_item tx_spare_info_items[] = {
 static void hns3_dbg_tx_spare_info(struct hns3_enet_ring *ring, char *buf,
 				   int len, u32 ring_num, int *pos)
 {
-	char data_str[ARRAY_SIZE(tx_spare_info_items)][HNS3_DBG_DATA_STR_LEN];
+	size_t item_num = ARRAY_SIZE(tx_spare_info_items);
 	struct hns3_tx_spare *tx_spare = ring->tx_spare;
 	char *result[ARRAY_SIZE(tx_spare_info_items)];
 	char content[HNS3_DBG_INFO_LEN];
+	char *data_str;
 	u32 i, j;
 
 	if (!tx_spare) {
@@ -604,12 +605,16 @@ static void hns3_dbg_tx_spare_info(struct hns3_enet_ring *ring, char *buf,
 		return;
 	}
 
-	for (i = 0; i < ARRAY_SIZE(tx_spare_info_items); i++)
-		result[i] = &data_str[i][0];
+	data_str = kzalloc(item_num * HNS3_DBG_DATA_STR_LEN, GFP_KERNEL);
+	if (!data_str)
+		return;
+
+	for (i = 0; i < item_num; i++)
+		result[i] = &data_str[i * HNS3_DBG_DATA_STR_LEN];
 
 	*pos += scnprintf(buf + *pos, len - *pos, "tx spare buffer info\n");
 	hns3_dbg_fill_content(content, sizeof(content), tx_spare_info_items,
-			      NULL, ARRAY_SIZE(tx_spare_info_items));
+			      NULL, item_num);
 	*pos += scnprintf(buf + *pos, len - *pos, "%s", content);
 
 	for (i = 0; i < ring_num; i++) {
@@ -623,10 +628,11 @@ static void hns3_dbg_tx_spare_info(struct hns3_enet_ring *ring, char *buf,
 		sprintf(result[j++], "%pad", &tx_spare->dma);
 		hns3_dbg_fill_content(content, sizeof(content),
 				      tx_spare_info_items,
-				      (const char **)result,
-				      ARRAY_SIZE(tx_spare_info_items));
+				      (const char **)result, item_num);
 		*pos += scnprintf(buf + *pos, len - *pos, "%s", content);
 	}
+
+	kfree(data_str);
 }
 
 static const struct hns3_dbg_item rx_queue_info_items[] = {
@@ -793,12 +799,13 @@ static void hns3_dump_tx_queue_info(struct hns3_enet_ring *ring,
 static int hns3_dbg_tx_queue_info(struct hnae3_handle *h,
 				  char *buf, int len)
 {
-	char data_str[ARRAY_SIZE(tx_queue_info_items)][HNS3_DBG_DATA_STR_LEN];
+	size_t item_num = ARRAY_SIZE(tx_queue_info_items);
 	struct hnae3_ae_dev *ae_dev = hns3_get_ae_dev(h);
 	char *result[ARRAY_SIZE(tx_queue_info_items)];
 	struct hns3_nic_priv *priv = h->priv;
 	char content[HNS3_DBG_INFO_LEN];
 	struct hns3_enet_ring *ring;
+	char *data_str;
 	int pos = 0;
 	u32 i;
 
@@ -807,11 +814,15 @@ static int hns3_dbg_tx_queue_info(struct hnae3_handle *h,
 		return -EFAULT;
 	}
 
-	for (i = 0; i < ARRAY_SIZE(tx_queue_info_items); i++)
-		result[i] = &data_str[i][0];
+	data_str = kzalloc(item_num * HNS3_DBG_DATA_STR_LEN, GFP_KERNEL);
+	if (!data_str)
+		return -ENOMEM;
+
+	for (i = 0; i < item_num; i++)
+		result[i] = &data_str[i * HNS3_DBG_DATA_STR_LEN];
 
 	hns3_dbg_fill_content(content, sizeof(content), tx_queue_info_items,
-			      NULL, ARRAY_SIZE(tx_queue_info_items));
+			      NULL, item_num);
 	pos += scnprintf(buf + pos, len - pos, "%s", content);
 
 	for (i = 0; i < h->kinfo.num_tqps; i++) {
@@ -827,13 +838,13 @@ static int hns3_dbg_tx_queue_info(struct hnae3_handle *h,
 		hns3_dump_tx_queue_info(ring, ae_dev, result, i);
 		hns3_dbg_fill_content(content, sizeof(content),
 				      tx_queue_info_items,
-				      (const char **)result,
-				      ARRAY_SIZE(tx_queue_info_items));
+				      (const char **)result, item_num);
 		pos += scnprintf(buf + pos, len - pos, "%s", content);
 	}
 
-	hns3_dbg_tx_spare_info(ring, buf, len, h->kinfo.num_tqps, &pos);
+	kfree(data_str);
 
+	hns3_dbg_tx_spare_info(ring, buf, len, h->kinfo.num_tqps, &pos);
 	return 0;
 }
 
-- 
2.33.0