Linux-NFS Archive on
 help / Atom feed
* [RFC for-stable PATCH] SUNRPC: Always drop the XPRT_LOCK on XPRT_CLOSE_WAIT
@ 2019-01-09 21:53 Benjamin Coddington
  0 siblings, 0 replies; 1+ messages in thread
From: Benjamin Coddington @ 2019-01-09 21:53 UTC (permalink / raw)
  To: linux-nfs; +Cc: Trond Myklebust, Dave Wysochanski, Scott Mayhew

Hello, I'd like this patch to land in stable but I'd like to give everyone
here a chance to NAK or add a SOB before I send it, since it doesn't have an
exact mainline copy.  Thanks for any review, and really thanks to Dave and
Trond for doing all the actual work.


This patch is only appropriate for stable kernels v4.16 - v4.19

Since commit 9b30889c548a ("SUNRPC: Ensure we always close the socket after
a connection shuts down"), and until commit c544577daddb ("SUNRPC: Clean up
transport write space handling"), it is possible for the NFS client to spin
in the following tight loop:

269.964083: rpc_task_run_action: task:43@0 flags=5a81 state=0005 status=0 action=call_bind [sunrpc]
269.964083: rpc_task_run_action: task:43@0 flags=5a81 state=0005 status=0 action=call_connect [sunrpc]
269.964083: rpc_task_run_action: task:43@0 flags=5a81 state=0005 status=0 action=call_transmit [sunrpc]
269.964085: xprt_transmit: peer=[]:2049 xid=0x761d3f77 status=-32
269.964085: rpc_task_run_action: task:43@0 flags=5a81 state=0005 status=-32 action=call_transmit_status [sunrpc]
269.964085: rpc_task_run_action: task:43@0 flags=5a81 state=0005 status=-32 action=call_status [sunrpc]
269.964085: rpc_call_status: task:43@0 status=-32

The issue is that the path through call_transmit_status does not release
the XPRT_LOCK when the transmit result is -EPIPE, so the socket cannot be
properly shut down.

The below commit fixed things up in mainline by unconditionally calling
xprt_end_transmit() which releases the XPRT_LOCK after every pass through
call_transmit.  However, the entirety of this commit is not appropriate for
stable kernels because its original inclusion was part of a series that
modifies the sunrpc code to use a different queueing model.  As a result,
there are machinations within this patch that are not needed for a stable
fix and will not make sense without a larger backport of the mainline

In this patch, we take the slightly modified bit of the mainline patch
below, which is to release the XPRT_LOCK on transmission error should we
detect that the transport is waiting to close.

commit c544577daddb618c7dd5fa7fb98d6a41782f020e
Author: Trond Myklebust <>
Date:   Mon Sep 3 23:39:27 2018 -0400

    SUNRPC: Clean up transport write space handling

    Treat socket write space handling in the same way we now treat transport
    congestion: by denying the XPRT_LOCK until the transport signals that it
    has free buffer space.

    Signed-off-by: Trond Myklebust <>

The original discussion of the problem is here:

This passes my usual cthon and xfstests on NFS as applied on v4.19 mainline.

Reported-by: Dave Wysochanski <>
Suggested-by: Trond Myklebust <>
Signed-off-by: Benjamin Coddington <>
 include/linux/sunrpc/xprt.h | 5 +++++
 net/sunrpc/clnt.c           | 6 ++++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/include/linux/sunrpc/xprt.h b/include/linux/sunrpc/xprt.h
index 336fd1a19cca..f30bf500888d 100644
--- a/include/linux/sunrpc/xprt.h
+++ b/include/linux/sunrpc/xprt.h
@@ -443,6 +443,11 @@ static inline int xprt_test_and_set_connecting(struct rpc_xprt *xprt)
 	return test_and_set_bit(XPRT_CONNECTING, &xprt->state);
+static inline int xprt_close_wait(struct rpc_xprt *xprt)
+	return test_bit(XPRT_CLOSE_WAIT, &xprt->state);
 static inline void xprt_set_bound(struct rpc_xprt *xprt)
 	test_and_set_bit(XPRT_BOUND, &xprt->state);
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index 8ea2f5fadd96..1fc812ba9871 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -1992,13 +1992,15 @@ call_transmit(struct rpc_task *task)
 static void
 call_transmit_status(struct rpc_task *task)
+	struct rpc_xprt *xprt = task->tk_rqstp->rq_xprt;
 	task->tk_action = call_status;
 	 * Common case: success.  Force the compiler to put this
-	 * test first.
+	 * test first.  Or, if any error and xprt_close_wait,
+	 * release the xprt lock so the socket can close.
-	if (task->tk_status == 0) {
+	if (task->tk_status == 0 || xprt_close_wait(xprt)) {

^ permalink raw reply	[flat|nested] 1+ messages in thread

only message in thread, back to index

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-09 21:53 [RFC for-stable PATCH] SUNRPC: Always drop the XPRT_LOCK on XPRT_CLOSE_WAIT Benjamin Coddington

Linux-NFS Archive on

Archives are clonable:
	git clone --mirror linux-nfs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-nfs linux-nfs/ \
	public-inbox-index linux-nfs

Newsgroup available over NNTP:

AGPL code for this site: git clone public-inbox