| Message ID | 20140722145208.6010.76678.stgit@manet.1015granger.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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. 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; > -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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(). > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 > > > -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Roland, I'm collecting changes for NFS over RDMA, and I wasn't sure if the change to include/rdma/ib_verbs.h in this patch (adding ib_dec_rkey()) was something that could be merged through the NFS tree. What do you think? Thanks! Anna 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_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; > -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hey Roland, Sean and Hal. We would like to submit this patch through the NFS tree, since the function added to include/rdma/ib_verbs.h is only used for NFS over RDMA. Do any of you have a problem with this patch, and can I have an acked-by if this is okay? Thanks! Anna 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_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; > -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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_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;