On Fri, 8 May 2026 09:19:05 +0530 Ratheesh Kannoth <[email protected]> wrote:
> union devlink_param_value grew when U64 array parameters were added. > Keeping a four-element array of that union in > mlx5e_pcie_cong_get_thresh_config() inflated the stack frame past the > -Wframe-larger-than limit. > > Read each driverinit value into a single reused union, then store the > four u16 thresholds in struct mlx5e_pcie_cong_thresh field order via a > temporary u16 pointer to config. > > Signed-off-by: Ratheesh Kannoth <[email protected]> > --- > .../mellanox/mlx5/core/en/pcie_cong_event.c | 34 +++++++++++-------- > 1 file changed, 19 insertions(+), 15 deletions(-) > > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/pcie_cong_event.c > b/drivers/net/ethernet/mellanox/mlx5/core/en/pcie_cong_event.c > index 2eb666a46f39..88e76be3a73d 100644 > --- a/drivers/net/ethernet/mellanox/mlx5/core/en/pcie_cong_event.c > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/pcie_cong_event.c > @@ -252,28 +252,32 @@ static int > mlx5e_pcie_cong_get_thresh_config(struct mlx5_core_dev *dev, > struct mlx5e_pcie_cong_thresh *config) > { > + enum { > + INBOUND_HIGH, > + INBOUND_LOW, > + OUTBOUND_HIGH, > + OUTBOUND_LOW, > + }; > + > u32 ids[4] = { Someone will suggest that should be 'static const'. It may make the code smaller. > - MLX5_DEVLINK_PARAM_ID_PCIE_CONG_IN_LOW, > - MLX5_DEVLINK_PARAM_ID_PCIE_CONG_IN_HIGH, > - MLX5_DEVLINK_PARAM_ID_PCIE_CONG_OUT_LOW, > - MLX5_DEVLINK_PARAM_ID_PCIE_CONG_OUT_HIGH, > + [INBOUND_LOW] = MLX5_DEVLINK_PARAM_ID_PCIE_CONG_IN_LOW, > + [INBOUND_HIGH] = MLX5_DEVLINK_PARAM_ID_PCIE_CONG_IN_HIGH, > + [OUTBOUND_LOW] = MLX5_DEVLINK_PARAM_ID_PCIE_CONG_OUT_LOW, > + [OUTBOUND_HIGH] = MLX5_DEVLINK_PARAM_ID_PCIE_CONG_OUT_HIGH, > }; > - struct devlink *devlink = priv_to_devlink(dev); > - union devlink_param_value val[4]; > > - for (int i = 0; i < 4; i++) { > - u32 id = ids[i]; > - int err; > + struct devlink *devlink = priv_to_devlink(dev); > + union devlink_param_value val; > + u16 *dst = (u16 *)config; You can't do that - far too fragile. Maybe &config->inbound_low - but even that assumes the values are in order. A safer way would be using a temporary 'u16 val16[4]'. (Or even overwrite ids[] with the result.) But the code might even be smaller if you just unroll the loop: err = devl_param_driverinit_value_get(devlink, MLX5_DEVLINK_PARAM_ID_PCIE_CONG_IN_LOW, &val); if (err) return err; config->inbound_low = val.vu16; err = devl_param_driverinit_value_get(devlink, MLX5_DEVLINK_PARAM_ID_PCIE_CONG_IN_HIGH, &val); if (err) return err; config->inbound_high = val.vu16; err = devl_param_driverinit_value_get(devlink, MLX5_DEVLINK_PARAM_ID_PCIE_CONG_OUT_LOW, &val); if (err) return err; config->outbound_low = val.vu16; err = devl_param_driverinit_value_get(devlink, MLX5_DEVLINK_PARAM_ID_PCIE_CONG_OUT_HIGH, &val); if (err) return err; config->outbound_high = val.vu16; -- David > + int err; > > - err = devl_param_driverinit_value_get(devlink, id, &val[i]); > + for (int i = 0; i < ARRAY_SIZE(ids); i++) { > + err = devl_param_driverinit_value_get(devlink, ids[i], &val); > if (err) > return err; > - } > > - config->inbound_low = val[0].vu16; > - config->inbound_high = val[1].vu16; > - config->outbound_low = val[2].vu16; > - config->outbound_high = val[3].vu16; > + dst[i] = val.vu16; > + } > > return 0; > }
