[PATCH] hns3: work around stack size warning

[PATCH] hns3: work around stack size warning

[Arnd Bergmann]

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.

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.

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;
-- 
2.39.5

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

[Andrew Lunn]

On Tue, Jun 10, 2025 at 11:21:08AM +0200, 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.
> 
> Annotate hns3_dbg_tx_spare_info() as noinline_for_stack to force the
> behavior that gcc has, regardless of the compiler.

This warning is about the potential to exceed the stack space. That
potential still exists because of the tail call from
hns3_dbg_tx_queue_info() to hns3_dbg_tx_spare_info(), preventing the
compile from inlining does nothing against that.

> Ideally all the functions in here would be changed to avoid on-stack
> output buffers.

That would be my preference as well. Lets give Huawei a bit of time to
rewrite this code.

	Andrew

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

[Jijie Shao]

[-- 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

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

[Arnd Bergmann]

On Wed, Jun 11, 2025, at 04:10, Jijie Shao wrote:
> on 2025/6/10 17:21, Arnd Bergmann wrote:
>> From: Arnd Bergmann <arnd@arndb.de>
>>>>
>> 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...

Your patch arrived with whitespace corruption here, so I could not
try it, but I'm sure it would help avoid the warning.

However, this is not what meant with my suggestion: you already
allocate a temporary buffer in hns3_dbg_open() and I would
expect it to be possible to read into that buffer directly
without a second temporary buffer (on stack or kmalloc).

The normal way of doing this would be to use the infrastructure
from seq_file and then seq_printf() and not have any extra buffers
on top of that.

      Arnd

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

[Jijie Shao]

on 2025/6/12 0:36, Arnd Bergmann wrote:
> On Wed, Jun 11, 2025, at 04:10, Jijie Shao wrote:
>> on 2025/6/10 17:21, Arnd Bergmann wrote:
>>> From: Arnd Bergmann <arnd@arndb.de>
>>> 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...
> Your patch arrived with whitespace corruption here, so I could not
> try it, but I'm sure it would help avoid the warning.
>
> However, this is not what meant with my suggestion: you already
> allocate a temporary buffer in hns3_dbg_open() and I would
> expect it to be possible to read into that buffer directly
> without a second temporary buffer (on stack or kmalloc).
>
> The normal way of doing this would be to use the infrastructure
> from seq_file and then seq_printf() and not have any extra buffers
> on top of that.
>
>        Arnd


Hi,

seq_file is good. But the change is quite big.
I need to discuss it internally, and it may not be completed so quickly.
I will also need consider the maintainer's suggestion.

Thanks,
Jijie Shao

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

[Jakub Kicinski]

On Thu, 12 Jun 2025 21:09:40 +0800 Jijie Shao wrote:
> >> 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...  
> > Your patch arrived with whitespace corruption here, so I could not
> > try it, but I'm sure it would help avoid the warning.
> >
> > However, this is not what meant with my suggestion: you already
> > allocate a temporary buffer in hns3_dbg_open() and I would
> > expect it to be possible to read into that buffer directly
> > without a second temporary buffer (on stack or kmalloc).
> >
> > The normal way of doing this would be to use the infrastructure
> > from seq_file and then seq_printf() and not have any extra buffers
> > on top of that.
> 
> seq_file is good. But the change is quite big.
> I need to discuss it internally, and it may not be completed so quickly.
> I will also need consider the maintainer's suggestion.

Arnd, do you mind waiting? We can apply your patch as is if the warning
is a nuisance.

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

[Arnd Bergmann]

On Thu, Jun 12, 2025, at 16:32, Jakub Kicinski wrote:
> On Thu, 12 Jun 2025 21:09:40 +0800 Jijie Shao wrote:
>> > The normal way of doing this would be to use the infrastructure
>> > from seq_file and then seq_printf() and not have any extra buffers
>> > on top of that.
>> 
>> seq_file is good. But the change is quite big.
>> I need to discuss it internally, and it may not be completed so quickly.
>> I will also need consider the maintainer's suggestion.
>
> Arnd, do you mind waiting? We can apply your patch as is if the warning
> is a nuisance.

It's not urgent, as the current warning limit is still 2048 bytes
for 64-bit targets. I am hoping to have all patches in place for
the next merge window to be able to reduce the default limit to
1280 bytes and I can just carry my local fix for my own testing
until then.

    Arnd

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

[Jakub Kicinski]

On Thu, 12 Jun 2025 21:09:40 +0800 Jijie Shao wrote:
> seq_file is good. But the change is quite big.
> I need to discuss it internally, and it may not be completed so quickly.
> I will also need consider the maintainer's suggestion.

Please work on the seq_file conversion, given that the merge window
just closed you have around 6 weeks to get it done, so hopefully plenty
of time.

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

[Jijie Shao]

on 2025/6/12 23:33, Jakub Kicinski wrote:
> On Thu, 12 Jun 2025 21:09:40 +0800 Jijie Shao wrote:
>> seq_file is good. But the change is quite big.
>> I need to discuss it internally, and it may not be completed so quickly.
>> I will also need consider the maintainer's suggestion.
> Please work on the seq_file conversion, given that the merge window
> just closed you have around 6 weeks to get it done, so hopefully plenty
> of time.

Ok

I will try to send patch as soon as possible to complete this conversion


Thanks
Jijie Shao

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

[Jijie Shao]

on 2025/6/13 13:59, Jijie Shao wrote:
>
> on 2025/6/12 23:33, Jakub Kicinski wrote:
>> On Thu, 12 Jun 2025 21:09:40 +0800 Jijie Shao wrote:
>>> seq_file is good. But the change is quite big.
>>> I need to discuss it internally, and it may not be completed so 
>>> quickly.
>>> I will also need consider the maintainer's suggestion.
>> Please work on the seq_file conversion, given that the merge window
>> just closed you have around 6 weeks to get it done, so hopefully plenty
>> of time.
>
> Ok
>
> I will try to send patch as soon as possible to complete this conversion
>
>
> Thanks
> Jijie Shao
>
>
We tried modifying the relevant debugfs files,
and both compilation and tests passed.
Please help us check if the solution is OK.
If it is OK, we will modify all the debugfs files in this way.

====================

 From c62568f3e91eb5725211fde8e63d44f68452b4e3 Mon Sep 17 00:00:00 2001
From: Jian Shen <shenjian15@huawei.com>
Date: Thu, 19 Jun 2025 16:21:17 +0800
Subject: [PATCH  net-next] net: hns3: clean up the build warning in debugfs by
  use seq file

Arnd reported that there are two build warning for on-stasck
buffer oversize[1]. As Arnd's suggestion, using seq file way
to avoid the stack buffer or kmalloc buffer allocating.

Reported-by: Arnd Bergmann <arnd@kernel.org>
Closes: https://lore.kernel.org/all/20250610092113.2639248-1-arnd@kernel.org/
Signed-off-by: Jian Shen <shenjian15@huawei.com>
Signed-off-by: Jijie Shao <shaojijie@huawei.com>
---
  drivers/net/ethernet/hisilicon/hns3/hnae3.h   |   3 +
  .../ethernet/hisilicon/hns3/hns3_debugfs.c    | 160 ++++++++----------
  .../net/ethernet/hisilicon/hns3/hns3_enet.c   |   2 +
  3 files changed, 79 insertions(+), 86 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hnae3.h b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
index 8dc7d6fae224..58a63d2eb69b 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hnae3.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
@@ -434,8 +434,11 @@ struct hnae3_ae_dev {
  	u32 dev_version;
  	DECLARE_BITMAP(caps, HNAE3_DEV_CAPS_MAX_NUM);
  	void *priv;
+	struct hnae3_handle *handle;
  };
  
+typedef int (*read_func)(struct seq_file *s, void *data);
+
  /* This struct defines the operation on the handle.
   *
   * init_ae_dev(): (mandatory)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c b/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
index 4e5d8bc39a1b..458a2944ee3c 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
@@ -3,6 +3,7 @@
  
  #include <linux/debugfs.h>
  #include <linux/device.h>
+#include <linux/seq_file.h>
  #include <linux/string_choices.h>
  
  #include "hnae3.h"
@@ -41,6 +42,7 @@ static struct hns3_dbg_dentry_info hns3_dbg_dentry[] = {
  
  static int hns3_dbg_bd_file_init(struct hnae3_handle *handle, u32 cmd);
  static int hns3_dbg_common_file_init(struct hnae3_handle *handle, u32 cmd);
+static int hns3_dbg_common_init_t1(struct hnae3_handle *handle, u32 cmd);
  
  static struct hns3_dbg_cmd_info hns3_dbg_cmd[] = {
  	{
@@ -300,7 +302,7 @@ static struct hns3_dbg_cmd_info hns3_dbg_cmd[] = {
  		.cmd = HNAE3_DBG_CMD_TX_QUEUE_INFO,
  		.dentry = HNS3_DBG_DENTRY_QUEUE,
  		.buf_len = HNS3_DBG_READ_LEN_1MB,
-		.init = hns3_dbg_common_file_init,
+		.init = hns3_dbg_common_init_t1,
  	},
  	{
  		.name = "fd_tcam",
@@ -580,44 +582,29 @@ 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 void hns3_dbg_tx_spare_info(struct hns3_enet_ring *ring,
+				   struct seq_file *s, u32 ring_num)
  {
-	char data_str[ARRAY_SIZE(tx_spare_info_items)][HNS3_DBG_DATA_STR_LEN];
  	struct hns3_tx_spare *tx_spare = ring->tx_spare;
-	char *result[ARRAY_SIZE(tx_spare_info_items)];
-	char content[HNS3_DBG_INFO_LEN];
-	u32 i, j;
+	u32 i;
  
  	if (!tx_spare) {
-		*pos += scnprintf(buf + *pos, len - *pos,
-				  "tx spare buffer is not enabled\n");
+		seq_puts(s, "tx spare buffer is not enabled\n");
  		return;
  	}
  
-	for (i = 0; i < ARRAY_SIZE(tx_spare_info_items); i++)
-		result[i] = &data_str[i][0];
-
-	*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));
-	*pos += scnprintf(buf + *pos, len - *pos, "%s", content);
+	seq_puts(s, "tx spare buffer info\n");
  
-	for (i = 0; i < ring_num; i++) {
-		j = 0;
-		sprintf(result[j++], "%u", i);
-		sprintf(result[j++], "%u", ring->tx_copybreak);
-		sprintf(result[j++], "%u", tx_spare->len);
-		sprintf(result[j++], "%u", tx_spare->next_to_use);
-		sprintf(result[j++], "%u", tx_spare->next_to_clean);
-		sprintf(result[j++], "%u", tx_spare->last_to_clean);
-		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));
-		*pos += scnprintf(buf + *pos, len - *pos, "%s", content);
-	}
+	for (i = 0; i < ARRAY_SIZE(tx_spare_info_items); i++)
+		seq_printf(s, "%s%*s", tx_spare_info_items[i].name,
+			   tx_spare_info_items[i].interval, " ");
+	seq_puts(s, "\n");
+
+	for (i = 0; i < ring_num; i++)
+		seq_printf(s, "%-4u%6s%-5u%6s%-8u%2s%-5u%2s%-5u%2s%-5u%2s%pad\n",
+			   i, " ", ring->tx_copybreak, " ", tx_spare->len, " ",
+			   tx_spare->next_to_use, " ", tx_spare->next_to_clean,
+			   " ", tx_spare->last_to_clean, " ", &tx_spare->dma);
  }
  
  static const struct hns3_dbg_item rx_queue_info_items[] = {
@@ -739,62 +726,52 @@ static const struct hns3_dbg_item tx_queue_info_items[] = {
  };
  
  static void hns3_dump_tx_queue_info(struct hns3_enet_ring *ring,
-				    struct hnae3_ae_dev *ae_dev, char **result,
-				    u32 index)
+				    struct hnae3_ae_dev *ae_dev,
+				    struct seq_file *s, u32 index)
  {
+	void __iomem *base = ring->tqp->io_base;
  	u32 base_add_l, base_add_h;
-	u32 j = 0;
-
-	sprintf(result[j++], "%u", index);
-	sprintf(result[j++], "%u", readl_relaxed(ring->tqp->io_base +
-		HNS3_RING_TX_RING_BD_NUM_REG));
-
-	sprintf(result[j++], "%u", readl_relaxed(ring->tqp->io_base +
-		HNS3_RING_TX_RING_TC_REG));
-
-	sprintf(result[j++], "%u", readl_relaxed(ring->tqp->io_base +
-		HNS3_RING_TX_RING_TAIL_REG));
-
-	sprintf(result[j++], "%u", readl_relaxed(ring->tqp->io_base +
-		HNS3_RING_TX_RING_HEAD_REG));
-
-	sprintf(result[j++], "%u", readl_relaxed(ring->tqp->io_base +
-		HNS3_RING_TX_RING_FBDNUM_REG));
-
-	sprintf(result[j++], "%u", readl_relaxed(ring->tqp->io_base +
-		HNS3_RING_TX_RING_OFFSET_REG));
-
-	sprintf(result[j++], "%u", readl_relaxed(ring->tqp->io_base +
-		HNS3_RING_TX_RING_PKTNUM_RECORD_REG));
  
-	sprintf(result[j++], "%s",
-		str_on_off(readl_relaxed(ring->tqp->io_base +
-					 HNS3_RING_EN_REG)));
+	seq_printf(s, "%-4u%6s", index, " ");
+	seq_printf(s, "%-5u%3s",
+		   readl_relaxed(base + HNS3_RING_TX_RING_BD_NUM_REG), " ");
+	seq_printf(s, "%u%3s",
+		   readl_relaxed(base + HNS3_RING_TX_RING_TC_REG), " ");
+	seq_printf(s, "%-4u%2s",
+		   readl_relaxed(base + HNS3_RING_TX_RING_TAIL_REG), " ");
+	seq_printf(s, "%-4u%2s",
+		   readl_relaxed(base + HNS3_RING_TX_RING_HEAD_REG), " ");
+	seq_printf(s, "%-4u%4s",
+		   readl_relaxed(base + HNS3_RING_TX_RING_FBDNUM_REG), " ");
+	seq_printf(s, "%-4u%4s",
+		   readl_relaxed(base + HNS3_RING_TX_RING_OFFSET_REG), " ");
+	seq_printf(s, "%-9u%2s",
+		   readl_relaxed(base + HNS3_RING_TX_RING_PKTNUM_RECORD_REG),
+		   " ");
+	seq_printf(s, "%-3s%6s",
+		   str_on_off(readl_relaxed(base + HNS3_RING_EN_REG)), " ");
  
  	if (hnae3_ae_dev_tqp_txrx_indep_supported(ae_dev))
-		sprintf(result[j++], "%s",
-			str_on_off(readl_relaxed(ring->tqp->io_base +
-						 HNS3_RING_TX_EN_REG)));
+		seq_printf(s, "%-3s%9s",
+			   str_on_off(readl_relaxed(base +
+						    HNS3_RING_TX_EN_REG)),
+			   " ");
  	else
-		sprintf(result[j++], "%s", "NA");
+		seq_printf(s, "%-3s%9s", "NA", " ");
  
  	base_add_h = readl_relaxed(ring->tqp->io_base +
  					HNS3_RING_TX_RING_BASEADDR_H_REG);
  	base_add_l = readl_relaxed(ring->tqp->io_base +
  					HNS3_RING_TX_RING_BASEADDR_L_REG);
-	sprintf(result[j++], "0x%08x%08x", base_add_h, base_add_l);
+	seq_printf(s, "0x%08x%08x\n", base_add_h, base_add_l);
  }
  
-static int hns3_dbg_tx_queue_info(struct hnae3_handle *h,
-				  char *buf, int len)
+static int hns3_dbg_tx_queue_info(struct seq_file *s, void *data)
  {
-	char data_str[ARRAY_SIZE(tx_queue_info_items)][HNS3_DBG_DATA_STR_LEN];
-	struct hnae3_ae_dev *ae_dev = pci_get_drvdata(h->pdev);
-	char *result[ARRAY_SIZE(tx_queue_info_items)];
+	struct hnae3_ae_dev *ae_dev = dev_get_drvdata(s->private);
+	struct hnae3_handle *h = ae_dev->handle;
  	struct hns3_nic_priv *priv = h->priv;
-	char content[HNS3_DBG_INFO_LEN];
  	struct hns3_enet_ring *ring;
-	int pos = 0;
  	u32 i;
  
  	if (!priv->ring) {
@@ -803,11 +780,10 @@ static int hns3_dbg_tx_queue_info(struct hnae3_handle *h,
  	}
  
  	for (i = 0; i < ARRAY_SIZE(tx_queue_info_items); i++)
-		result[i] = &data_str[i][0];
+		seq_printf(s, "%s%*s", tx_queue_info_items[i].name,
+			   tx_queue_info_items[i].interval, " ");
  
-	hns3_dbg_fill_content(content, sizeof(content), tx_queue_info_items,
-			      NULL, ARRAY_SIZE(tx_queue_info_items));
-	pos += scnprintf(buf + pos, len - pos, "%s", content);
+	seq_puts(s, "\n");
  
  	for (i = 0; i < h->kinfo.num_tqps; i++) {
  		/* Each cycle needs to determine whether the instance is reset,
@@ -819,15 +795,10 @@ static int hns3_dbg_tx_queue_info(struct hnae3_handle *h,
  			return -EPERM;
  
  		ring = &priv->ring[i];
-		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));
-		pos += scnprintf(buf + pos, len - pos, "%s", content);
+		hns3_dump_tx_queue_info(ring, ae_dev, s, i);
  	}
  
-	hns3_dbg_tx_spare_info(ring, buf, len, h->kinfo.num_tqps, &pos);
+	hns3_dbg_tx_spare_info(ring, s, h->kinfo.num_tqps);
  
  	return 0;
  }
@@ -1222,10 +1193,6 @@ static const struct hns3_dbg_func hns3_dbg_cmd_func[] = {
  		.cmd = HNAE3_DBG_CMD_RX_QUEUE_INFO,
  		.dbg_dump = hns3_dbg_rx_queue_info,
  	},
-	{
-		.cmd = HNAE3_DBG_CMD_TX_QUEUE_INFO,
-		.dbg_dump = hns3_dbg_tx_queue_info,
-	},
  	{
  		.cmd = HNAE3_DBG_CMD_PAGE_POOL_INFO,
  		.dbg_dump = hns3_dbg_page_pool_info,
@@ -1362,6 +1329,27 @@ hns3_dbg_common_file_init(struct hnae3_handle *handle, u32 cmd)
  	return 0;
  }
  
+static int hns3_dbg_common_init_t1(struct hnae3_handle *handle, u32 cmd)
+{
+	struct device *dev = &handle->pdev->dev;
+	struct dentry *entry_dir;
+	read_func func = NULL;
+
+	switch (hns3_dbg_cmd[cmd].cmd) {
+	case HNAE3_DBG_CMD_TX_QUEUE_INFO:
+		func = hns3_dbg_tx_queue_info;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	entry_dir = hns3_dbg_dentry[hns3_dbg_cmd[cmd].dentry].dentry;
+	debugfs_create_devm_seqfile(dev, hns3_dbg_cmd[cmd].name, entry_dir,
+				    func);
+
+	return 0;
+}
+
  int hns3_dbg_init(struct hnae3_handle *handle)
  {
  	struct hnae3_ae_dev *ae_dev = pci_get_drvdata(handle->pdev);
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index 5c8c62ea6ac0..f01c7e45e674 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
@@ -5299,6 +5299,8 @@ static int hns3_client_init(struct hnae3_handle *handle)
  	struct net_device *netdev;
  	int ret;
  
+	ae_dev->handle = handle;
+
  	handle->ae_algo->ops->get_tqps_and_rss_info(handle, &alloc_tqps,
  						    &max_rss_size);
  	netdev = alloc_etherdev_mq(sizeof(struct hns3_nic_priv), alloc_tqps);
-- 
2.33.0

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

[Jijie Shao]

on 2025/6/13 13:59, Jijie Shao wrote:
>
> on 2025/6/12 23:33, Jakub Kicinski wrote:
>> On Thu, 12 Jun 2025 21:09:40 +0800 Jijie Shao wrote:
>>> seq_file is good. But the change is quite big.
>>> I need to discuss it internally, and it may not be completed so 
>>> quickly.
>>> I will also need consider the maintainer's suggestion.
>> Please work on the seq_file conversion, given that the merge window
>> just closed you have around 6 weeks to get it done, so hopefully plenty
>> of time.
>
> Ok
>
> I will try to send patch as soon as possible to complete this conversion
>
>
> Thanks
> Jijie Shao
>
>

*Hi Jakub, Arnd We have changed the impleament as your suggestion. Would 
you please help check it ? If it's OK, we will rewrite the rest parts of 
our debugfs code. Thanks! *

==========

 From c62568f3e91eb5725211fde8e63d44f68452b4e3 Mon Sep 17 00:00:00 2001
From: Jian Shen <shenjian15@huawei.com>
Date: Thu, 19 Jun 2025 16:21:17 +0800
Subject: [PATCH  net-next] net: hns3: clean up the build warning in debugfs by
  use seq file

Arnd reported that there are two build warning for on-stasck
buffer oversize[1]. As Arnd's suggestion, using seq file way
to avoid the stack buffer or kmalloc buffer allocating.

Reported-by: Arnd Bergmann <arnd@kernel.org>
Closes: https://lore.kernel.org/all/20250610092113.2639248-1-arnd@kernel.org/
Signed-off-by: Jian Shen <shenjian15@huawei.com>
Signed-off-by: Jijie Shao <shaojijie@huawei.com>
---
  drivers/net/ethernet/hisilicon/hns3/hnae3.h   |   3 +
  .../ethernet/hisilicon/hns3/hns3_debugfs.c    | 160 ++++++++----------
  .../net/ethernet/hisilicon/hns3/hns3_enet.c   |   2 +
  3 files changed, 79 insertions(+), 86 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hnae3.h b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
index 8dc7d6fae224..58a63d2eb69b 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hnae3.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
@@ -434,8 +434,11 @@ struct hnae3_ae_dev {
      u32 dev_version;
      DECLARE_BITMAP(caps, HNAE3_DEV_CAPS_MAX_NUM);
      void *priv;
+    struct hnae3_handle *handle;
  };
  
+typedef int (*read_func)(struct seq_file *s, void *data);
+
  /* This struct defines the operation on the handle.
   *
   * init_ae_dev(): (mandatory)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c b/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
index 4e5d8bc39a1b..458a2944ee3c 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
@@ -3,6 +3,7 @@
  
  #include <linux/debugfs.h>
  #include <linux/device.h>
+#include <linux/seq_file.h>
  #include <linux/string_choices.h>
  
  #include "hnae3.h"
@@ -41,6 +42,7 @@ static struct hns3_dbg_dentry_info hns3_dbg_dentry[] = {
  
  static int hns3_dbg_bd_file_init(struct hnae3_handle *handle, u32 cmd);
  static int hns3_dbg_common_file_init(struct hnae3_handle *handle, u32 cmd);
+static int hns3_dbg_common_init_t1(struct hnae3_handle *handle, u32 cmd);
  
  static struct hns3_dbg_cmd_info hns3_dbg_cmd[] = {
      {
@@ -300,7 +302,7 @@ static struct hns3_dbg_cmd_info hns3_dbg_cmd[] = {
          .cmd = HNAE3_DBG_CMD_TX_QUEUE_INFO,
          .dentry = HNS3_DBG_DENTRY_QUEUE,
          .buf_len = HNS3_DBG_READ_LEN_1MB,
-        .init = hns3_dbg_common_file_init,
+        .init = hns3_dbg_common_init_t1,
      },
      {
          .name = "fd_tcam",
@@ -580,44 +582,29 @@ 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 void hns3_dbg_tx_spare_info(struct hns3_enet_ring *ring,
+                   struct seq_file *s, u32 ring_num)
  {
-    char data_str[ARRAY_SIZE(tx_spare_info_items)][HNS3_DBG_DATA_STR_LEN];
      struct hns3_tx_spare *tx_spare = ring->tx_spare;
-    char *result[ARRAY_SIZE(tx_spare_info_items)];
-    char content[HNS3_DBG_INFO_LEN];
-    u32 i, j;
+    u32 i;
  
      if (!tx_spare) {
-        *pos += scnprintf(buf + *pos, len - *pos,
-                  "tx spare buffer is not enabled\n");
+        seq_puts(s, "tx spare buffer is not enabled\n");
          return;
      }
  
-    for (i = 0; i < ARRAY_SIZE(tx_spare_info_items); i++)
-        result[i] = &data_str[i][0];
-
-    *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));
-    *pos += scnprintf(buf + *pos, len - *pos, "%s", content);
+    seq_puts(s, "tx spare buffer info\n");
  
-    for (i = 0; i < ring_num; i++) {
-        j = 0;
-        sprintf(result[j++], "%u", i);
-        sprintf(result[j++], "%u", ring->tx_copybreak);
-        sprintf(result[j++], "%u", tx_spare->len);
-        sprintf(result[j++], "%u", tx_spare->next_to_use);
-        sprintf(result[j++], "%u", tx_spare->next_to_clean);
-        sprintf(result[j++], "%u", tx_spare->last_to_clean);
-        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));
-        *pos += scnprintf(buf + *pos, len - *pos, "%s", content);
-    }
+    for (i = 0; i < ARRAY_SIZE(tx_spare_info_items); i++)
+        seq_printf(s, "%s%*s", tx_spare_info_items[i].name,
+               tx_spare_info_items[i].interval, " ");
+    seq_puts(s, "\n");
+
+    for (i = 0; i < ring_num; i++)
+        seq_printf(s, "%-4u%6s%-5u%6s%-8u%2s%-5u%2s%-5u%2s%-5u%2s%pad\n",
+               i, " ", ring->tx_copybreak, " ", tx_spare->len, " ",
+               tx_spare->next_to_use, " ", tx_spare->next_to_clean,
+               " ", tx_spare->last_to_clean, " ", &tx_spare->dma);
  }
  
  static const struct hns3_dbg_item rx_queue_info_items[] = {
@@ -739,62 +726,52 @@ static const struct hns3_dbg_item tx_queue_info_items[] = {
  };
  
  static void hns3_dump_tx_queue_info(struct hns3_enet_ring *ring,
-                    struct hnae3_ae_dev *ae_dev, char **result,
-                    u32 index)
+                    struct hnae3_ae_dev *ae_dev,
+                    struct seq_file *s, u32 index)
  {
+    void __iomem *base = ring->tqp->io_base;
      u32 base_add_l, base_add_h;
-    u32 j = 0;
-
-    sprintf(result[j++], "%u", index);
-    sprintf(result[j++], "%u", readl_relaxed(ring->tqp->io_base +
-        HNS3_RING_TX_RING_BD_NUM_REG));
-
-    sprintf(result[j++], "%u", readl_relaxed(ring->tqp->io_base +
-        HNS3_RING_TX_RING_TC_REG));
-
-    sprintf(result[j++], "%u", readl_relaxed(ring->tqp->io_base +
-        HNS3_RING_TX_RING_TAIL_REG));
-
-    sprintf(result[j++], "%u", readl_relaxed(ring->tqp->io_base +
-        HNS3_RING_TX_RING_HEAD_REG));
-
-    sprintf(result[j++], "%u", readl_relaxed(ring->tqp->io_base +
-        HNS3_RING_TX_RING_FBDNUM_REG));
-
-    sprintf(result[j++], "%u", readl_relaxed(ring->tqp->io_base +
-        HNS3_RING_TX_RING_OFFSET_REG));
-
-    sprintf(result[j++], "%u", readl_relaxed(ring->tqp->io_base +
-        HNS3_RING_TX_RING_PKTNUM_RECORD_REG));
  
-    sprintf(result[j++], "%s",
-        str_on_off(readl_relaxed(ring->tqp->io_base +
-                     HNS3_RING_EN_REG)));
+    seq_printf(s, "%-4u%6s", index, " ");
+    seq_printf(s, "%-5u%3s",
+           readl_relaxed(base + HNS3_RING_TX_RING_BD_NUM_REG), " ");
+    seq_printf(s, "%u%3s",
+           readl_relaxed(base + HNS3_RING_TX_RING_TC_REG), " ");
+    seq_printf(s, "%-4u%2s",
+           readl_relaxed(base + HNS3_RING_TX_RING_TAIL_REG), " ");
+    seq_printf(s, "%-4u%2s",
+           readl_relaxed(base + HNS3_RING_TX_RING_HEAD_REG), " ");
+    seq_printf(s, "%-4u%4s",
+           readl_relaxed(base + HNS3_RING_TX_RING_FBDNUM_REG), " ");
+    seq_printf(s, "%-4u%4s",
+           readl_relaxed(base + HNS3_RING_TX_RING_OFFSET_REG), " ");
+    seq_printf(s, "%-9u%2s",
+           readl_relaxed(base + HNS3_RING_TX_RING_PKTNUM_RECORD_REG),
+           " ");
+    seq_printf(s, "%-3s%6s",
+           str_on_off(readl_relaxed(base + HNS3_RING_EN_REG)), " ");
  
      if (hnae3_ae_dev_tqp_txrx_indep_supported(ae_dev))
-        sprintf(result[j++], "%s",
-            str_on_off(readl_relaxed(ring->tqp->io_base +
-                         HNS3_RING_TX_EN_REG)));
+        seq_printf(s, "%-3s%9s",
+               str_on_off(readl_relaxed(base +
+                            HNS3_RING_TX_EN_REG)),
+               " ");
      else
-        sprintf(result[j++], "%s", "NA");
+        seq_printf(s, "%-3s%9s", "NA", " ");
  
      base_add_h = readl_relaxed(ring->tqp->io_base +
                      HNS3_RING_TX_RING_BASEADDR_H_REG);
      base_add_l = readl_relaxed(ring->tqp->io_base +
                      HNS3_RING_TX_RING_BASEADDR_L_REG);
-    sprintf(result[j++], "0x%08x%08x", base_add_h, base_add_l);
+    seq_printf(s, "0x%08x%08x\n", base_add_h, base_add_l);
  }
  
-static int hns3_dbg_tx_queue_info(struct hnae3_handle *h,
-                  char *buf, int len)
+static int hns3_dbg_tx_queue_info(struct seq_file *s, void *data)
  {
-    char data_str[ARRAY_SIZE(tx_queue_info_items)][HNS3_DBG_DATA_STR_LEN];
-    struct hnae3_ae_dev *ae_dev = pci_get_drvdata(h->pdev);
-    char *result[ARRAY_SIZE(tx_queue_info_items)];
+    struct hnae3_ae_dev *ae_dev = dev_get_drvdata(s->private);
+    struct hnae3_handle *h = ae_dev->handle;
      struct hns3_nic_priv *priv = h->priv;
-    char content[HNS3_DBG_INFO_LEN];
      struct hns3_enet_ring *ring;
-    int pos = 0;
      u32 i;
  
      if (!priv->ring) {
@@ -803,11 +780,10 @@ static int hns3_dbg_tx_queue_info(struct hnae3_handle *h,
      }
  
      for (i = 0; i < ARRAY_SIZE(tx_queue_info_items); i++)
-        result[i] = &data_str[i][0];
+        seq_printf(s, "%s%*s", tx_queue_info_items[i].name,
+               tx_queue_info_items[i].interval, " ");
  
-    hns3_dbg_fill_content(content, sizeof(content), tx_queue_info_items,
-                  NULL, ARRAY_SIZE(tx_queue_info_items));
-    pos += scnprintf(buf + pos, len - pos, "%s", content);
+    seq_puts(s, "\n");
  
      for (i = 0; i < h->kinfo.num_tqps; i++) {
          /* Each cycle needs to determine whether the instance is reset,
@@ -819,15 +795,10 @@ static int hns3_dbg_tx_queue_info(struct hnae3_handle *h,
              return -EPERM;
  
          ring = &priv->ring[i];
-        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));
-        pos += scnprintf(buf + pos, len - pos, "%s", content);
+        hns3_dump_tx_queue_info(ring, ae_dev, s, i);
      }
  
-    hns3_dbg_tx_spare_info(ring, buf, len, h->kinfo.num_tqps, &pos);
+    hns3_dbg_tx_spare_info(ring, s, h->kinfo.num_tqps);
  
      return 0;
  }
@@ -1222,10 +1193,6 @@ static const struct hns3_dbg_func hns3_dbg_cmd_func[] = {
          .cmd = HNAE3_DBG_CMD_RX_QUEUE_INFO,
          .dbg_dump = hns3_dbg_rx_queue_info,
      },
-    {
-        .cmd = HNAE3_DBG_CMD_TX_QUEUE_INFO,
-        .dbg_dump = hns3_dbg_tx_queue_info,
-    },
      {
          .cmd = HNAE3_DBG_CMD_PAGE_POOL_INFO,
          .dbg_dump = hns3_dbg_page_pool_info,
@@ -1362,6 +1329,27 @@ hns3_dbg_common_file_init(struct hnae3_handle *handle, u32 cmd)
      return 0;
  }
  
+static int hns3_dbg_common_init_t1(struct hnae3_handle *handle, u32 cmd)
+{
+    struct device *dev = &handle->pdev->dev;
+    struct dentry *entry_dir;
+    read_func func = NULL;
+
+    switch (hns3_dbg_cmd[cmd].cmd) {
+    case HNAE3_DBG_CMD_TX_QUEUE_INFO:
+        func = hns3_dbg_tx_queue_info;
+        break;
+    default:
+        return -EINVAL;
+    }
+
+    entry_dir = hns3_dbg_dentry[hns3_dbg_cmd[cmd].dentry].dentry;
+    debugfs_create_devm_seqfile(dev, hns3_dbg_cmd[cmd].name, entry_dir,
+                    func);
+
+    return 0;
+}
+
  int hns3_dbg_init(struct hnae3_handle *handle)
  {
      struct hnae3_ae_dev *ae_dev = pci_get_drvdata(handle->pdev);
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index 5c8c62ea6ac0..f01c7e45e674 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
@@ -5299,6 +5299,8 @@ static int hns3_client_init(struct hnae3_handle *handle)
      struct net_device *netdev;
      int ret;
  
+    ae_dev->handle = handle;
+
      handle->ae_algo->ops->get_tqps_and_rss_info(handle, &alloc_tqps,
                              &max_rss_size);
      netdev = alloc_etherdev_mq(sizeof(struct hns3_nic_priv), alloc_tqps);
-- 
2.33.0

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

[Arnd Bergmann]

On Mon, Jun 23, 2025, at 05:19, Jijie Shao wrote:
>> on 2025/6/12 23:33, Jakub Kicinski wrote:
>>
>
> *Hi Jakub, Arnd We have changed the impleament as your suggestion. Would 
> you please help check it ? If it's OK, we will rewrite the rest parts of 
> our debugfs code. Thanks! *

The conversion to seq_file looks good to me, this does address the
stack usage problems I was observing.
Thanks for cleaning this up!

> -    sprintf(result[j++], "%u", index);
> -    sprintf(result[j++], "%u", readl_relaxed(ring->tqp->io_base +
> -        HNS3_RING_TX_RING_BD_NUM_REG));
> +    seq_printf(s, "%-4u%6s", index, " ");
> +    seq_printf(s, "%-5u%3s",
> +           readl_relaxed(base + HNS3_RING_TX_RING_BD_NUM_REG), " ");

I'm not sure I understand the format string changes here, I did
not think they were necessary.

Are you doing this to keep the output the same as before, or are
you reformatting the contents for readability?

> +static int hns3_dbg_common_init_t1(struct hnae3_handle *handle, u32 
> cmd)
> +{
> +    struct device *dev = &handle->pdev->dev;
> +    struct dentry *entry_dir;
> +    read_func func = NULL;
> +
> +    switch (hns3_dbg_cmd[cmd].cmd) {
> +    case HNAE3_DBG_CMD_TX_QUEUE_INFO:
> +        func = hns3_dbg_tx_queue_info;
> +        break;
> +    default:
> +        return -EINVAL;
> +    }
> +
> +    entry_dir = hns3_dbg_dentry[hns3_dbg_cmd[cmd].dentry].dentry;
> +    debugfs_create_devm_seqfile(dev, hns3_dbg_cmd[cmd].name, entry_dir,
> +                    func);
> +
> +    return 0;

This will work fine as well, but I think you can do slightly better
by having your own file_operations with a read function based
on single_open() and your current hns3_dbg_read_cmd().

I don't think you gain anything from using debugfs_create_devm_seqfile()
since you use debugfs_remove_recursive() for cleaning it up anyway.

     Arnd

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

[Jijie Shao]

on 2025/6/23 13:56, Arnd Bergmann wrote:
> On Mon, Jun 23, 2025, at 05:19, Jijie Shao wrote:
>>> on 2025/6/12 23:33, Jakub Kicinski wrote:
>>>
>> *Hi Jakub, Arnd We have changed the impleament as your suggestion. Would
>> you please help check it ? If it's OK, we will rewrite the rest parts of
>> our debugfs code. Thanks! *
> The conversion to seq_file looks good to me, this does address the
> stack usage problems I was observing.
> Thanks for cleaning this up!
>
>> -    sprintf(result[j++], "%u", index);
>> -    sprintf(result[j++], "%u", readl_relaxed(ring->tqp->io_base +
>> -        HNS3_RING_TX_RING_BD_NUM_REG));
>> +    seq_printf(s, "%-4u%6s", index, " ");
>> +    seq_printf(s, "%-5u%3s",
>> +           readl_relaxed(base + HNS3_RING_TX_RING_BD_NUM_REG), " ");
> I'm not sure I understand the format string changes here, I did
> not think they were necessary.
>
> Are you doing this to keep the output the same as before, or are
> you reformatting the contents for readability?

yeah, just to keep the output the same as before


>
>> +static int hns3_dbg_common_init_t1(struct hnae3_handle *handle, u32
>> cmd)
>> +{
>> +    struct device *dev = &handle->pdev->dev;
>> +    struct dentry *entry_dir;
>> +    read_func func = NULL;
>> +
>> +    switch (hns3_dbg_cmd[cmd].cmd) {
>> +    case HNAE3_DBG_CMD_TX_QUEUE_INFO:
>> +        func = hns3_dbg_tx_queue_info;
>> +        break;
>> +    default:
>> +        return -EINVAL;
>> +    }
>> +
>> +    entry_dir = hns3_dbg_dentry[hns3_dbg_cmd[cmd].dentry].dentry;
>> +    debugfs_create_devm_seqfile(dev, hns3_dbg_cmd[cmd].name, entry_dir,
>> +                    func);
>> +
>> +    return 0;
> This will work fine as well, but I think you can do slightly better
> by having your own file_operations with a read function based
> on single_open() and your current hns3_dbg_read_cmd().
>
> I don't think you gain anything from using debugfs_create_devm_seqfile()
> since you use debugfs_remove_recursive() for cleaning it up anyway.
>
>       Arnd

Using debugfs_create_devm_seqfile() is just to simplify the code.
We only need to focus on the implementation of .read() function.

Thanks
Jijie Shao

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

[Arnd Bergmann]

On Mon, Jun 23, 2025, at 08:21, Jijie Shao wrote:
> on 2025/6/23 13:56, Arnd Bergmann wrote:
>>
>>> -    sprintf(result[j++], "%u", index);
>>> -    sprintf(result[j++], "%u", readl_relaxed(ring->tqp->io_base +
>>> -        HNS3_RING_TX_RING_BD_NUM_REG));
>>> +    seq_printf(s, "%-4u%6s", index, " ");
>>> +    seq_printf(s, "%-5u%3s",
>>> +           readl_relaxed(base + HNS3_RING_TX_RING_BD_NUM_REG), " ");
>> I'm not sure I understand the format string changes here, I did
>> not think they were necessary.
>>
>> Are you doing this to keep the output the same as before, or are
>> you reformatting the contents for readability?
>
> yeah, just to keep the output the same as before

Ok. 

>>> +static int hns3_dbg_common_init_t1(struct hnae3_handle *handle, u32
>>> cmd)
>>> +{
>>> +    struct device *dev = &handle->pdev->dev;
>>> +    struct dentry *entry_dir;
>>> +    read_func func = NULL;
>>> +
>>> +    switch (hns3_dbg_cmd[cmd].cmd) {
>>> +    case HNAE3_DBG_CMD_TX_QUEUE_INFO:
>>> +        func = hns3_dbg_tx_queue_info;
>>> +        break;
>>> +    default:
>>> +        return -EINVAL;
>>> +    }
>>> +
>>> +    entry_dir = hns3_dbg_dentry[hns3_dbg_cmd[cmd].dentry].dentry;
>>> +    debugfs_create_devm_seqfile(dev, hns3_dbg_cmd[cmd].name, entry_dir,
>>> +                    func);
>>> +
>>> +    return 0;
>> This will work fine as well, but I think you can do slightly better
>> by having your own file_operations with a read function based
>> on single_open() and your current hns3_dbg_read_cmd().
>>
>> I don't think you gain anything from using debugfs_create_devm_seqfile()
>> since you use debugfs_remove_recursive() for cleaning it up anyway.
>
> Using debugfs_create_devm_seqfile() is just to simplify the code.
> We only need to focus on the implementation of .read() function.

What I meant is that it doesn't seem simpler to me, as it adds one
level of indirection to both the file creation and the read()
function compared to having a single_open() helpe with that
switch()/case or the corresponding hns3_dbg_cmd_func[] array.

Either way, I'm not worried about it, there is no actual problem
that I see with your version.

     Arnd