* [PATCH] [v2] net: hns3: reduce stack usage in hclge_dbg_dump_tm_pri()
@ 2023-12-04 8:57 Arnd Bergmann
2023-12-04 14:50 ` Jijie Shao
2023-12-04 18:54 ` Christophe JAILLET
0 siblings, 2 replies; 5+ messages in thread
From: Arnd Bergmann @ 2023-12-04 8:57 UTC (permalink / raw)
To: Yisen Zhuang, Salil Mehta, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
Cc: Arnd Bergmann, Jijie Shao, Hao Chen, netdev, linux-kernel
From: Arnd Bergmann <arnd@arndb.de>
This function exceeds the stack frame warning limit:
drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c: In function 'hclge_dbg_dump_tm_pri':
drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:1039:1: error: the frame size of 1408 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
Use dynamic allocation for the largest stack object instead. It
would be nice to rewrite this file to completely avoid the extra
buffer and just use the one that was already allocated by debugfs,
but that is a much larger change.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
v2: fix error handling leak
---
.../hisilicon/hns3/hns3pf/hclge_debugfs.c | 21 ++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c
index ff3f8f424ad9..8f94e13c1edf 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c
@@ -981,7 +981,7 @@ static const struct hclge_dbg_item tm_pri_items[] = {
static int hclge_dbg_dump_tm_pri(struct hclge_dev *hdev, char *buf, int len)
{
- char data_str[ARRAY_SIZE(tm_pri_items)][HCLGE_DBG_DATA_STR_LEN];
+ char *data_str;
struct hclge_tm_shaper_para c_shaper_para, p_shaper_para;
char *result[ARRAY_SIZE(tm_pri_items)], *sch_mode_str;
char content[HCLGE_DBG_TM_INFO_LEN];
@@ -992,8 +992,13 @@ static int hclge_dbg_dump_tm_pri(struct hclge_dev *hdev, char *buf, int len)
if (ret)
return ret;
+ data_str = kcalloc(ARRAY_SIZE(tm_pri_items), HCLGE_DBG_DATA_STR_LEN,
+ GFP_KERNEL);
+ if (!data_str)
+ return -ENOMEM;
+
for (i = 0; i < ARRAY_SIZE(tm_pri_items); i++)
- result[i] = &data_str[i][0];
+ result[i] = &data_str[i * HCLGE_DBG_DATA_STR_LEN];
hclge_dbg_fill_content(content, sizeof(content), tm_pri_items,
NULL, ARRAY_SIZE(tm_pri_items));
@@ -1002,23 +1007,23 @@ static int hclge_dbg_dump_tm_pri(struct hclge_dev *hdev, char *buf, int len)
for (i = 0; i < pri_num; i++) {
ret = hclge_tm_get_pri_sch_mode(hdev, i, &sch_mode);
if (ret)
- return ret;
+ goto out;
ret = hclge_tm_get_pri_weight(hdev, i, &weight);
if (ret)
- return ret;
+ goto out;
ret = hclge_tm_get_pri_shaper(hdev, i,
HCLGE_OPC_TM_PRI_C_SHAPPING,
&c_shaper_para);
if (ret)
- return ret;
+ goto out;
ret = hclge_tm_get_pri_shaper(hdev, i,
HCLGE_OPC_TM_PRI_P_SHAPPING,
&p_shaper_para);
if (ret)
- return ret;
+ goto out;
sch_mode_str = sch_mode & HCLGE_TM_TX_SCHD_DWRR_MSK ? "dwrr" :
"sp";
@@ -1035,7 +1040,9 @@ static int hclge_dbg_dump_tm_pri(struct hclge_dev *hdev, char *buf, int len)
pos += scnprintf(buf + pos, len - pos, "%s", content);
}
- return 0;
+out:
+ kfree(data_str);
+ return ret;
}
static const struct hclge_dbg_item tm_qset_items[] = {
--
2.39.2
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] [v2] net: hns3: reduce stack usage in hclge_dbg_dump_tm_pri()
2023-12-04 8:57 [PATCH] [v2] net: hns3: reduce stack usage in hclge_dbg_dump_tm_pri() Arnd Bergmann
@ 2023-12-04 14:50 ` Jijie Shao
2023-12-06 3:23 ` Jakub Kicinski
2023-12-04 18:54 ` Christophe JAILLET
1 sibling, 1 reply; 5+ messages in thread
From: Jijie Shao @ 2023-12-04 14:50 UTC (permalink / raw)
To: Arnd Bergmann, Yisen Zhuang, Salil Mehta, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: shaojijie, Arnd Bergmann, Hao Chen, netdev, linux-kernel
on 2023/12/4 16:57, Arnd Bergmann wrote:
> s already allocated by debugfs,
> but that is a much larger change.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> v2: fix error handling leak
> ---
> .../hisilicon/hns3/hns3pf/hclge_debugfs.c | 21 ++++++++++++-------
> 1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c
> index ff3f8f424ad9..8f94e13c1edf 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c
> @@ -981,7 +981,7 @@ static const struct hclge_dbg_item tm_pri_items[] = {
>
> static int hclge_dbg_dump_tm_pri(struct hclge_dev *hdev, char *buf, int len)
> {
> - char data_str[ARRAY_SIZE(tm_pri_items)][HCLGE_DBG_DATA_STR_LEN];
> + char *data_str;
We want to define variables in an inverted triangle based on the code length.
so, "char *data_str" should move four lines down.
struct hclge_tm_shaper_para c_shaper_para, p_shaper_para;
char *result[ARRAY_SIZE(tm_pri_items)], *sch_mode_str;
char content[HCLGE_DBG_TM_INFO_LEN];
u8 pri_num, sch_mode, weight, i, j;
char *data_str;
int pos, ret;
> struct hclge_tm_shaper_para c_shaper_para, p_shaper_para;
> char *result[ARRAY_SIZE(tm_pri_items)], *sch_mode_str;
> char content[HCLGE_DBG_TM_INFO_LEN];
> @@ -992,8 +992,13 @@ static int hclge_dbg_dump_tm_pri(struct hclge_dev *hdev, char *buf, int len)
> if (ret)
> return ret;
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] [v2] net: hns3: reduce stack usage in hclge_dbg_dump_tm_pri()
2023-12-04 14:50 ` Jijie Shao
@ 2023-12-06 3:23 ` Jakub Kicinski
0 siblings, 0 replies; 5+ messages in thread
From: Jakub Kicinski @ 2023-12-06 3:23 UTC (permalink / raw)
To: Jijie Shao
Cc: Arnd Bergmann, Yisen Zhuang, Salil Mehta, David S. Miller,
Eric Dumazet, Paolo Abeni, Arnd Bergmann, Hao Chen, netdev,
linux-kernel
On Mon, 4 Dec 2023 22:50:55 +0800 Jijie Shao wrote:
> > static int hclge_dbg_dump_tm_pri(struct hclge_dev *hdev, char *buf, int len)
> > {
> > - char data_str[ARRAY_SIZE(tm_pri_items)][HCLGE_DBG_DATA_STR_LEN];
> > + char *data_str;
>
> We want to define variables in an inverted triangle based on the code length.
> so, "char *data_str" should move four lines down.
>
> struct hclge_tm_shaper_para c_shaper_para, p_shaper_para;
> char *result[ARRAY_SIZE(tm_pri_items)], *sch_mode_str;
> char content[HCLGE_DBG_TM_INFO_LEN];
> u8 pri_num, sch_mode, weight, i, j;
> char *data_str;
> int pos, ret;
I took the liberty of fixing this when applying.
Don't want this to fall thru the cracks.
Applied to net-next now, thanks!
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] [v2] net: hns3: reduce stack usage in hclge_dbg_dump_tm_pri()
2023-12-04 8:57 [PATCH] [v2] net: hns3: reduce stack usage in hclge_dbg_dump_tm_pri() Arnd Bergmann
2023-12-04 14:50 ` Jijie Shao
@ 2023-12-04 18:54 ` Christophe JAILLET
2023-12-04 20:06 ` Arnd Bergmann
1 sibling, 1 reply; 5+ messages in thread
From: Christophe JAILLET @ 2023-12-04 18:54 UTC (permalink / raw)
To: Arnd Bergmann, Yisen Zhuang, Salil Mehta, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Arnd Bergmann, Jijie Shao, Hao Chen, netdev, linux-kernel
Le 04/12/2023 à 09:57, Arnd Bergmann a écrit :
> From: Arnd Bergmann <arnd@arndb.de>
>
> This function exceeds the stack frame warning limit:
>
> drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c: In function 'hclge_dbg_dump_tm_pri':
> drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:1039:1: error: the frame size of 1408 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
>
> Use dynamic allocation for the largest stack object instead. It
> would be nice to rewrite this file to completely avoid the extra
> buffer and just use the one that was already allocated by debugfs,
> but that is a much larger change.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> v2: fix error handling leak
> ---
> .../hisilicon/hns3/hns3pf/hclge_debugfs.c | 21 ++++++++++++-------
> 1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c
> index ff3f8f424ad9..8f94e13c1edf 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c
> @@ -981,7 +981,7 @@ static const struct hclge_dbg_item tm_pri_items[] = {
>
> static int hclge_dbg_dump_tm_pri(struct hclge_dev *hdev, char *buf, int len)
> {
> - char data_str[ARRAY_SIZE(tm_pri_items)][HCLGE_DBG_DATA_STR_LEN];
> + char *data_str;
> struct hclge_tm_shaper_para c_shaper_para, p_shaper_para;
> char *result[ARRAY_SIZE(tm_pri_items)], *sch_mode_str;
> char content[HCLGE_DBG_TM_INFO_LEN];
> @@ -992,8 +992,13 @@ static int hclge_dbg_dump_tm_pri(struct hclge_dev *hdev, char *buf, int len)
> if (ret)
> return ret;
>
> + data_str = kcalloc(ARRAY_SIZE(tm_pri_items), HCLGE_DBG_DATA_STR_LEN,
> + GFP_KERNEL);
> + if (!data_str)
> + return -ENOMEM;
> +
> for (i = 0; i < ARRAY_SIZE(tm_pri_items); i++)
> - result[i] = &data_str[i][0];
> + result[i] = &data_str[i * HCLGE_DBG_DATA_STR_LEN];
>
> hclge_dbg_fill_content(content, sizeof(content), tm_pri_items,
> NULL, ARRAY_SIZE(tm_pri_items));
> @@ -1002,23 +1007,23 @@ static int hclge_dbg_dump_tm_pri(struct hclge_dev *hdev, char *buf, int len)
> for (i = 0; i < pri_num; i++) {
> ret = hclge_tm_get_pri_sch_mode(hdev, i, &sch_mode);
> if (ret)
> - return ret;
> + goto out;
>
> ret = hclge_tm_get_pri_weight(hdev, i, &weight);
> if (ret)
> - return ret;
> + goto out;
>
> ret = hclge_tm_get_pri_shaper(hdev, i,
> HCLGE_OPC_TM_PRI_C_SHAPPING,
> &c_shaper_para);
> if (ret)
> - return ret;
> + goto out;
>
> ret = hclge_tm_get_pri_shaper(hdev, i,
> HCLGE_OPC_TM_PRI_P_SHAPPING,
> &p_shaper_para);
> if (ret)
> - return ret;
> + goto out;
>
> sch_mode_str = sch_mode & HCLGE_TM_TX_SCHD_DWRR_MSK ? "dwrr" :
> "sp";
> @@ -1035,7 +1040,9 @@ static int hclge_dbg_dump_tm_pri(struct hclge_dev *hdev, char *buf, int len)
> pos += scnprintf(buf + pos, len - pos, "%s", content);
> }
>
> - return 0;
> +out:
> + kfree(data_str);
> + return ret;
> }
>
> static const struct hclge_dbg_item tm_qset_items[] = {
Hi,
could :
pos += scnprintf(buf + pos, len - pos, "%s", <something>);
be more widely used to avoid the alloc()/free() + copy of strings?
CJ
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] [v2] net: hns3: reduce stack usage in hclge_dbg_dump_tm_pri()
2023-12-04 18:54 ` Christophe JAILLET
@ 2023-12-04 20:06 ` Arnd Bergmann
0 siblings, 0 replies; 5+ messages in thread
From: Arnd Bergmann @ 2023-12-04 20:06 UTC (permalink / raw)
To: Christophe JAILLET, Arnd Bergmann, Yisen Zhuang, Salil Mehta,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Jijie Shao, Hao Chen, Netdev, linux-kernel
On Mon, Dec 4, 2023, at 19:54, Christophe JAILLET wrote:
> Le 04/12/2023 à 09:57, Arnd Bergmann a écrit :
>>
>> Use dynamic allocation for the largest stack object instead. It
>> would be nice to rewrite this file to completely avoid the extra
>> buffer and just use the one that was already allocated by debugfs,
>> but that is a much larger change.
>>
> could :
> pos += scnprintf(buf + pos, len - pos, "%s", <something>);
> be more widely used to avoid the alloc()/free() + copy of strings?
Yes, I think that would help, but this is beyond what I trust
myself to do blindly.
Arnd
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-12-06 3:23 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-04 8:57 [PATCH] [v2] net: hns3: reduce stack usage in hclge_dbg_dump_tm_pri() Arnd Bergmann
2023-12-04 14:50 ` Jijie Shao
2023-12-06 3:23 ` Jakub Kicinski
2023-12-04 18:54 ` Christophe JAILLET
2023-12-04 20:06 ` Arnd Bergmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).