All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH rdma-next] RDMA/irdma: Check MR contiguity before PBLE alloc
@ 2026-10-10 19:48 Jacob Moroni
  2026-10-10 19:57 ` sashiko-bot
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Jacob Moroni @ 2026-10-10 19:48 UTC (permalink / raw)
  To: tatyana.e.nikolova, jgg, leon; +Cc: linux-rdma, Jacob Moroni

The original logic would attempt to allocate PBLEs, then would
check whether the region was contiguous _after_, and if so,
would free the PBLEs and use a direct mapping instead.

These unnecessary temporary PBLE allocations can technically
fail (PBLEs are a finite resource), which can cause an MR
registration failure even if the region is contiguous and
should have succeeded.

Fix this by checking whether the region is contiguous before
allocating any PBLEs, and use the ib_umem_is_contiguous
helper while we're at it.

Ring MRs still have their own logic. That will be addressed next.

Signed-off-by: Jacob Moroni <jmoroni@google.com>
---
 drivers/infiniband/hw/irdma/verbs.c | 50 +++--------------------------
 drivers/infiniband/hw/irdma/verbs.h |  1 +
 2 files changed, 5 insertions(+), 46 deletions(-)

diff --git a/drivers/infiniband/hw/irdma/verbs.c b/drivers/infiniband/hw/irdma/verbs.c
index 71d7ae7baa47..7ea256e640fa 100644
--- a/drivers/infiniband/hw/irdma/verbs.c
+++ b/drivers/infiniband/hw/irdma/verbs.c
@@ -2917,42 +2917,6 @@ static bool irdma_check_mem_contiguous(u64 *arr, u32 npages, u32 pg_size)
 	return true;
 }
 
-/**
- * irdma_check_mr_contiguous - check if MR is physically contiguous
- * @palloc: pbl allocation struct
- * @pg_size: page size
- */
-static bool irdma_check_mr_contiguous(struct irdma_pble_alloc *palloc,
-				      u32 pg_size)
-{
-	struct irdma_pble_level2 *lvl2 = &palloc->level2;
-	struct irdma_pble_info *leaf = lvl2->leaf;
-	u64 *arr = NULL;
-	u64 *start_addr = NULL;
-	int i;
-	bool ret;
-
-	if (palloc->level == PBLE_LEVEL_1) {
-		arr = palloc->level1.addr;
-		ret = irdma_check_mem_contiguous(arr, palloc->total_cnt,
-						 pg_size);
-		return ret;
-	}
-
-	start_addr = leaf->addr;
-
-	for (i = 0; i < lvl2->leaf_cnt; i++, leaf++) {
-		arr = leaf->addr;
-		if ((*start_addr + ((u64)i * pg_size * PBLE_PER_PAGE)) != *arr)
-			return false;
-		ret = irdma_check_mem_contiguous(arr, leaf->cnt, pg_size);
-		if (!ret)
-			return false;
-	}
-
-	return true;
-}
-
 /**
  * irdma_setup_pbles - copy user pg address to pble's
  * @rf: RDMA PCI function
@@ -3417,22 +3381,13 @@ static int irdma_reg_user_mr_type_mem(struct irdma_mr *iwmr, int access,
 	u8 lvl;
 	int err;
 
-	lvl = iwmr->page_cnt != 1 ? PBLE_LEVEL_1 | PBLE_LEVEL_2 : PBLE_LEVEL_0;
+	lvl = iwmr->is_contig ? PBLE_LEVEL_0 : PBLE_LEVEL_1 | PBLE_LEVEL_2;
 	iwmr->access = access;
 
 	err = irdma_setup_pbles(iwdev->rf, iwmr, lvl);
 	if (err)
 		return err;
 
-	if (lvl) {
-		err = irdma_check_mr_contiguous(&iwpbl->pble_alloc,
-						iwmr->page_size);
-		if (err) {
-			irdma_free_pble(iwdev->rf->pble_rsrc, &iwpbl->pble_alloc);
-			iwpbl->pbl_allocated = false;
-		}
-	}
-
 	if (create_stag) {
 		stag = irdma_create_stag(iwdev);
 		if (!stag) {
@@ -3487,6 +3442,8 @@ static struct irdma_mr *irdma_alloc_iwmr(struct ib_umem *region,
 	pgsz_bitmap = (reg_type == IRDMA_MEMREG_TYPE_MEM) ?
 		iwdev->rf->sc_dev.hw_attrs.page_size_cap : SZ_4K;
 
+	iwmr->is_contig = ib_umem_is_contiguous(region);
+	/* Call best_pgsz after is_contiguous to restore correct umem->iova. */
 	iwmr->page_size = ib_umem_find_best_pgsz(region, pgsz_bitmap, virt);
 	if (unlikely(!iwmr->page_size)) {
 		kfree(iwmr);
@@ -3846,6 +3803,7 @@ static int irdma_rereg_mr_trans(struct irdma_mr *iwmr, u64 start, u64 len,
 	iwmr->region = region;
 	iwmr->ibmr.iova = virt;
 	iwmr->ibmr.pd = pd;
+	iwmr->is_contig = ib_umem_is_contiguous(region);
 	iwmr->page_size = ib_umem_find_best_pgsz(region,
 				iwdev->rf->sc_dev.hw_attrs.page_size_cap,
 				virt);
diff --git a/drivers/infiniband/hw/irdma/verbs.h b/drivers/infiniband/hw/irdma/verbs.h
index a1651641eb71..e4dffe3f191d 100644
--- a/drivers/infiniband/hw/irdma/verbs.h
+++ b/drivers/infiniband/hw/irdma/verbs.h
@@ -112,6 +112,7 @@ struct irdma_mr {
 	int access;
 	bool is_hwreg:1;
 	bool dma_mr:1;
+	bool is_contig:1;
 	u16 type;
 	u32 page_cnt;
 	u64 page_size;
-- 
2.56.0.385.gd3acb90ef8-goog


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH rdma-next] RDMA/irdma: Check MR contiguity before PBLE alloc
  2026-10-10 19:48 [PATCH rdma-next] RDMA/irdma: Check MR contiguity before PBLE alloc Jacob Moroni
@ 2026-10-10 19:57 ` sashiko-bot
  2026-10-12 14:00 ` Leon Romanovsky
  2026-10-12 18:50 ` Jason Gunthorpe
  2 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-10-10 19:57 UTC (permalink / raw)
  To: Jacob Moroni; +Cc: linux-rdma

> The original logic would attempt to allocate PBLEs, then would
> check whether the region was contiguous _after_, and if so,
> would free the PBLEs and use a direct mapping instead.
> 
> These unnecessary temporary PBLE allocations can technically
> fail (PBLEs are a finite resource), which can cause an MR
> registration failure even if the region is contiguous and
> should have succeeded.
> 
> Fix this by checking whether the region is contiguous before
> allocating any PBLEs, and use the ib_umem_is_contiguous
> helper while we're at it.
> 
> Ring MRs still have their own logic. That will be addressed next.
> 
> Signed-off-by: Jacob Moroni <jmoroni@google.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261010194832.2415477-1-jmoroni@google.com?part=1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH rdma-next] RDMA/irdma: Check MR contiguity before PBLE alloc
  2026-10-10 19:48 [PATCH rdma-next] RDMA/irdma: Check MR contiguity before PBLE alloc Jacob Moroni
  2026-10-10 19:57 ` sashiko-bot
@ 2026-10-12 14:00 ` Leon Romanovsky
  2026-10-12 18:50 ` Jason Gunthorpe
  2 siblings, 0 replies; 5+ messages in thread
From: Leon Romanovsky @ 2026-10-12 14:00 UTC (permalink / raw)
  To: tatyana.e.nikolova, jgg, Jacob Moroni; +Cc: linux-rdma


On Sat, 10 Oct 2026 19:48:32 +0000, Jacob Moroni wrote:
> The original logic would attempt to allocate PBLEs, then would
> check whether the region was contiguous _after_, and if so,
> would free the PBLEs and use a direct mapping instead.
> 
> These unnecessary temporary PBLE allocations can technically
> fail (PBLEs are a finite resource), which can cause an MR
> registration failure even if the region is contiguous and
> should have succeeded.
> 
> [...]

Applied, thanks!

[1/1] RDMA/irdma: Check MR contiguity before PBLE alloc
      https://git.kernel.org/rdma/rdma/c/023b4b6ea490b3

Best regards,
-- 
Leon Romanovsky <leon@kernel.org>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH rdma-next] RDMA/irdma: Check MR contiguity before PBLE alloc
  2026-10-10 19:48 [PATCH rdma-next] RDMA/irdma: Check MR contiguity before PBLE alloc Jacob Moroni
  2026-10-10 19:57 ` sashiko-bot
  2026-10-12 14:00 ` Leon Romanovsky
@ 2026-10-12 18:50 ` Jason Gunthorpe
  2026-10-13 11:22   ` Leon Romanovsky
  2 siblings, 1 reply; 5+ messages in thread
From: Jason Gunthorpe @ 2026-10-12 18:50 UTC (permalink / raw)
  To: Jacob Moroni; +Cc: tatyana.e.nikolova, leon, linux-rdma

On Sat, Oct 10, 2026 at 07:48:32PM +0000, Jacob Moroni wrote:
> @@ -3487,6 +3442,8 @@ static struct irdma_mr *irdma_alloc_iwmr(struct ib_umem *region,
>  	pgsz_bitmap = (reg_type == IRDMA_MEMREG_TYPE_MEM) ?
>  		iwdev->rf->sc_dev.hw_attrs.page_size_cap : SZ_4K;
>  
> +	iwmr->is_contig = ib_umem_is_contiguous(region);
> +	/* Call best_pgsz after is_contiguous to restore correct umem->iova. */
>  	iwmr->page_size = ib_umem_find_best_pgsz(region, pgsz_bitmap, virt);
>  	if (unlikely(!iwmr->page_size)) {
>  		kfree(iwmr);
> @@ -3846,6 +3803,7 @@ static int irdma_rereg_mr_trans(struct irdma_mr *iwmr, u64 start, u64 len,
>  	iwmr->region = region;
>  	iwmr->ibmr.iova = virt;
>  	iwmr->ibmr.pd = pd;
> +	iwmr->is_contig = ib_umem_is_contiguous(region);
>  	iwmr->page_size = ib_umem_find_best_pgsz(region,
>  				iwdev->rf->sc_dev.hw_attrs.page_size_cap,
>  				virt);

You should not call ib_umem_find_best_pgsz() after
ib_umem_is_contiguous(), it is nonsense to compute a page_size once
the contiguous optimization is being used.

Once you decide to use contiguous the start dma address should go
straight into the MR structure and just avoid all the pbl stuff.

Jason

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH rdma-next] RDMA/irdma: Check MR contiguity before PBLE alloc
  2026-10-12 18:50 ` Jason Gunthorpe
@ 2026-10-13 11:22   ` Leon Romanovsky
  0 siblings, 0 replies; 5+ messages in thread
From: Leon Romanovsky @ 2026-10-13 11:22 UTC (permalink / raw)
  To: Jason Gunthorpe; +Cc: Jacob Moroni, tatyana.e.nikolova, linux-rdma

On Mon, Oct 12, 2026 at 03:50:10PM -0300, Jason Gunthorpe wrote:
> On Sat, Oct 10, 2026 at 07:48:32PM +0000, Jacob Moroni wrote:
> > @@ -3487,6 +3442,8 @@ static struct irdma_mr *irdma_alloc_iwmr(struct ib_umem *region,
> >  	pgsz_bitmap = (reg_type == IRDMA_MEMREG_TYPE_MEM) ?
> >  		iwdev->rf->sc_dev.hw_attrs.page_size_cap : SZ_4K;
> >  
> > +	iwmr->is_contig = ib_umem_is_contiguous(region);
> > +	/* Call best_pgsz after is_contiguous to restore correct umem->iova. */
> >  	iwmr->page_size = ib_umem_find_best_pgsz(region, pgsz_bitmap, virt);
> >  	if (unlikely(!iwmr->page_size)) {
> >  		kfree(iwmr);
> > @@ -3846,6 +3803,7 @@ static int irdma_rereg_mr_trans(struct irdma_mr *iwmr, u64 start, u64 len,
> >  	iwmr->region = region;
> >  	iwmr->ibmr.iova = virt;
> >  	iwmr->ibmr.pd = pd;
> > +	iwmr->is_contig = ib_umem_is_contiguous(region);
> >  	iwmr->page_size = ib_umem_find_best_pgsz(region,
> >  				iwdev->rf->sc_dev.hw_attrs.page_size_cap,
> >  				virt);
> 
> You should not call ib_umem_find_best_pgsz() after
> ib_umem_is_contiguous(), it is nonsense to compute a page_size once
> the contiguous optimization is being used.

ok, I dropped the patch from mu wip/leon-for-next branch for now.

Thanks

> 
> Once you decide to use contiguous the start dma address should go
> straight into the MR structure and just avoid all the pbl stuff.
> 
> Jason

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-10-13 11:22 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-10 19:48 [PATCH rdma-next] RDMA/irdma: Check MR contiguity before PBLE alloc Jacob Moroni
2026-10-10 19:57 ` sashiko-bot
2026-10-12 14:00 ` Leon Romanovsky
2026-10-12 18:50 ` Jason Gunthorpe
2026-10-13 11:22   ` Leon Romanovsky

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.