Skip to contentSkip to conversation

ODP Fixes

PATCH Applied Leon Romanovsky 2 participants 5 messages Dec 22, 2019 – Jan 3, 2020

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover ODP Fixes

From: Leon Romanovsky <[email protected]>

Changelog:
v0 -> v1: https://lore.kernel.org/linux-rdma/[email protected]
* Added description of fix to patch #1
* Added Jason's ROB tag to patch #2
* Dropped changes in page fault handler in patch #3

-------------------------------------------------------------------------------------------
Please find below three patches that fix ODP flow.

Thanks

Artemy Kovalyov (1):
IB/mlx5: Unify ODP MR code paths to allow extra flexibility

Yishai Hadas (2):
IB/core: Fix ODP get user pages flow IB/core: Fix ODP with IB_ACCESS_HUGETLB handling

 drivers/infiniband/core/umem_odp.c   | 23 +++--------
 drivers/infiniband/hw/mlx5/mem.c     | 25 ------------
 drivers/infiniband/hw/mlx5/mlx5_ib.h |  9 ++---
 drivers/infiniband/hw/mlx5/mr.c      | 58 +++++++++++-----------------
 drivers/infiniband/hw/mlx5/odp.c     | 42 +++++++++++++++++++-
 5 files changed, 72 insertions(+), 85 deletions(-)

--
2.20.1

#1 lore raw

PATCH 1/3 IB/mlx5: Unify ODP MR code paths to allow extra flexibility

From: Artemy Kovalyov <[email protected]>

Building MR translation table in ODP case requires additional flexibility, namely random access to DMA addresses. Make both direct and indirect ODP MR use same code path, separated from non-ODP MR code path.

This change fixed the broken __mlx5_ib_populate_pas() shift calculation.

Fixes d2183c6f1958 ("RDMA/umem: Move page_shift from ib_umem to ib_odp_umem")
---
 drivers/infiniband/hw/mlx5/mem.c     | 25 ------------
 drivers/infiniband/hw/mlx5/mlx5_ib.h |  9 ++---
 drivers/infiniband/hw/mlx5/mr.c      | 58 +++++++++++-----------------
 drivers/infiniband/hw/mlx5/odp.c     | 42 +++++++++++++++++++-
 4 files changed, 67 insertions(+), 67 deletions(-)
drivers/infiniband/hw/mlx5/mem.c +0 -25
index 048f4e974a61..b90a3649e7d1 100644
@@ -101,18 +101,6 @@ void mlx5_ib_cont_pages(struct ib_umem *umem, u64 addr,
 	*count = i;
 }
 
-static u64 umem_dma_to_mtt(dma_addr_t umem_dma)
-{
-	u64 mtt_entry = umem_dma & ODP_DMA_ADDR_MASK;
-
-	if (umem_dma & ODP_READ_ALLOWED_BIT)
-		mtt_entry |= MLX5_IB_MTT_READ;
-	if (umem_dma & ODP_WRITE_ALLOWED_BIT)
-		mtt_entry |= MLX5_IB_MTT_WRITE;
-
-	return mtt_entry;
-}
-
 /*
  * Populate the given array with bus addresses from the umem.
  *
@@ -139,19 +127,6 @@ void __mlx5_ib_populate_pas(struct mlx5_ib_dev *dev, struct ib_umem *umem,
 	struct scatterlist *sg;
 	int entry;
 
-	if (umem->is_odp) {
-		WARN_ON(shift != 0);
-		WARN_ON(access_flags != (MLX5_IB_MTT_READ | MLX5_IB_MTT_WRITE));
-
-		for (i = 0; i < num_pages; ++i) {
-			dma_addr_t pa =
-				to_ib_umem_odp(umem)->dma_list[offset + i];
-
-			pas[i] = cpu_to_be64(umem_dma_to_mtt(pa));
-		}
-		return;
-	}
-
 	i = 0;
 	for_each_sg(umem->sg_head.sgl, sg, umem->nmap, entry) {
 		len = sg_dma_len(sg) >> PAGE_SHIFT;
drivers/infiniband/hw/mlx5/mlx5_ib.h +4 -5
index b06f32ff5748..927948328184 100644
@@ -1276,8 +1276,8 @@ void mlx5_ib_odp_cleanup_one(struct mlx5_ib_dev *ibdev);
 int __init mlx5_ib_odp_init(void);
 void mlx5_ib_odp_cleanup(void);
 void mlx5_odp_init_mr_cache_entry(struct mlx5_cache_ent *ent);
-void mlx5_odp_populate_klm(struct mlx5_klm *pklm, size_t offset,
-			   size_t nentries, struct mlx5_ib_mr *mr, int flags);
+void mlx5_odp_populate_xlt(void *xlt, size_t idx, size_t nentries,
+			   struct mlx5_ib_mr *mr, int flags);
 
 int mlx5_ib_advise_mr_prefetch(struct ib_pd *pd,
 			       enum ib_uverbs_advise_mr_advice advice,
@@ -1293,9 +1293,8 @@ static inline void mlx5_ib_odp_cleanup_one(struct mlx5_ib_dev *ibdev) {}
 static inline int mlx5_ib_odp_init(void) { return 0; }
 static inline void mlx5_ib_odp_cleanup(void)				    {}
 static inline void mlx5_odp_init_mr_cache_entry(struct mlx5_cache_ent *ent) {}
-static inline void mlx5_odp_populate_klm(struct mlx5_klm *pklm, size_t offset,
-					 size_t nentries, struct mlx5_ib_mr *mr,
-					 int flags) {}
+static inline void mlx5_odp_populate_xlt(void *xlt, size_t idx, size_t nentries,
+					 struct mlx5_ib_mr *mr, int flags) {}
 
 static inline int
 mlx5_ib_advise_mr_prefetch(struct ib_pd *pd,
drivers/infiniband/hw/mlx5/mr.c +23 -35

85 lines not on this page · raw message

drivers/infiniband/hw/mlx5/odp.c +40 -2

54 lines not on this page · raw message

Signature · 1 line
2.20.1
#2 lore raw

PATCH 2/3 IB/core: Fix ODP get user pages flow

In reply to Leon Romanovsky

From: Yishai Hadas <[email protected]>

When calling get_user_pages_remote() need to work with granularity of PAGE_SIZE.

Fix the calculation of how many entries can be read in one call to consider that.

Fixes 403cd12e2cf7 ("IB/umem: Add contiguous ODP support")
---
 drivers/infiniband/core/umem_odp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/infiniband/core/umem_odp.c +1 -1
index e42d44e501fd..2e9ee7adab13 100644
@@ -440,7 +440,7 @@ int ib_umem_odp_map_dma_pages(struct ib_umem_odp *umem_odp, u64 user_virt,
 
 	while (bcnt > 0) {
 		const size_t gup_num_pages = min_t(size_t,
-				(bcnt + BIT(page_shift) - 1) >> page_shift,
+				ALIGN(bcnt, PAGE_SIZE) / PAGE_SIZE,
 				PAGE_SIZE / sizeof(struct page *));
 
 		down_read(&owning_mm->mmap_sem);
Signature · 1 line
2.20.1
#3 lore raw

PATCH 3/3 IB/core: Fix ODP with IB_ACCESS_HUGETLB handling

In reply to Leon Romanovsky

From: Yishai Hadas <[email protected]>

As VMAs for a given range might not be available as part of the registration phase in ODP.

ib_init_umem_odp() considered the expected page shift value that was previously set and initializes its internals accordingly.

If memory doesn't backed by physical contiguous pages aligned to hugepage boundary an error will be set as part of the page fault flow once be detected.

Fixes 0008b84ea9af ("IB/umem: Add support to huge ODP")
---
 drivers/infiniband/core/umem_odp.c | 21 ++++-----------------
 1 file changed, 4 insertions(+), 17 deletions(-)
drivers/infiniband/core/umem_odp.c +4 -17
index 2e9ee7adab13..f42fa31c24a2 100644
@@ -241,22 +241,10 @@ struct ib_umem_odp *ib_umem_odp_get(struct ib_udata *udata, unsigned long addr,
 	umem_odp->umem.owning_mm = mm = current->mm;
 	umem_odp->notifier.ops = ops;
 
-	umem_odp->page_shift = PAGE_SHIFT;
-	if (access & IB_ACCESS_HUGETLB) {
-		struct vm_area_struct *vma;
-		struct hstate *h;
-
-		down_read(&mm->mmap_sem);
-		vma = find_vma(mm, ib_umem_start(umem_odp));
-		if (!vma || !is_vm_hugetlb_page(vma)) {
-			up_read(&mm->mmap_sem);
-			ret = -EINVAL;
-			goto err_free;
-		}
-		h = hstate_vma(vma);
-		umem_odp->page_shift = huge_page_shift(h);
-		up_read(&mm->mmap_sem);
-	}
+	if (access & IB_ACCESS_HUGETLB)
+		umem_odp->page_shift = HPAGE_SHIFT;
+	else
+		umem_odp->page_shift = PAGE_SHIFT;
 
 	umem_odp->tgid = get_task_pid(current->group_leader, PIDTYPE_PID);
 	ret = ib_init_umem_odp(umem_odp, ops);
@@ -266,7 +254,6 @@ struct ib_umem_odp *ib_umem_odp_get(struct ib_udata *udata, unsigned long addr,
 
 err_put_pid:
 	put_pid(umem_odp->tgid);
-err_free:
 	kfree(umem_odp);
 	return ERR_PTR(ret);
 }
Signature · 1 line
2.20.1
#4 lore raw
In reply to Leon Romanovsky
Quoted 9 lines · Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Changelog:
v0 -> v1: https://lore.kernel.org/linux-rdma/[email protected]
* Added description of fix to patch #1
* Added Jason's ROB tag to patch #2
* Dropped changes in page fault handler in patch #3

Please find below three patches that fix ODP flow.

Applied to for-next, not rc

Quoted 2 lines

Artemy Kovalyov (1):
IB/mlx5: Unify ODP MR code paths to allow extra flexibility

This is the only one that really fixes a bug, and it is too big for rc5 at this point.

Quoted 3 lines

Yishai Hadas (2):
IB/core: Fix ODP get user pages flow IB/core: Fix ODP with IB_ACCESS_HUGETLB handling

While these are needed fixes they are not bugs...

Thanks,
Jason