Re: [PATCH vhost 2/7] vdpa/mlx5: Delete direct MKEYs in parallel

From: Eugenio Perez Martin
Date: Thu Aug 29 2024 - 09:43:38 EST


On Wed, Aug 21, 2024 at 1:42 PM Dragos Tatulea <dtatulea@xxxxxxxxxx> wrote:
>
> Use the async interface to issue MTT MKEY deletion.
>
> This makes destroy_user_mr() on average 8x times faster.
> This number is also dependent on the size of the MR being
> deleted.
>
> Signed-off-by: Dragos Tatulea <dtatulea@xxxxxxxxxx>
> Reviewed-by: Cosmin Ratiu <cratiu@xxxxxxxxxx>
> ---
> drivers/vdpa/mlx5/core/mr.c | 66 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 66 insertions(+)
>
> diff --git a/drivers/vdpa/mlx5/core/mr.c b/drivers/vdpa/mlx5/core/mr.c
> index 66e6a15f823f..8cedf2969991 100644
> --- a/drivers/vdpa/mlx5/core/mr.c
> +++ b/drivers/vdpa/mlx5/core/mr.c
> @@ -55,6 +55,11 @@ struct mlx5_create_mkey_mem {
> DECLARE_FLEX_ARRAY(__be64, mtt);
> };
>
> +struct mlx5_destroy_mkey_mem {
> + u8 out[MLX5_ST_SZ_BYTES(destroy_mkey_out)];
> + u8 in[MLX5_ST_SZ_BYTES(destroy_mkey_in)];
> +};
> +
> static void fill_create_direct_mr(struct mlx5_vdpa_dev *mvdev,
> struct mlx5_vdpa_direct_mr *mr,
> struct mlx5_create_mkey_mem *mem)
> @@ -91,6 +96,17 @@ static void create_direct_mr_end(struct mlx5_vdpa_dev *mvdev,
> mr->mr = mlx5_idx_to_mkey(mkey_index);
> }
>
> +static void fill_destroy_direct_mr(struct mlx5_vdpa_dev *mvdev,
> + struct mlx5_vdpa_direct_mr *mr,
> + struct mlx5_destroy_mkey_mem *mem)
> +{
> + void *in = &mem->in;
> +

Nit, isn't this declaration redundant? Looking at the definition of
MLX5_SET, the second argument is casted to (__be32 *) anyway.

> + MLX5_SET(destroy_mkey_in, in, uid, mvdev->res.uid);
> + MLX5_SET(destroy_mkey_in, in, opcode, MLX5_CMD_OP_DESTROY_MKEY);
> + MLX5_SET(destroy_mkey_in, in, mkey_index, mlx5_mkey_to_idx(mr->mr));
> +}
> +
> static void destroy_direct_mr(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_direct_mr *mr)
> {
> if (!mr->mr)
> @@ -255,6 +271,55 @@ static int create_direct_keys(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_mr *
> return err;
> }
>
> +static int destroy_direct_keys(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_mr *mr)
> +{
> + struct mlx5_destroy_mkey_mem *cmd_mem;
> + struct mlx5_vdpa_async_cmd *cmds;
> + struct mlx5_vdpa_direct_mr *dmr;
> + int err = 0;
> + int i = 0;
> +
> + cmds = kvcalloc(mr->num_directs, sizeof(*cmds), GFP_KERNEL);
> + cmd_mem = kvcalloc(mr->num_directs, sizeof(*cmd_mem), GFP_KERNEL);
> + if (!cmds || !cmd_mem) {
> + err = -ENOMEM;
> + goto done;
> + }
> +
> + list_for_each_entry(dmr, &mr->head, list) {
> + cmds[i].out = cmd_mem[i].out;
> + cmds[i].outlen = sizeof(cmd_mem[i].out);
> + cmds[i].in = cmd_mem[i].in;
> + cmds[i].inlen = sizeof(cmd_mem[i].in);
> + fill_destroy_direct_mr(mvdev, dmr, &cmd_mem[i]);
> + i++;
> + }
> +
> + err = mlx5_vdpa_exec_async_cmds(mvdev, cmds, mr->num_directs);
> + if (err) {
> +
> + mlx5_vdpa_err(mvdev, "error issuing MTT mkey deletion for direct mrs: %d\n", err);
> + goto done;
> + }
> +
> + i = 0;
> + list_for_each_entry(dmr, &mr->head, list) {
> + struct mlx5_vdpa_async_cmd *cmd = &cmds[i++];
> +
> + dmr->mr = 0;
> + if (cmd->err) {
> + err = err ? err : cmd->err;
> + mlx5_vdpa_err(mvdev, "error deleting MTT mkey [0x%llx, 0x%llx]: %d\n",
> + dmr->start, dmr->end, cmd->err);
> + }
> + }
> +
> +done:
> + kvfree(cmd_mem);
> + kvfree(cmds);

Same nitpick here as in the previous patch about the Scope-based
Cleanup Helpers. Either way,

Acked-by: Eugenio Pérez <eperezma@xxxxxxxxxx>

> + return err;
> +}
> +
> static int create_indirect_key(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_mr *mr)
> {
> int inlen;
> @@ -563,6 +628,7 @@ static void destroy_user_mr(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_mr *mr
> struct mlx5_vdpa_direct_mr *n;
>
> destroy_indirect_key(mvdev, mr);
> + destroy_direct_keys(mvdev, mr);
> list_for_each_entry_safe_reverse(dmr, n, &mr->head, list) {
> list_del_init(&dmr->list);
> unmap_direct_mr(mvdev, dmr);
> --
> 2.45.1
>