On Mon, 2020-04-13 at 16:36 +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@xxxxxxxxxxxx>
>
> Reduce the amount of kzalloc/kfree cycles by allocating
> command structure in the parent function and leverage the
> knowledge that set_caps() is called for HCA capabilities
> only with specific HW structure as parameter to calculate
> mailbox size.
>
> Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> ---
> .../net/ethernet/mellanox/mlx5/core/main.c | 66 +++++++--------
> ----
> 1 file changed, 24 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c
> b/drivers/net/ethernet/mellanox/mlx5/core/main.c
> index 7af4210c1b96..8b9182add689 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
> @@ -407,20 +407,19 @@ int mlx5_core_get_caps(struct mlx5_core_dev
> *dev, enum mlx5_cap_type cap_type)
> return mlx5_core_get_caps_mode(dev, cap_type,
> HCA_CAP_OPMOD_GET_MAX);
> }
>
> -static int set_caps(struct mlx5_core_dev *dev, void *in, int in_sz,
> int opmod)
> +static int set_caps(struct mlx5_core_dev *dev, void *in, int opmod)
> {
> - u32 out[MLX5_ST_SZ_DW(set_hca_cap_out)] = {0};
> + u32 out[MLX5_ST_SZ_DW(set_hca_cap_out)] = {};
>
> MLX5_SET(set_hca_cap_in, in, opcode, MLX5_CMD_OP_SET_HCA_CAP);
> MLX5_SET(set_hca_cap_in, in, op_mod, opmod << 1);
> - return mlx5_cmd_exec(dev, in, in_sz, out, sizeof(out));
> + return mlx5_cmd_exec(dev, in, MLX5_ST_SZ_BYTES(set_hca_cap_in),
> out,
> + sizeof(out));
> }
>
> -static int handle_hca_cap_atomic(struct mlx5_core_dev *dev)
> +static int handle_hca_cap_atomic(struct mlx5_core_dev *dev, void
> *set_ctx)
> {
> - void *set_ctx;
> void *set_hca_cap;
> - int set_sz = MLX5_ST_SZ_BYTES(set_hca_cap_in);
> int req_endianness;
> int err;
>
> @@ -439,27 +438,19 @@ static int handle_hca_cap_atomic(struct
> mlx5_core_dev *dev)
> if (req_endianness != MLX5_ATOMIC_REQ_MODE_HOST_ENDIANNESS)
> return 0;
>
> - set_ctx = kzalloc(set_sz, GFP_KERNEL);
> - if (!set_ctx)
> - return -ENOMEM;
> -
> set_hca_cap = MLX5_ADDR_OF(set_hca_cap_in, set_ctx,
> capability);
>
> /* Set requestor to host endianness */
> MLX5_SET(atomic_caps, set_hca_cap,
> atomic_req_8B_endianness_mode,
> MLX5_ATOMIC_REQ_MODE_HOST_ENDIANNESS);
>
> - err = set_caps(dev, set_ctx, set_sz,
> MLX5_SET_HCA_CAP_OP_MOD_ATOMIC);
> -
> - kfree(set_ctx);
> + err = set_caps(dev, set_ctx, MLX5_SET_HCA_CAP_OP_MOD_ATOMIC);
> return err;
> }
>
> -static int handle_hca_cap_odp(struct mlx5_core_dev *dev)
> +static int handle_hca_cap_odp(struct mlx5_core_dev *dev, void
> *set_ctx)
> {
> void *set_hca_cap;
> - void *set_ctx;
> - int set_sz;
> bool do_set = false;
> int err;
>
> @@ -471,11 +462,6 @@ static int handle_hca_cap_odp(struct
> mlx5_core_dev *dev)
> if (err)
> return err;
>
> - set_sz = MLX5_ST_SZ_BYTES(set_hca_cap_in);
> - set_ctx = kzalloc(set_sz, GFP_KERNEL);
> - if (!set_ctx)
> - return -ENOMEM;
> -
> set_hca_cap = MLX5_ADDR_OF(set_hca_cap_in, set_ctx,
> capability);
> memcpy(set_hca_cap, dev->caps.hca_cur[MLX5_CAP_ODP],
> MLX5_ST_SZ_BYTES(odp_cap));
> @@ -505,29 +491,20 @@ static int handle_hca_cap_odp(struct
> mlx5_core_dev *dev)
> ODP_CAP_SET_MAX(dev, dc_odp_caps.atomic);
>
> if (do_set)
assigning to err is now redundant in all of the functions and the next
patch.
you can do early exits when required and then just "return set_caps();"
in this example:
if (!do_set)
return 0;
return set_caps(...);
> - err = set_caps(dev, set_ctx, set_sz,
> - MLX5_SET_HCA_CAP_OP_MOD_ODP);
> -
> - kfree(set_ctx);
> + err = set_caps(dev, set_ctx,
> MLX5_SET_HCA_CAP_OP_MOD_ODP);
>
> return err;
> }
>
> -static int handle_hca_cap(struct mlx5_core_dev *dev)
> +static int handle_hca_cap(struct mlx5_core_dev *dev, void *set_ctx)
> {
> - void *set_ctx = NULL;
> struct mlx5_profile *prof = dev->profile;
> - int err = -ENOMEM;
> - int set_sz = MLX5_ST_SZ_BYTES(set_hca_cap_in);
> void *set_hca_cap;
> -
> - set_ctx = kzalloc(set_sz, GFP_KERNEL);
> - if (!set_ctx)
> - goto query_ex;
> + int err;
>
> err = mlx5_core_get_caps(dev, MLX5_CAP_GENERAL);
> if (err)
> - goto query_ex;
> + return err;
>
> set_hca_cap = MLX5_ADDR_OF(set_hca_cap_in, set_ctx,
> capability);
> @@ -578,37 +555,42 @@ static int handle_hca_cap(struct mlx5_core_dev
> *dev)
> num_vhca_ports,
> MLX5_CAP_GEN_MAX(dev, num_vhca_ports));
>
> - err = set_caps(dev, set_ctx, set_sz,
> - MLX5_SET_HCA_CAP_OP_MOD_GENERAL_DEVICE);
> -
> -query_ex:
> - kfree(set_ctx);
> + err = set_caps(dev, set_ctx,
> MLX5_SET_HCA_CAP_OP_MOD_GENERAL_DEVICE);
> return err;
just return set_caps();
other than this the series is ok,
Acked-by: Saeed Mahameed <saeedm@xxxxxxxxxxxx>
> }
>
> static int set_hca_cap(struct mlx5_core_dev *dev)
> {
> + int set_sz = MLX5_ST_SZ_BYTES(set_hca_cap_in);
> + void *set_ctx;
> int err;
>
> - err = handle_hca_cap(dev);
> + set_ctx = kzalloc(set_sz, GFP_KERNEL);
> + if (!set_ctx)
> + return -ENOMEM;
> +
> + err = handle_hca_cap(dev, set_ctx);
> if (err) {
> mlx5_core_err(dev, "handle_hca_cap failed\n");
> goto out;
> }
>
> - err = handle_hca_cap_atomic(dev);
> + memset(set_ctx, 0, set_sz);
> + err = handle_hca_cap_atomic(dev, set_ctx);
> if (err) {
> mlx5_core_err(dev, "handle_hca_cap_atomic failed\n");
> goto out;
> }
>
> - err = handle_hca_cap_odp(dev);
> + memset(set_ctx, 0, set_sz);
> + err = handle_hca_cap_odp(dev, set_ctx);
> if (err) {
> mlx5_core_err(dev, "handle_hca_cap_odp failed\n");
> goto out;
> }
>
> out:
> + kfree(set_ctx);
> return err;
> }
>
