2021-12-22 10:14:17

by Maor Gottlieb

[permalink] [raw]
Subject: [PATCH rdma-rc] Revert "RDMA/mlx5: Fix releasing unallocated memory in dereg MR flow"

This patch is not the full fix and still causes to call traces
during mlx5_ib_dereg_mr().

This reverts commit f0ae4afe3d35e67db042c58a52909e06262b740f.

Fixes: f0ae4afe3d35 ("RDMA/mlx5: Fix releasing unallocated memory in dereg MR flow")
Signed-off-by: Maor Gottlieb <[email protected]>
---
drivers/infiniband/hw/mlx5/mlx5_ib.h | 6 +++---
drivers/infiniband/hw/mlx5/mr.c | 26 ++++++++++++++------------
2 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h b/drivers/infiniband/hw/mlx5/mlx5_ib.h
index 4a7a56ed740b..e636e954f6bf 100644
--- a/drivers/infiniband/hw/mlx5/mlx5_ib.h
+++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
@@ -664,6 +664,7 @@ struct mlx5_ib_mr {

/* User MR data */
struct mlx5_cache_ent *cache_ent;
+ struct ib_umem *umem;

/* This is zero'd when the MR is allocated */
union {
@@ -675,7 +676,7 @@ struct mlx5_ib_mr {
struct list_head list;
};

- /* Used only by kernel MRs */
+ /* Used only by kernel MRs (umem == NULL) */
struct {
void *descs;
void *descs_alloc;
@@ -696,9 +697,8 @@ struct mlx5_ib_mr {
int data_length;
};

- /* Used only by User MRs */
+ /* Used only by User MRs (umem != NULL) */
struct {
- struct ib_umem *umem;
unsigned int page_shift;
/* Current access_flags */
int access_flags;
diff --git a/drivers/infiniband/hw/mlx5/mr.c b/drivers/infiniband/hw/mlx5/mr.c
index 63e2129f1142..157d862fb864 100644
--- a/drivers/infiniband/hw/mlx5/mr.c
+++ b/drivers/infiniband/hw/mlx5/mr.c
@@ -1904,18 +1904,19 @@ mlx5_alloc_priv_descs(struct ib_device *device,
return ret;
}

-static void mlx5_free_priv_descs(struct mlx5_ib_mr *mr)
+static void
+mlx5_free_priv_descs(struct mlx5_ib_mr *mr)
{
- struct mlx5_ib_dev *dev = to_mdev(mr->ibmr.device);
- int size = mr->max_descs * mr->desc_size;
-
- if (!mr->descs)
- return;
+ if (!mr->umem && mr->descs) {
+ struct ib_device *device = mr->ibmr.device;
+ int size = mr->max_descs * mr->desc_size;
+ struct mlx5_ib_dev *dev = to_mdev(device);

- dma_unmap_single(&dev->mdev->pdev->dev, mr->desc_map, size,
- DMA_TO_DEVICE);
- kfree(mr->descs_alloc);
- mr->descs = NULL;
+ dma_unmap_single(&dev->mdev->pdev->dev, mr->desc_map, size,
+ DMA_TO_DEVICE);
+ kfree(mr->descs_alloc);
+ mr->descs = NULL;
+ }
}

int mlx5_ib_dereg_mr(struct ib_mr *ibmr, struct ib_udata *udata)
@@ -1991,8 +1992,7 @@ int mlx5_ib_dereg_mr(struct ib_mr *ibmr, struct ib_udata *udata)
if (mr->cache_ent) {
mlx5_mr_cache_free(dev, mr);
} else {
- if (!udata)
- mlx5_free_priv_descs(mr);
+ mlx5_free_priv_descs(mr);
kfree(mr);
}
return 0;
@@ -2079,6 +2079,7 @@ static struct mlx5_ib_mr *mlx5_ib_alloc_pi_mr(struct ib_pd *pd,
if (err)
goto err_free_in;

+ mr->umem = NULL;
kfree(in);

return mr;
@@ -2205,6 +2206,7 @@ static struct ib_mr *__mlx5_ib_alloc_mr(struct ib_pd *pd,
}

mr->ibmr.device = pd->device;
+ mr->umem = NULL;

switch (mr_type) {
case IB_MR_TYPE_MEM_REG:
--
2.25.4



2021-12-23 09:37:06

by Leon Romanovsky

[permalink] [raw]
Subject: Re: [PATCH rdma-rc] Revert "RDMA/mlx5: Fix releasing unallocated memory in dereg MR flow"

On Wed, Dec 22, 2021 at 12:13:12PM +0200, Maor Gottlieb wrote:
> This patch is not the full fix and still causes to call traces
> during mlx5_ib_dereg_mr().
>
> This reverts commit f0ae4afe3d35e67db042c58a52909e06262b740f.
>
> Fixes: f0ae4afe3d35 ("RDMA/mlx5: Fix releasing unallocated memory in dereg MR flow")
> Signed-off-by: Maor Gottlieb <[email protected]>
> ---
> drivers/infiniband/hw/mlx5/mlx5_ib.h | 6 +++---
> drivers/infiniband/hw/mlx5/mr.c | 26 ++++++++++++++------------
> 2 files changed, 17 insertions(+), 15 deletions(-)
>

Thanks,
Acked-by: Leon Romanovsky <[email protected]>

2021-12-24 02:51:52

by Jason Gunthorpe

[permalink] [raw]
Subject: Re: [PATCH rdma-rc] Revert "RDMA/mlx5: Fix releasing unallocated memory in dereg MR flow"

On Wed, Dec 22, 2021 at 12:13:12PM +0200, Maor Gottlieb wrote:
> This patch is not the full fix and still causes to call traces
> during mlx5_ib_dereg_mr().
>
> This reverts commit f0ae4afe3d35e67db042c58a52909e06262b740f.
>
> Fixes: f0ae4afe3d35 ("RDMA/mlx5: Fix releasing unallocated memory in dereg MR flow")
> Signed-off-by: Maor Gottlieb <[email protected]>
> ---
> drivers/infiniband/hw/mlx5/mlx5_ib.h | 6 +++---
> drivers/infiniband/hw/mlx5/mr.c | 26 ++++++++++++++------------
> 2 files changed, 17 insertions(+), 15 deletions(-)

Huh? Why is this such a problem to fix the missing udatas?

Jason

2021-12-24 06:01:04

by Leon Romanovsky

[permalink] [raw]
Subject: Re: [PATCH rdma-rc] Revert "RDMA/mlx5: Fix releasing unallocated memory in dereg MR flow"

On Thu, Dec 23, 2021 at 10:51:45PM -0400, Jason Gunthorpe wrote:
> On Wed, Dec 22, 2021 at 12:13:12PM +0200, Maor Gottlieb wrote:
> > This patch is not the full fix and still causes to call traces
> > during mlx5_ib_dereg_mr().
> >
> > This reverts commit f0ae4afe3d35e67db042c58a52909e06262b740f.
> >
> > Fixes: f0ae4afe3d35 ("RDMA/mlx5: Fix releasing unallocated memory in dereg MR flow")
> > Signed-off-by: Maor Gottlieb <[email protected]>
> > ---
> > drivers/infiniband/hw/mlx5/mlx5_ib.h | 6 +++---
> > drivers/infiniband/hw/mlx5/mr.c | 26 ++++++++++++++------------
> > 2 files changed, 17 insertions(+), 15 deletions(-)
>
> Huh? Why is this such a problem to fix the missing udatas?

Because of internal structure of Nvidia/Mellanox, we are having very
hard time to properly test any fix in ULP area.

In addition, the vacation season doesn't help either.

Feel free to approach me or Maor offline and we will explain you the
non-technical details behind this revert.

Thanks

>
> Jason

2022-01-05 17:00:29

by Jason Gunthorpe

[permalink] [raw]
Subject: Re: [PATCH rdma-rc] Revert "RDMA/mlx5: Fix releasing unallocated memory in dereg MR flow"

On Wed, Dec 22, 2021 at 12:13:12PM +0200, Maor Gottlieb wrote:
> This patch is not the full fix and still causes to call traces
> during mlx5_ib_dereg_mr().
>
> This reverts commit f0ae4afe3d35e67db042c58a52909e06262b740f.
>
> Fixes: f0ae4afe3d35 ("RDMA/mlx5: Fix releasing unallocated memory in dereg MR flow")
> Signed-off-by: Maor Gottlieb <[email protected]>
> Acked-by: Leon Romanovsky <[email protected]>
> ---
> drivers/infiniband/hw/mlx5/mlx5_ib.h | 6 +++---
> drivers/infiniband/hw/mlx5/mr.c | 26 ++++++++++++++------------
> 2 files changed, 17 insertions(+), 15 deletions(-)

Applied to for-rc, lets get the correct fix for the next cycle.

Thanks
Jason