On Mon, Sep 02, 2024 at 01:37:03PM +0300, Leon Romanovsky wrote:
> From: Maher Sanalla <msanalla@xxxxxxxxxx>
>
> When allocating MRs, it is not definitive whether they will be used for
> peer-to-peer transactions or for other usecases.
>
> Since peer-to-peer transactions benefit significantly from ATS
> performance-wise, enable ATS on newly-allocated MRs when supported.
?
On the upstream kernel there is no way for a peer-to-peer page to get
into the normal MRs, is there? So why do this?
This also makes all CPU memory run slower.
> Signed-off-by: Maher Sanalla <msanalla@xxxxxxxxxx>
> Reviewed-by: Gal Shalom <galshalom@xxxxxxxxxx>
> Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxx>
> ---
> drivers/infiniband/hw/mlx5/mr.c | 5 +++++
> 1 file changed, 5 insertions(+)
shoulnd't g
Are yo sur? ATS has a big negative impact if it isn't required..
> diff --git a/drivers/infiniband/hw/mlx5/mr.c b/drivers/infiniband/hw/mlx5/mr.c
> index 3320238a0eb8..7d8c58f803ac 100644
> --- a/drivers/infiniband/hw/mlx5/mr.c
> +++ b/drivers/infiniband/hw/mlx5/mr.c
> @@ -1080,6 +1080,7 @@ struct ib_mr *mlx5_ib_get_dma_mr(struct ib_pd *pd, int acc)
> MLX5_SET(mkc, mkc, length64, 1);
> set_mkc_access_pd_addr_fields(mkc, acc | IB_ACCESS_RELAXED_ORDERING, 0,
> pd);
> + MLX5_SET(mkc, mkc, ma_translation_mode, MLX5_CAP_GEN(dev->mdev, ats));
>
> err = mlx5_ib_create_mkey(dev, &mr->mmkey, in, inlen);
> if (err)
> @@ -2218,6 +2219,7 @@ int mlx5_ib_dereg_mr(struct ib_mr *ibmr, struct ib_udata *udata)
> static void mlx5_set_umr_free_mkey(struct ib_pd *pd, u32 *in, int ndescs,
> int access_mode, int page_shift)
> {
> + struct mlx5_ib_dev *dev = to_mdev(pd->device);
> void *mkc;
>
> mkc = MLX5_ADDR_OF(create_mkey_in, in, memory_key_mkey_entry);
> @@ -2230,6 +2232,9 @@ static void mlx5_set_umr_free_mkey(struct ib_pd *pd, u32 *in, int ndescs,
> MLX5_SET(mkc, mkc, access_mode_4_2, (access_mode >> 2) & 0x7);
> MLX5_SET(mkc, mkc, umr_en, 1);
> MLX5_SET(mkc, mkc, log_page_size, page_shift);
> + if (access_mode == MLX5_MKC_ACCESS_MODE_PA ||
> + access_mode == MLX5_MKC_ACCESS_MODE_MTT)
> + MLX5_SET(mkc, mkc, ma_translation_mode, MLX5_CAP_GEN(dev->mdev, ats));
> }
This doesn't look right, isn't it re-introducing the bug I fixed about
mis-caching the ATS property?
drivers/infiniband/hw/mlx5/mr.c: MLX5_SET(mkc, mkc, ma_translation_mode, !!ent->rb_key.ats);
?
Or at least shouldn't all the old ATS stuff be ripped out if it is
being forced on for every mkey?
Jasan
