diff mbox series

net: hns3: fix hclge_dbg_dump_tm_pg() stack usage

Message ID 20210927095006.868305-1-arnd@kernel.org
State Superseded
Headers show
Series net: hns3: fix hclge_dbg_dump_tm_pg() stack usage | expand

Commit Message

Arnd Bergmann Sept. 27, 2021, 9:49 a.m. UTC
From: Arnd Bergmann <arnd@arndb.de>


This function copies strings around between multiple buffers
including a large on-stack array that causes a build warning
on 32-bit systems:

drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c: In function 'hclge_dbg_dump_tm_pg':
drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:782:1: error: the frame size of 1424 bytes is larger than 1400 bytes [-Werror=frame-larger-than=]

The function can probably be cleaned up a lot, to go back to
printing directly into the output buffer, but dynamically allocating
the structure is a simpler workaround for now.

Fixes: 04d96139ddb3 ("net: hns3: refine function hclge_dbg_dump_tm_pri()")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>

---
 .../hisilicon/hns3/hns3pf/hclge_debugfs.c     | 26 ++++++++++++++++---
 1 file changed, 22 insertions(+), 4 deletions(-)

-- 
2.29.2

Comments

huangguangbin (A) Sept. 28, 2021, 8:34 a.m. UTC | #1
On 2021/9/27 17:49, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>

> 

> This function copies strings around between multiple buffers

> including a large on-stack array that causes a build warning

> on 32-bit systems:

> 

> drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c: In function 'hclge_dbg_dump_tm_pg':

> drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:782:1: error: the frame size of 1424 bytes is larger than 1400 bytes [-Werror=frame-larger-than=]

> 

> The function can probably be cleaned up a lot, to go back to

> printing directly into the output buffer, but dynamically allocating

> the structure is a simpler workaround for now.

> 

> Fixes: 04d96139ddb3 ("net: hns3: refine function hclge_dbg_dump_tm_pri()")

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

> ---

>   .../hisilicon/hns3/hns3pf/hclge_debugfs.c     | 26 ++++++++++++++++---

>   1 file changed, 22 insertions(+), 4 deletions(-)

> 

> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c

> index 87d96f82c318..3ed87814100a 100644

> --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c

> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c

> @@ -719,9 +719,9 @@ static void hclge_dbg_fill_shaper_content(struct hclge_tm_shaper_para *para,

>   	sprintf(result[(*index)++], "%6u", para->rate);

>   }

>   

> -static int hclge_dbg_dump_tm_pg(struct hclge_dev *hdev, char *buf, int len)

> +static int __hclge_dbg_dump_tm_pg(struct hclge_dev *hdev, char *data_str,

> +				  char *buf, int len)

>   {

> -	char data_str[ARRAY_SIZE(tm_pg_items)][HCLGE_DBG_DATA_STR_LEN];

>   	struct hclge_tm_shaper_para c_shaper_para, p_shaper_para;

>   	char *result[ARRAY_SIZE(tm_pg_items)], *sch_mode_str;

>   	u8 pg_id, sch_mode, weight, pri_bit_map, i, j;

> @@ -729,8 +729,10 @@ static int hclge_dbg_dump_tm_pg(struct hclge_dev *hdev, char *buf, int len)

>   	int pos = 0;

>   	int ret;

>   

> -	for (i = 0; i < ARRAY_SIZE(tm_pg_items); i++)

> -		result[i] = &data_str[i][0];

> +	for (i = 0; i < ARRAY_SIZE(tm_pg_items); i++) {

> +		result[i] = data_str;

> +		data_str += HCLGE_DBG_DATA_STR_LEN;

> +	}

>   

>   	hclge_dbg_fill_content(content, sizeof(content), tm_pg_items,

>   			       NULL, ARRAY_SIZE(tm_pg_items));

> @@ -781,6 +783,22 @@ static int hclge_dbg_dump_tm_pg(struct hclge_dev *hdev, char *buf, int len)

>   	return 0;

>   }

>   

> +static int hclge_dbg_dump_tm_pg(struct hclge_dev *hdev, char *buf, int len)

> +{

> +	int ret;

> +	char *data_str = kcalloc(ARRAY_SIZE(tm_pg_items),

> +				 HCLGE_DBG_DATA_STR_LEN, GFP_KERNEL);

> +

Hi Arnd, thanks your modification, according to linux code style, should the code be written as follow?
	char *data_str;
	int ret;

	data_str = kcalloc(ARRAY_SIZE(tm_pg_items),
			   HCLGE_DBG_DATA_STR_LEN, GFP_KERNEL);
> +	if (!data_str)

> +		return -ENOMEM;

> +

> +	ret = __hclge_dbg_dump_tm_pg(hdev, data_str, buf, len);

> +

> +	kfree(data_str);

> +

> +	return ret;

> +}

> +

>   static int hclge_dbg_dump_tm_port(struct hclge_dev *hdev,  char *buf, int len)

>   {

>   	struct hclge_tm_shaper_para shaper_para;

>
Arnd Bergmann Sept. 28, 2021, 8:58 a.m. UTC | #2
On Tue, Sep 28, 2021 at 10:34 AM huangguangbin (A)
<huangguangbin2@huawei.com> wrote:
> On 2021/9/27 17:49, Arnd Bergmann wrote: From: Arnd Bergmann <arnd@arndb.de>


> > +static int hclge_dbg_dump_tm_pg(struct hclge_dev *hdev, char *buf, int len)

> > +{

> > +     int ret;

> > +     char *data_str = kcalloc(ARRAY_SIZE(tm_pg_items),

> > +                              HCLGE_DBG_DATA_STR_LEN, GFP_KERNEL);

> > +

> Hi Arnd, thanks your modification, according to linux code style, should the code be written as follow?

>

>         char *data_str;

>         int ret;

>

>         data_str = kcalloc(ARRAY_SIZE(tm_pg_items),

>                            HCLGE_DBG_DATA_STR_LEN, GFP_KERNEL);


That's actually one of the versions I tried, but I didn't really like
any of them, so
I went with the shorter version.

Sending a v2 now with that changed black.

        Arnd
diff mbox series

Patch

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c
index 87d96f82c318..3ed87814100a 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c
@@ -719,9 +719,9 @@  static void hclge_dbg_fill_shaper_content(struct hclge_tm_shaper_para *para,
 	sprintf(result[(*index)++], "%6u", para->rate);
 }
 
-static int hclge_dbg_dump_tm_pg(struct hclge_dev *hdev, char *buf, int len)
+static int __hclge_dbg_dump_tm_pg(struct hclge_dev *hdev, char *data_str,
+				  char *buf, int len)
 {
-	char data_str[ARRAY_SIZE(tm_pg_items)][HCLGE_DBG_DATA_STR_LEN];
 	struct hclge_tm_shaper_para c_shaper_para, p_shaper_para;
 	char *result[ARRAY_SIZE(tm_pg_items)], *sch_mode_str;
 	u8 pg_id, sch_mode, weight, pri_bit_map, i, j;
@@ -729,8 +729,10 @@  static int hclge_dbg_dump_tm_pg(struct hclge_dev *hdev, char *buf, int len)
 	int pos = 0;
 	int ret;
 
-	for (i = 0; i < ARRAY_SIZE(tm_pg_items); i++)
-		result[i] = &data_str[i][0];
+	for (i = 0; i < ARRAY_SIZE(tm_pg_items); i++) {
+		result[i] = data_str;
+		data_str += HCLGE_DBG_DATA_STR_LEN;
+	}
 
 	hclge_dbg_fill_content(content, sizeof(content), tm_pg_items,
 			       NULL, ARRAY_SIZE(tm_pg_items));
@@ -781,6 +783,22 @@  static int hclge_dbg_dump_tm_pg(struct hclge_dev *hdev, char *buf, int len)
 	return 0;
 }
 
+static int hclge_dbg_dump_tm_pg(struct hclge_dev *hdev, char *buf, int len)
+{
+	int ret;
+	char *data_str = kcalloc(ARRAY_SIZE(tm_pg_items),
+				 HCLGE_DBG_DATA_STR_LEN, GFP_KERNEL);
+
+	if (!data_str)
+		return -ENOMEM;
+
+	ret = __hclge_dbg_dump_tm_pg(hdev, data_str, buf, len);
+
+	kfree(data_str);
+
+	return ret;
+}
+
 static int hclge_dbg_dump_tm_port(struct hclge_dev *hdev,  char *buf, int len)
 {
 	struct hclge_tm_shaper_para shaper_para;