Re: [PATCH v1] xprtrdma: Use xprt_pin_rqst in rpcrdma_reply_handler

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> 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



[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux