Re: [PATCH v4 08/21] xprtrdma: Back off rkey when FAST_REG_MR fails

[Anna Schumaker <Anna.Schumaker@netapp.com>]

On 07/22/2014 05:18 PM, Chuck Lever wrote:
> 
> On Jul 22, 2014, at 4:03 PM, Anna Schumaker <Anna.Schumaker@netapp.com> wrote:
> 
>> On 07/22/2014 10:52 AM, Chuck Lever wrote:
>>> If posting a FAST_REG_MR Work Reqeust fails, revert the rkey update
>>> to avoid subsequent IB_WC_MW_BIND_ERR completions.
>>>
>>> Suggested-by: Steve Wise <swise@opengridcomputing.com>
>>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>>> Tested-by: Shirley Ma <shirley.ma@oracle.com>
>>> Tested-by: Devesh Sharma <devesh.sharma@emulex.com>
>>> ---
>>> include/rdma/ib_verbs.h     |   11 +++++++++++
>>> net/sunrpc/xprtrdma/verbs.c |   12 +++++-------
>>> 2 files changed, 16 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
>>> index 7ccef34..c5c7ec6 100644
>>> --- a/include/rdma/ib_verbs.h
>>> +++ b/include/rdma/ib_verbs.h
>>> @@ -2479,6 +2479,17 @@ static inline u32 ib_inc_rkey(u32 rkey)
>>> }
>>>
>>> /**
>>> + * ib_dec_rkey - decrements the key portion of the given rkey. Can be used
>>> + * when recovering from a local immediate error.
>>> + * @rkey - the rkey to decrement.
>>> + */
>>> +static inline u32 ib_dec_rkey(u32 rkey)
>>> +{
>>> +	const u32 mask = 0x000000ff;
>>> +	return ((rkey - 1) & mask) | (rkey & ~mask);
>>
>> ib_inc_rkey() uses the same mask, and ib_update_fast_reg_key() uses !mask.  I think this should be a #define constant, rather than hardcoded in four places.
> 
> Such a clean up is not related to the change in this patch.
> 
> However, it could go as a separate patch, sent from someone
> who can test the other current users of ib_inc_rkey() and
> ib_update_fast_reg_key().

I agree with separate patch sometime in the future.  Is it appropriate to add ib_dec_rkey() through the NFS tree, or is there an Infiniband tree that this should go through instead?

Anna

> 
> 
>> Anna
>>
>>
>>> +}
>>> +
>>> +/**
>>>  * ib_alloc_mw - Allocates a memory window.
>>>  * @pd: The protection domain associated with the memory window.
>>>  * @type: The type of the memory window (1 or 2).
>>> diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c
>>> index 8bb7945..394b13f 100644
>>> --- a/net/sunrpc/xprtrdma/verbs.c
>>> +++ b/net/sunrpc/xprtrdma/verbs.c
>>> @@ -1497,13 +1497,12 @@ rpcrdma_register_frmr_external(struct rpcrdma_mr_seg *seg,
>>> 	struct rpcrdma_frmr *frmr = &mw->r.frmr;
>>> 	struct ib_mr *mr = frmr->fr_mr;
>>> 	struct ib_send_wr invalidate_wr, frmr_wr, *bad_wr, *post_wr;
>>> -
>>> -	u8 key;
>>> 	int len, pageoff;
>>> 	int i, rc;
>>> 	int seg_len;
>>> 	u64 pa;
>>> 	int page_no;
>>> +	u32 rkey;
>>>
>>> 	pageoff = offset_in_page(seg1->mr_offset);
>>> 	seg1->mr_offset -= pageoff;	/* start of page */
>>> @@ -1558,14 +1557,11 @@ rpcrdma_register_frmr_external(struct rpcrdma_mr_seg *seg,
>>> 		rc = -EIO;
>>> 		goto out_err;
>>> 	}
>>> -
>>> -	/* Bump the key */
>>> -	key = (u8)(mr->rkey & 0x000000FF);
>>> -	ib_update_fast_reg_key(mr, ++key);
>>> -
>>> 	frmr_wr.wr.fast_reg.access_flags = (writing ?
>>> 				IB_ACCESS_REMOTE_WRITE | IB_ACCESS_LOCAL_WRITE :
>>> 				IB_ACCESS_REMOTE_READ);
>>> +	rkey = ib_inc_rkey(mr->rkey);
>>> +	ib_update_fast_reg_key(mr, rkey);
>>> 	frmr_wr.wr.fast_reg.rkey = mr->rkey;
>>> 	DECR_CQCOUNT(&r_xprt->rx_ep);
>>>
>>> @@ -1574,6 +1570,8 @@ rpcrdma_register_frmr_external(struct rpcrdma_mr_seg *seg,
>>> 	if (rc) {
>>> 		dprintk("RPC:       %s: failed ib_post_send for register,"
>>> 			" status %i\n", __func__, rc);
>>> +		rkey = ib_dec_rkey(mr->rkey);
>>> +		ib_update_fast_reg_key(mr, rkey);
>>> 		goto out_err;
>>> 	} else {
>>> 		seg1->mr_rkey = mr->rkey;
>>>
>>
> 
> --
> Chuck Lever
> chuck[dot]lever[at]oracle[dot]com
> 
> 
>