Messages in this thread | ![/](/images/icornerl.gif) | | Date | Tue, 17 Jan 2023 21:01:03 +0100 | From | "Arnd Bergmann" <> | Subject | Re: [PATCH] mlx5: reduce stack usage in mlx5_setup_tc |
| |
On Tue, Jan 17, 2023, at 18:46, Tariq Toukan wrote: > On 17/01/2023 19:28, Arnd Bergmann wrote: >> From: Arnd Bergmann <arnd@arndb.de> >> >> Clang warns about excessive stack usage on 32-bit targets: >> >> drivers/net/ethernet/mellanox/mlx5/core/en_main.c:3597:12: error: stack frame size (1184) exceeds limit (1024) in 'mlx5e_setup_tc' [-Werror,-Wframe-larger-than] >> static int mlx5e_setup_tc(struct net_device *dev, enum tc_setup_type type, >> >> It turns out that both the mlx5e_setup_tc_mqprio_dcb() function and >> the mlx5e_safe_switch_params() function it calls have a copy of >> 'struct mlx5e_params' on the stack, and this structure is fairly >> large. >> >> Use dynamic allocation for both. >>
>> >> - err = mlx5e_safe_switch_params(priv, &new_params, >> + err = mlx5e_safe_switch_params(priv, new_params, >> mlx5e_num_channels_changed_ctx, NULL, true); >> > > Is this change really required, even after new_chs are dynamically > allocated? > As this code pattern of static local new_params repeats in all callers > of mlx5e_safe_switch_params, let's not change this one alone if not > necessary.
I'm not sure any more now, I actually did the patch a few weeks ago and only now came across it while going through my backlog.
Generally speaking, the 'new_params' structure on the stack is too large, but I no longer see warnings after my patch.
> Same for the noinline_for_stack. Are they really needed even after using > dynamic allocation for new_chs?
I've reverted both of those hunks now, let me try reproducing the original randconfig reports and see what still happens.
Arnd
| ![\](/images/icornerr.gif) |