On Tue, 2017-09-05 at 17:06 -0400, Chuck Lever wrote:
> > On Sep 5, 2017, at 4:33 PM, Trond Myklebust <trondmy@xxxxxxxxxxxxxx
> > m> 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.
>
Patch updated. Please see http://git.linux-nfs.org/?p=trondmy/linux-nfs
.git;a=commit;h=9590d083c1bb1419b7992609d1a0a3e3517d3893
--
Trond Myklebust
Linux NFS client maintainer, PrimaryData
trond.myklebust@xxxxxxxxxxxxxxx
��.n��������+%������w��{.n�����{��w��� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
