Linux NFS development
 help / color / mirror / Atom feed
From: Jeff Layton <jlayton@kernel.org>
To: Trond Myklebust <trondmy@kernel.org>,
	Anna Schumaker <anna@kernel.org>,
	 Chuck Lever <chuck.lever@oracle.com>, Neil Brown <neilb@suse.de>,
	 Olga Kornievskaia <okorniev@redhat.com>,
	Dai Ngo <Dai.Ngo@oracle.com>,  Tom Talpey <tom@talpey.com>,
	"David S. Miller" <davem@davemloft.net>,
	 Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>,
	 Paolo Abeni <pabeni@redhat.com>, Simon Horman <horms@kernel.org>
Cc: Josef Bacik <josef@toxicpanda.com>,
	 Benjamin Coddington <bcodding@redhat.com>,
	linux-nfs@vger.kernel.org,  linux-kernel@vger.kernel.org,
	netdev@vger.kernel.org,  Jeff Layton <jlayton@kernel.org>
Subject: [PATCH RFC 8/9] sunrpc: don't hold a struct net reference in rpc_xprt
Date: Mon, 17 Mar 2025 17:00:00 -0400	[thread overview]
Message-ID: <20250317-rpc-shutdown-v1-8-85ba8e20b75d@kernel.org> (raw)
In-Reply-To: <20250317-rpc-shutdown-v1-0-85ba8e20b75d@kernel.org>

Currently each rpc_xprt holds a reference to struct net. This is
problematic for at least a couple of reasons:

1/ a container with an nfs mount inside can spontaneously die and take
network connectivity with it. When this happens, the netns and rpc_clnt
stick around and the client continually tries to retransmit any RPCs in
a net namespace that is otherwise defunct.

2/ the gssproxy rpc_clnt is torn down when the netns exits, but that
can never happen due to the circular dependency. The result is that any
container that runs gssproxy will leak the netns on exit.

Instead of holding a reference to the netns in rpc_xprt, add a pre_exit
routine that will shut down all rpc_clnt's associated with the netns,
and then wait for the list to go empty.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 include/linux/sunrpc/xprt.h |  1 -
 net/sunrpc/clnt.c           |  2 ++
 net/sunrpc/sunrpc_syms.c    | 29 +++++++++++++++++++++++++++++
 net/sunrpc/xprt.c           |  3 +--
 4 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/include/linux/sunrpc/xprt.h b/include/linux/sunrpc/xprt.h
index 81b952649d35e3ad4fa8c7e77388ac2ceb44ce60..67c41917e3d83fc0b5c2240f2f1243a2b67da0ec 100644
--- a/include/linux/sunrpc/xprt.h
+++ b/include/linux/sunrpc/xprt.h
@@ -296,7 +296,6 @@ struct rpc_xprt {
 	} stat;
 
 	struct net		*xprt_net;
-	netns_tracker		ns_tracker;
 	const char		*servername;
 	const char		*address_strings[RPC_DISPLAY_MAX];
 #if IS_ENABLED(CONFIG_SUNRPC_DEBUG)
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index 0028858b12d97e7b45f4c24cfbd761ba2a734b32..80cd1ddd155db64fc5b2449ebf54714d80a2838c 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -91,6 +91,8 @@ static void rpc_unregister_client(struct rpc_clnt *clnt)
 
 	spin_lock(&sn->rpc_client_lock);
 	list_del(&clnt->cl_clients);
+	if (list_empty(&sn->all_clients))
+		wake_up_var(&sn->all_clients);
 	spin_unlock(&sn->rpc_client_lock);
 }
 
diff --git a/net/sunrpc/sunrpc_syms.c b/net/sunrpc/sunrpc_syms.c
index bab6cab2940524a970422b62b3fa4212c61c4f43..d919f77522a7c6f7c477b2674f0b3a54155c91d9 100644
--- a/net/sunrpc/sunrpc_syms.c
+++ b/net/sunrpc/sunrpc_syms.c
@@ -77,9 +77,38 @@ static __net_exit void sunrpc_exit_net(struct net *net)
 	WARN_ON_ONCE(!list_empty(&sn->all_clients));
 }
 
+static void shutdown_all_clients(struct sunrpc_net *sn)
+{
+	struct rpc_clnt *clnt;
+
+	lockdep_assert_held(&sn->rpc_client_lock);
+
+	list_for_each_entry(clnt, &sn->all_clients, cl_clients)
+		rpc_clnt_shutdown(clnt);
+}
+
+static bool all_clients_gone(struct sunrpc_net *sn)
+{
+	bool empty;
+
+	spin_lock(&sn->rpc_client_lock);
+	empty = list_empty(&sn->all_clients);
+	spin_unlock(&sn->rpc_client_lock);
+	return empty;
+}
+
+static void sunrpc_pre_exit_net(struct net *net)
+{
+	struct sunrpc_net *sn = net_generic(net, sunrpc_net_id);
+
+	shutdown_all_clients(sn);
+	wait_var_event(&sn->all_clients, all_clients_gone(sn));
+}
+
 static struct pernet_operations sunrpc_net_ops = {
 	.init = sunrpc_init_net,
 	.exit = sunrpc_exit_net,
+	.pre_exit = sunrpc_pre_exit_net,
 	.id = &sunrpc_net_id,
 	.size = sizeof(struct sunrpc_net),
 };
diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
index 09f245cda5262a572c450237419c80b183a83568..040cc9bf92cfa8f28edaee707a09a9e8d9955c41 100644
--- a/net/sunrpc/xprt.c
+++ b/net/sunrpc/xprt.c
@@ -1849,7 +1849,6 @@ EXPORT_SYMBOL_GPL(xprt_alloc);
 
 void xprt_free(struct rpc_xprt *xprt)
 {
-	put_net_track(xprt->xprt_net, &xprt->ns_tracker);
 	xprt_free_all_slots(xprt);
 	xprt_free_id(xprt);
 	rpc_sysfs_xprt_destroy(xprt);
@@ -2047,7 +2046,7 @@ static void xprt_init(struct rpc_xprt *xprt, struct net *net)
 
 	xprt_init_xid(xprt);
 
-	xprt->xprt_net = get_net_track(net, &xprt->ns_tracker, GFP_KERNEL);
+	xprt->xprt_net = net;
 }
 
 /**

-- 
2.48.1


  parent reply	other threads:[~2025-03-17 21:00 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-17 20:59 [PATCH RFC 0/9] nfs/sunrpc: stop holding netns references in client-side NFS and RPC objects Jeff Layton
2025-03-17 20:59 ` [PATCH RFC 1/9] sunrpc: transplant shutdown_client() to sunrpc module Jeff Layton
2025-03-17 20:59 ` [PATCH RFC 2/9] lockd: add a helper to shut down rpc_clnt in nlm_host Jeff Layton
2025-03-17 20:59 ` [PATCH RFC 3/9] lockd: don't #include debug.h from lockd.h Jeff Layton
2025-03-17 20:59 ` [PATCH RFC 4/9] nfs: transplant nfs_server shutdown into a helper function Jeff Layton
2025-03-17 20:59 ` [PATCH RFC 5/9] nfs: don't hold a reference to struct net in struct nfs_client Jeff Layton
2025-03-17 20:59 ` [PATCH RFC 6/9] auth_gss: shut down gssproxy rpc_clnt in net pre_exit Jeff Layton
2025-03-17 20:59 ` [PATCH RFC 7/9] auth_gss: don't hold a net reference in gss_auth Jeff Layton
2025-03-17 21:00 ` Jeff Layton [this message]
2025-03-17 21:00 ` [PATCH RFC 9/9] sunrpc: don't upgrade passive net reference in xs_create_sock Jeff Layton
2025-03-17 21:28   ` Trond Myklebust
2025-03-17 21:36     ` Jeff Layton
2025-03-17 21:37       ` Trond Myklebust
2025-03-17 21:41         ` Jeff Layton
2025-03-17 21:35 ` [PATCH RFC 0/9] nfs/sunrpc: stop holding netns references in client-side NFS and RPC objects Trond Myklebust
2025-03-17 21:57   ` Jeff Layton
2025-03-17 22:11     ` Trond Myklebust
2025-03-18 11:30       ` Jeff Layton

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=20250317-rpc-shutdown-v1-8-85ba8e20b75d@kernel.org \
    --to=jlayton@kernel.org \
    --cc=Dai.Ngo@oracle.com \
    --cc=anna@kernel.org \
    --cc=bcodding@redhat.com \
    --cc=chuck.lever@oracle.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=josef@toxicpanda.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=neilb@suse.de \
    --cc=netdev@vger.kernel.org \
    --cc=okorniev@redhat.com \
    --cc=pabeni@redhat.com \
    --cc=tom@talpey.com \
    --cc=trondmy@kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox