[PATCH 4/4] rdma_rxe: remove duplicate entries in struct rxe_mr

[PATCH 4/4] rdma_rxe: remove duplicate entries in struct rxe_mr

[Bob Pearson]

  - Struct rxe_mem had pd, lkey and rkey values both in itself
    and in the struct ib_mr which is also included in rxe_mem.
  - Delete these entries and replace references with ones in ibmr.
  - Add mr_lkey and mr_rkey macros which extract these values from mr.
  - Added mr_pd macro which extracts pd from mr.

Signed-off-by: Bob Pearson <rpearson@hpe.com>
---
 drivers/infiniband/sw/rxe/rxe_mr.c    | 25 ++++++++++---------------
 drivers/infiniband/sw/rxe/rxe_req.c   |  4 ++--
 drivers/infiniband/sw/rxe/rxe_verbs.c |  2 +-
 drivers/infiniband/sw/rxe/rxe_verbs.h |  8 ++++----
 4 files changed, 17 insertions(+), 22 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_mr.c b/drivers/infiniband/sw/rxe/rxe_mr.c
index 390d8e6629ad..6e8c41567ba0 100644
--- a/drivers/infiniband/sw/rxe/rxe_mr.c
+++ b/drivers/infiniband/sw/rxe/rxe_mr.c
@@ -51,13 +51,8 @@ static void rxe_mem_init(int access, struct rxe_mem *mem)
 	u32 lkey = mem->pelem.index << 8 | rxe_get_key();
 	u32 rkey = (access & IB_ACCESS_REMOTE) ? lkey : 0;
 
-	if (mem->pelem.pool->type == RXE_TYPE_MR) {
-		mem->ibmr.lkey		= lkey;
-		mem->ibmr.rkey		= rkey;
-	}
-
-	mem->lkey		= lkey;
-	mem->rkey		= rkey;
+	mem->ibmr.lkey		= lkey;
+	mem->ibmr.rkey		= rkey;
 	mem->state		= RXE_MEM_STATE_INVALID;
 	mem->type		= RXE_MEM_TYPE_NONE;
 	mem->map_shift		= ilog2(RXE_BUF_PER_MAP);
@@ -121,7 +116,7 @@ void rxe_mem_init_dma(struct rxe_pd *pd,
 {
 	rxe_mem_init(access, mem);
 
-	mem->pd			= pd;
+	mem->ibmr.pd		= &pd->ibpd;
 	mem->access		= access;
 	mem->state		= RXE_MEM_STATE_VALID;
 	mem->type		= RXE_MEM_TYPE_DMA;
@@ -190,7 +185,7 @@ int rxe_mem_init_user(struct rxe_pd *pd, u64 start,
 		}
 	}
 
-	mem->pd			= pd;
+	mem->ibmr.pd		= &pd->ibpd;
 	mem->umem		= umem;
 	mem->access		= access;
 	mem->length		= length;
@@ -220,7 +215,7 @@ int rxe_mem_init_fast(struct rxe_pd *pd,
 	if (err)
 		goto err1;
 
-	mem->pd			= pd;
+	mem->ibmr.pd		= &pd->ibpd;
 	mem->max_buf		= max_pages;
 	mem->state		= RXE_MEM_STATE_FREE;
 	mem->type		= RXE_MEM_TYPE_MR;
@@ -340,7 +335,7 @@ int rxe_mem_copy(struct rxe_mem *mem, u64 iova, void *addr, int length,
 		memcpy(dest, src, length);
 
 		if (crcp)
-			*crcp = rxe_crc32(to_rdev(mem->pd->ibpd.device),
+			*crcp = rxe_crc32(to_rdev(mem->ibmr.device),
 					*crcp, dest, length);
 
 		return 0;
@@ -374,7 +369,7 @@ int rxe_mem_copy(struct rxe_mem *mem, u64 iova, void *addr, int length,
 		memcpy(dest, src, bytes);
 
 		if (crcp)
-			crc = rxe_crc32(to_rdev(mem->pd->ibpd.device),
+			crc = rxe_crc32(to_rdev(mem->ibmr.device),
 					crc, dest, bytes);
 
 		length	-= bytes;
@@ -547,9 +542,9 @@ struct rxe_mem *lookup_mem(struct rxe_pd *pd, int access, u32 key,
 	if (!mem)
 		return NULL;
 
-	if (unlikely((type == lookup_local && mem->lkey != key) ||
-		     (type == lookup_remote && mem->rkey != key) ||
-		     mem->pd != pd ||
+	if (unlikely((type == lookup_local && mr_lkey(mem) != key) ||
+		     (type == lookup_remote && mr_rkey(mem) != key) ||
+		     mr_pd(mem) != pd ||
 		     (access && !(access & mem->access)) ||
 		     mem->state != RXE_MEM_STATE_VALID)) {
 		rxe_drop_ref(mem);
diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c
index e27585ce9eb7..af3923bf0a36 100644
--- a/drivers/infiniband/sw/rxe/rxe_req.c
+++ b/drivers/infiniband/sw/rxe/rxe_req.c
@@ -617,8 +617,8 @@ int rxe_requester(void *arg)
 
 			rmr->state = RXE_MEM_STATE_VALID;
 			rmr->access = wqe->wr.wr.reg.access;
-			rmr->lkey = wqe->wr.wr.reg.key;
-			rmr->rkey = wqe->wr.wr.reg.key;
+			rmr->ibmr.lkey = wqe->wr.wr.reg.key;
+			rmr->ibmr.rkey = wqe->wr.wr.reg.key;
 			rmr->iova = wqe->wr.wr.reg.mr->iova;
 			wqe->state = wqe_state_done;
 			wqe->status = IB_WC_SUCCESS;
diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c
index f368dc16281a..ba8faa34969b 100644
--- a/drivers/infiniband/sw/rxe/rxe_verbs.c
+++ b/drivers/infiniband/sw/rxe/rxe_verbs.c
@@ -921,7 +921,7 @@ static int rxe_dereg_mr(struct ib_mr *ibmr, struct ib_udata *udata)
 	struct rxe_mem *mr = to_rmr(ibmr);
 
 	mr->state = RXE_MEM_STATE_ZOMBIE;
-	rxe_drop_ref(mr->pd);
+	rxe_drop_ref(mr_pd(mr));
 	rxe_drop_index(mr);
 	rxe_drop_ref(mr);
 	return 0;
diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.h b/drivers/infiniband/sw/rxe/rxe_verbs.h
index 658b9b1ebc62..efe5d9f34fc1 100644
--- a/drivers/infiniband/sw/rxe/rxe_verbs.h
+++ b/drivers/infiniband/sw/rxe/rxe_verbs.h
@@ -294,12 +294,8 @@ struct rxe_mem {
 		struct ib_mw		ibmw;
 	};
 
-	struct rxe_pd		*pd;
 	struct ib_umem		*umem;
 
-	u32			lkey;
-	u32			rkey;
-
 	enum rxe_mem_state	state;
 	enum rxe_mem_type	type;
 	u64			va;
@@ -333,6 +329,10 @@ struct rxe_mc_grp {
 	u16			pkey;
 };
 
+#define mr_pd(mr) to_rpd((mr)->ibmr.pd)
+#define mr_lkey(mr) ((mr)->ibmr.lkey)
+#define mr_rkey(mr) ((mr)->ibmr.rkey)
+
 struct rxe_mc_elem {
 	struct rxe_pool_entry	pelem;
 	struct list_head	qp_list;
-- 
2.25.1

Re: [PATCH 4/4] rdma_rxe: remove duplicate entries in struct rxe_mr

[Jason Gunthorpe]

On Thu, Oct 08, 2020 at 04:28:18PM -0500, Bob Pearson wrote:

Subject should be of the form

RDMA/rxe: Some subject

RDMA convention is to capitalize the first letter ie 'Some'

> - Struct rxe_mem had pd, lkey and rkey values both in itself
>     and in the struct ib_mr which is also included in rxe_mem.
>   - Delete these entries and replace references with ones in ibmr.
>   - Add mr_lkey and mr_rkey macros which extract these values from mr.
>   - Added mr_pd macro which extracts pd from mr.

Commit body text should be paragraphs not point form

> @@ -333,6 +329,10 @@ struct rxe_mc_grp {
>  	u16			pkey;
>  };
>  
> +#define mr_pd(mr) to_rpd((mr)->ibmr.pd)
> +#define mr_lkey(mr) ((mr)->ibmr.lkey)
> +#define mr_rkey(mr) ((mr)->ibmr.rkey)

Try to avoid macros for implementing functions, I changed this to:

+static inline struct rxe_pd *mr_pd(struct rxe_mem *mr)
+{
+       return to_rpd(mr->ibmr.pd);
+}
+
+static inline u32 mr_lkey(struct rxe_mem *mr)
+{
+       return mr->ibmr.lkey;
+}
+
+static inline u32 mr_rkey(struct rxe_mem *mr)
+{
+       return mr->ibmr.rkey;
+}
+

and fixed the other stuff, applied to for-next

Thanks,
Jason

Re: [PATCH 4/4] rdma_rxe: remove duplicate entries in struct rxe_mr

[Bob Pearson]

On 10/8/20 6:16 PM, Jason Gunthorpe wrote:
> On Thu, Oct 08, 2020 at 04:28:18PM -0500, Bob Pearson wrote:
> 
> Subject should be of the form
> 
> RDMA/rxe: Some subject
> 
> RDMA convention is to capitalize the first letter ie 'Some'
> 
>> - Struct rxe_mem had pd, lkey and rkey values both in itself
>>     and in the struct ib_mr which is also included in rxe_mem.
>>   - Delete these entries and replace references with ones in ibmr.
>>   - Add mr_lkey and mr_rkey macros which extract these values from mr.
>>   - Added mr_pd macro which extracts pd from mr.
> 
> Commit body text should be paragraphs not point form
> 
>> @@ -333,6 +329,10 @@ struct rxe_mc_grp {
>>  	u16			pkey;
>>  };
>>  
>> +#define mr_pd(mr) to_rpd((mr)->ibmr.pd)
>> +#define mr_lkey(mr) ((mr)->ibmr.lkey)
>> +#define mr_rkey(mr) ((mr)->ibmr.rkey)
> 
> Try to avoid macros for implementing functions, I changed this to:
> 
> +static inline struct rxe_pd *mr_pd(struct rxe_mem *mr)
> +{
> +       return to_rpd(mr->ibmr.pd);
> +}
> +
> +static inline u32 mr_lkey(struct rxe_mem *mr)
> +{
> +       return mr->ibmr.lkey;
> +}
> +
> +static inline u32 mr_rkey(struct rxe_mem *mr)
> +{
> +       return mr->ibmr.rkey;
> +}
> +
> 
> and fixed the other stuff, applied to for-next
> 
> Thanks,
> Jason
> 
Thanks for the style hints and applying the patch. Just guessing but I assume that in RDMA/somthing RDMA refers to the entire drivers/infiniband tree. The equivalent for user space is RDMA-CORE or rdma-core ??

Bob

Re: [PATCH 4/4] rdma_rxe: remove duplicate entries in struct rxe_mr

[Jason Gunthorpe]

On Fri, Oct 09, 2020 at 12:02:24PM -0500, Bob Pearson wrote:
> On 10/8/20 6:16 PM, Jason Gunthorpe wrote:
> > On Thu, Oct 08, 2020 at 04:28:18PM -0500, Bob Pearson wrote:
> > 
> > Subject should be of the form
> > 
> > RDMA/rxe: Some subject
> > 
> > RDMA convention is to capitalize the first letter ie 'Some'
> > 
> >> - Struct rxe_mem had pd, lkey and rkey values both in itself
> >>     and in the struct ib_mr which is also included in rxe_mem.
> >>   - Delete these entries and replace references with ones in ibmr.
> >>   - Add mr_lkey and mr_rkey macros which extract these values from mr.
> >>   - Added mr_pd macro which extracts pd from mr.
> > 
> > Commit body text should be paragraphs not point form
> > 
> >> @@ -333,6 +329,10 @@ struct rxe_mc_grp {
> >>  	u16			pkey;
> >>  };
> >>  
> >> +#define mr_pd(mr) to_rpd((mr)->ibmr.pd)
> >> +#define mr_lkey(mr) ((mr)->ibmr.lkey)
> >> +#define mr_rkey(mr) ((mr)->ibmr.rkey)
> > 
> > Try to avoid macros for implementing functions, I changed this to:
> > 
> > +static inline struct rxe_pd *mr_pd(struct rxe_mem *mr)
> > +{
> > +       return to_rpd(mr->ibmr.pd);
> > +}
> > +
> > +static inline u32 mr_lkey(struct rxe_mem *mr)
> > +{
> > +       return mr->ibmr.lkey;
> > +}
> > +
> > +static inline u32 mr_rkey(struct rxe_mem *mr)
> > +{
> > +       return mr->ibmr.rkey;
> > +}
> > +
> > 
> > and fixed the other stuff, applied to for-next
> > 
> > Thanks,
> > Jason
> > 
> Thanks for the style hints and applying the patch. Just guessing but
> I assume that in RDMA/somthing RDMA refers to the entire
> drivers/infiniband tree. The equivalent for user space is RDMA-CORE
> or rdma-core ??

Userspace is usualy something like providers/rxe:

Jason