All of lore.kernel.org
 help / color / mirror / Atom feed
From: NeilBrown <neilb@suse.de>
To: "J. Bruce Fields" <bfields@fieldses.org>,
	Trond Myklebust <trond.myklebust@hammerspace.com>,
	Anna Schumaker <anna.schumaker@netapp.com>
Cc: Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: [PATCH] SUNRPC: backchannel RPC request must reference XPRT
Date: Tue, 15 Oct 2019 10:36:27 +1100	[thread overview]
Message-ID: <87imoqrjb8.fsf@notabene.neil.brown.name> (raw)
In-Reply-To: <20191011165603.GD19318@fieldses.org>

[-- Attachment #1: Type: text/plain, Size: 2089 bytes --]


The backchannel RPC requests - that are queued waiting
for the reply to be sent by the "NFSv4 callback" thread -
have a pointer to the xprt, but it is not reference counted.
It is possible for the xprt to be freed while there are
still queued requests.

I think this has been a problem since
Commit fb7a0b9addbd ("nfs41: New backchannel helper routines")
when the code was introduced, but I suspect it became more of
a problem after
Commit 80b14d5e61ca ("SUNRPC: Add a structure to track multiple transports")
(or there abouts).
Before this second patch, the nfs client would hold a reference to
the xprt to keep it alive.  After multipath was introduced,
a client holds a reference to a swtich, and the switch can have multiple
xprts which can be added and removed.

I'm not sure of all the causal issues, but this patch has
fixed a customer problem were an NFSv4.1 client would run out
of memory with tens of thousands of backchannel rpc requests
queued for an xprt that had been freed.  This was a 64K-page
machine so each rpc_rqst consumed more than 128K of memory.

Fixes: 80b14d5e61ca ("SUNRPC: Add a structure to track multiple transports")
cc: stable@vger.kernel.org (v4.6)
Signed-off-by: NeilBrown <neilb@suse.de>
---
 net/sunrpc/backchannel_rqst.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/sunrpc/backchannel_rqst.c b/net/sunrpc/backchannel_rqst.c
index 339e8c077c2d..c95ca39688b6 100644
--- a/net/sunrpc/backchannel_rqst.c
+++ b/net/sunrpc/backchannel_rqst.c
@@ -61,6 +61,7 @@ static void xprt_free_allocation(struct rpc_rqst *req)
 	free_page((unsigned long)xbufp->head[0].iov_base);
 	xbufp = &req->rq_snd_buf;
 	free_page((unsigned long)xbufp->head[0].iov_base);
+	xprt_put(req->rq_xprt);
 	kfree(req);
 }
 
@@ -85,7 +86,7 @@ struct rpc_rqst *xprt_alloc_bc_req(struct rpc_xprt *xprt, gfp_t gfp_flags)
 	if (req == NULL)
 		return NULL;
 
-	req->rq_xprt = xprt;
+	req->rq_xprt = xprt_get(xprt);
 	INIT_LIST_HEAD(&req->rq_bc_list);
 
 	/* Preallocate one XDR receive buffer */
-- 
2.23.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

  reply	other threads:[~2019-10-14 23:36 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-09  5:15 NFSv4.1 backchannel xprt problems NeilBrown
2019-10-11 16:56 ` J. Bruce Fields
2019-10-14 23:36   ` NeilBrown [this message]
2019-10-15 21:16     ` [PATCH] SUNRPC: backchannel RPC request must reference XPRT Trond Myklebust
2019-10-15 21:47       ` Chuck Lever
2019-10-15 23:23       ` NeilBrown
2019-10-16  3:04         ` Trond Myklebust
2019-10-16  4:38           ` NeilBrown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87imoqrjb8.fsf@notabene.neil.brown.name \
    --to=neilb@suse.de \
    --cc=anna.schumaker@netapp.com \
    --cc=bfields@fieldses.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=trond.myklebust@hammerspace.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.