diff mbox series

net: microchip: sparx5: reduce stack usage

Message ID 20230217095815.2407377-1-arnd@kernel.org
State New
Headers
Series net: microchip: sparx5: reduce stack usage |

Commit Message

Arnd Bergmann Feb. 17, 2023, 9:58 a.m. UTC 
  From: Arnd Bergmann <arnd@arndb.de>

The vcap_admin structures in vcap_api_next_lookup_advanced_test()
take several hundred bytes of stack frame, but when CONFIG_KASAN_STACK
is enabled, each one of them also has extra padding before and after
it, which ends up blowing the warning limit:

In file included from drivers/net/ethernet/microchip/vcap/vcap_api.c:3521:
drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c: In function 'vcap_api_next_lookup_advanced_test':
drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c:1954:1: error: the frame size of 1448 bytes is larger than 1400 bytes [-Werror=frame-larger-than=]
 1954 | }

Reduce the total stack usage by replacing the five structures with
an array that only needs one pair of padding areas.

Fixes: 1f741f001160 ("net: microchip: sparx5: Add KUNIT tests for enabling/disabling chains")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 .../ethernet/microchip/vcap/vcap_api_kunit.c  | 26 +++++++++----------
 1 file changed, 12 insertions(+), 14 deletions(-)

Comments

Alexander Lobakin Feb. 17, 2023, 2:41 p.m. UTC | #1 
From: Arnd Bergmann <arnd@kernel.org>
Date: Fri, 17 Feb 2023 10:58:06 +0100

> From: Arnd Bergmann <arnd@arndb.de>
> 
> The vcap_admin structures in vcap_api_next_lookup_advanced_test()
> take several hundred bytes of stack frame, but when CONFIG_KASAN_STACK
> is enabled, each one of them also has extra padding before and after
> it, which ends up blowing the warning limit:
> 
> In file included from drivers/net/ethernet/microchip/vcap/vcap_api.c:3521:
> drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c: In function 'vcap_api_next_lookup_advanced_test':
> drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c:1954:1: error: the frame size of 1448 bytes is larger than 1400 bytes [-Werror=frame-larger-than=]
>  1954 | }
> 
> Reduce the total stack usage by replacing the five structures with
> an array that only needs one pair of padding areas.
> 
> Fixes: 1f741f001160 ("net: microchip: sparx5: Add KUNIT tests for enabling/disabling chains")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Reviewed-by: Alexander Lobakin <aleksander.lobakin@intel.com>

I'd suggest the driver authors to reduce the stack usage by moving big
data to the heap where possible tho. Otherwise, there probably will be
a new warning eventually one day.

> ---
>  .../ethernet/microchip/vcap/vcap_api_kunit.c  | 26 +++++++++----------
>  1 file changed, 12 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c b/drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c
> index 0a1d4d740567..c07f25e791c7 100644
> --- a/drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c
> +++ b/drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c
> @@ -1876,53 +1876,51 @@ static void vcap_api_next_lookup_basic_test(struct kunit *test)
>  
>  static void vcap_api_next_lookup_advanced_test(struct kunit *test)
>  {
> -	struct vcap_admin admin1 = {
> +	struct vcap_admin admin[] = {
> +	{
>  		.vtype = VCAP_TYPE_IS0,
>  		.vinst = 0,
>  		.first_cid = 1000000,
>  		.last_cid =  1199999,
>  		.lookups = 6,
>  		.lookups_per_instance = 2,
> -	};
> -	struct vcap_admin admin2 = {
> +	}, {
>  		.vtype = VCAP_TYPE_IS0,
>  		.vinst = 1,
>  		.first_cid = 1200000,
>  		.last_cid =  1399999,
>  		.lookups = 6,
>  		.lookups_per_instance = 2,
> -	};
> -	struct vcap_admin admin3 = {
> +	}, {
>  		.vtype = VCAP_TYPE_IS0,
>  		.vinst = 2,
>  		.first_cid = 1400000,
>  		.last_cid =  1599999,
>  		.lookups = 6,
>  		.lookups_per_instance = 2,
> -	};
> -	struct vcap_admin admin4 = {
> +	}, {
>  		.vtype = VCAP_TYPE_IS2,
>  		.vinst = 0,
>  		.first_cid = 8000000,
>  		.last_cid = 8199999,
>  		.lookups = 4,
>  		.lookups_per_instance = 2,
> -	};
> -	struct vcap_admin admin5 = {
> +	}, {
>  		.vtype = VCAP_TYPE_IS2,
>  		.vinst = 1,
>  		.first_cid = 8200000,
>  		.last_cid = 8399999,
>  		.lookups = 4,
>  		.lookups_per_instance = 2,
> +	}
>  	};
>  	bool ret;
>  
> -	vcap_test_api_init(&admin1);
> -	list_add_tail(&admin2.list, &test_vctrl.list);
> -	list_add_tail(&admin3.list, &test_vctrl.list);
> -	list_add_tail(&admin4.list, &test_vctrl.list);
> -	list_add_tail(&admin5.list, &test_vctrl.list);
> +	vcap_test_api_init(&admin[0]);
> +	list_add_tail(&admin[1].list, &test_vctrl.list);
> +	list_add_tail(&admin[2].list, &test_vctrl.list);
> +	list_add_tail(&admin[3].list, &test_vctrl.list);
> +	list_add_tail(&admin[4].list, &test_vctrl.list);
>  
>  	ret = vcap_is_next_lookup(&test_vctrl, 1000000, 1001000);
>  	KUNIT_EXPECT_EQ(test, false, ret);

Thanks,
Olek
patchwork-bot+netdevbpf@kernel.org Feb. 20, 2023, 11 a.m. UTC | #2 
Hello:

This patch was applied to netdev/net-next.git (master)
by David S. Miller <davem@davemloft.net>:

On Fri, 17 Feb 2023 10:58:06 +0100 you wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> The vcap_admin structures in vcap_api_next_lookup_advanced_test()
> take several hundred bytes of stack frame, but when CONFIG_KASAN_STACK
> is enabled, each one of them also has extra padding before and after
> it, which ends up blowing the warning limit:
> 
> [...]

Here is the summary with links:
  - net: microchip: sparx5: reduce stack usage
    https://git.kernel.org/netdev/net-next/c/129ff4de58ff

You are awesome, thank you!
diff mbox series

Patch

diff --git a/drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c b/drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c
index 0a1d4d740567..c07f25e791c7 100644
--- a/drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c
+++ b/drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c
@@ -1876,53 +1876,51 @@  static void vcap_api_next_lookup_basic_test(struct kunit *test)
 
 static void vcap_api_next_lookup_advanced_test(struct kunit *test)
 {
-	struct vcap_admin admin1 = {
+	struct vcap_admin admin[] = {
+	{
 		.vtype = VCAP_TYPE_IS0,
 		.vinst = 0,
 		.first_cid = 1000000,
 		.last_cid =  1199999,
 		.lookups = 6,
 		.lookups_per_instance = 2,
-	};
-	struct vcap_admin admin2 = {
+	}, {
 		.vtype = VCAP_TYPE_IS0,
 		.vinst = 1,
 		.first_cid = 1200000,
 		.last_cid =  1399999,
 		.lookups = 6,
 		.lookups_per_instance = 2,
-	};
-	struct vcap_admin admin3 = {
+	}, {
 		.vtype = VCAP_TYPE_IS0,
 		.vinst = 2,
 		.first_cid = 1400000,
 		.last_cid =  1599999,
 		.lookups = 6,
 		.lookups_per_instance = 2,
-	};
-	struct vcap_admin admin4 = {
+	}, {
 		.vtype = VCAP_TYPE_IS2,
 		.vinst = 0,
 		.first_cid = 8000000,
 		.last_cid = 8199999,
 		.lookups = 4,
 		.lookups_per_instance = 2,
-	};
-	struct vcap_admin admin5 = {
+	}, {
 		.vtype = VCAP_TYPE_IS2,
 		.vinst = 1,
 		.first_cid = 8200000,
 		.last_cid = 8399999,
 		.lookups = 4,
 		.lookups_per_instance = 2,
+	}
 	};
 	bool ret;
 
-	vcap_test_api_init(&admin1);
-	list_add_tail(&admin2.list, &test_vctrl.list);
-	list_add_tail(&admin3.list, &test_vctrl.list);
-	list_add_tail(&admin4.list, &test_vctrl.list);
-	list_add_tail(&admin5.list, &test_vctrl.list);
+	vcap_test_api_init(&admin[0]);
+	list_add_tail(&admin[1].list, &test_vctrl.list);
+	list_add_tail(&admin[2].list, &test_vctrl.list);
+	list_add_tail(&admin[3].list, &test_vctrl.list);
+	list_add_tail(&admin[4].list, &test_vctrl.list);
 
 	ret = vcap_is_next_lookup(&test_vctrl, 1000000, 1001000);
 	KUNIT_EXPECT_EQ(test, false, ret);