> On Sep 5, 2017, at 4:33 PM, Trond Myklebust <trondmy@xxxxxxxxxxxxxxx> wrote: > > On Thu, 2017-08-24 at 14:18 -0400, Anna Schumaker wrote: >> >> On 08/23/2017 05:40 PM, Trond Myklebust wrote: >>> On Wed, 2017-08-23 at 17:05 -0400, Chuck Lever wrote: >>>> Adopt the use of xprt_pin_rqst to eliminate contention between >>>> Call-side users of rb_lock and the use of rb_lock in >>>> rpcrdma_reply_handler. >>>> >>>> This replaces the mechanism introduced in 431af645cf66 >>>> ("xprtrdma: >>>> Fix client lock-up after application signal fires"). >>>> >>>> Use recv_lock to quickly find the completing rqst, pin it, then >>>> drop the lock. At that point invalidation and pull-up of the >>>> Reply >>>> XDR can be done. Both are often expensive operations. >>>> >>>> Finally, take recv_lock again to signal completion to the RPC >>>> layer. It also protects adjustment of "cwnd". >>>> >>>> This greatly reduces the amount of time a lock is held by the >>>> reply handler. Comparing lock_stat results shows a marked >>>> decrease >>>> in contention on rb_lock and recv_lock. >>>> >>>> Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx> >>>> --- >>>> Hi- >>>> >>>> If Trond's lock contention series is going into v4.14, I'd like >>>> you >>>> to consider this one (applied at the end of that series) as well. >>>> Without it, NFS/RDMA performance regresses a bit after the >>>> first xprt_pin_rqst patch is applied. Thanks! >>>> >>> >>> If Anna is OK with it, then I can apply this patch once she's sent >>> me >>> the pull request for the other RDMA client patches. >> >> Works for me. Thanks! > > Please can you both check http://git.linux-nfs.org/?p=trondmy/linux-nfs > .git;a=commit;h=b1da5d90857ee4b8f5acee1143705412b16fce56 and see if all > is well? > > Do note that I did remove the call to rpcrdma_buffer_put() which was > being called with an uninitialised argument. Looking at that again. rpcrdma_buffer_put() does look unnecessary, since out_norqst is being called now before "req" is initialized. On further inspection, "return;" should be replaced with "goto repost;". This is similar to "out_nomatch" and "out_duplicate", which are both being replaced in this patch. So, out_norqst should look something like this: out_norqst: spin_unlock(&xprt->recv_lock); dprintk("RPC: %s: no match for incoming xid 0x%08x\n", __func__, be32_to_cpu(xid)); goto repost; Otherwise, I don't see any obvious problems. -- Chuck Lever -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html