| Message ID | 20230117210324.1371169-1-arnd@kernel.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | [v2] mlx5: reduce stack usage in mlx5_setup_tc | expand |
On 1/17/2023 1:01 PM, 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 the inner one. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > v2: simplify the patch > --- This simpler version that focuses on fixing the stack frame issue looks good to me. Reviewed-by: Jacob Keller <jacob.e.keller@intel.com> Thanks, Jake > .../net/ethernet/mellanox/mlx5/core/en_main.c | 17 +++++++++++------ > 1 file changed, 11 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c > index 6bb0fdaa5efa..b0b872728653 100644 > --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c > @@ -2998,32 +2998,37 @@ int mlx5e_safe_switch_params(struct mlx5e_priv *priv, > mlx5e_fp_preactivate preactivate, > void *context, bool reset) > { > - struct mlx5e_channels new_chs = {}; > + struct mlx5e_channels *new_chs; > int err; > > reset &= test_bit(MLX5E_STATE_OPENED, &priv->state); > if (!reset) > return mlx5e_switch_priv_params(priv, params, preactivate, context); > > - new_chs.params = *params; > + new_chs = kzalloc(sizeof(*new_chs), GFP_KERNEL); > + if (!new_chs) > + return -ENOMEM; > + new_chs->params = *params; > > - mlx5e_selq_prepare_params(&priv->selq, &new_chs.params); > + mlx5e_selq_prepare_params(&priv->selq, &new_chs->params); > > - err = mlx5e_open_channels(priv, &new_chs); > + err = mlx5e_open_channels(priv, new_chs); > if (err) > goto err_cancel_selq; > > - err = mlx5e_switch_priv_channels(priv, &new_chs, preactivate, context); > + err = mlx5e_switch_priv_channels(priv, new_chs, preactivate, context); > if (err) > goto err_close; > > + kfree(new_chs); > return 0; > > err_close: > - mlx5e_close_channels(&new_chs); > + mlx5e_close_channels(new_chs); > > err_cancel_selq: > mlx5e_selq_cancel(&priv->selq); > + kfree(new_chs); > return err; > } >
On 17/01/2023 23:01, 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 the inner one. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > v2: simplify the patch > --- > .../net/ethernet/mellanox/mlx5/core/en_main.c | 17 +++++++++++------ > 1 file changed, 11 insertions(+), 6 deletions(-) > Thanks for your patch. Reviewed-by: Tariq Toukan <tariqt@nvidia.com>
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c index 6bb0fdaa5efa..b0b872728653 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c @@ -2998,32 +2998,37 @@ int mlx5e_safe_switch_params(struct mlx5e_priv *priv, mlx5e_fp_preactivate preactivate, void *context, bool reset) { - struct mlx5e_channels new_chs = {}; + struct mlx5e_channels *new_chs; int err; reset &= test_bit(MLX5E_STATE_OPENED, &priv->state); if (!reset) return mlx5e_switch_priv_params(priv, params, preactivate, context); - new_chs.params = *params; + new_chs = kzalloc(sizeof(*new_chs), GFP_KERNEL); + if (!new_chs) + return -ENOMEM; + new_chs->params = *params; - mlx5e_selq_prepare_params(&priv->selq, &new_chs.params); + mlx5e_selq_prepare_params(&priv->selq, &new_chs->params); - err = mlx5e_open_channels(priv, &new_chs); + err = mlx5e_open_channels(priv, new_chs); if (err) goto err_cancel_selq; - err = mlx5e_switch_priv_channels(priv, &new_chs, preactivate, context); + err = mlx5e_switch_priv_channels(priv, new_chs, preactivate, context); if (err) goto err_close; + kfree(new_chs); return 0; err_close: - mlx5e_close_channels(&new_chs); + mlx5e_close_channels(new_chs); err_cancel_selq: mlx5e_selq_cancel(&priv->selq); + kfree(new_chs); return err; }