From: NeilBrown <neilb@suse.de>
To: Chuck Lever <chuck.lever@oracle.com>, Jeff Layton <jlayton@kernel.org>
Cc: linux-nfs@vger.kernel.org, Olga Kornievskaia <kolga@netapp.com>,
Dai Ngo <Dai.Ngo@oracle.com>, Tom Talpey <tom@talpey.com>
Subject: [PATCH 2/2] sunrpc: allow svc threads to fail initialisation cleanly
Date: Mon, 29 Jul 2024 12:18:21 +1000 [thread overview]
Message-ID: <20240729022126.4450-3-neilb@suse.de> (raw)
In-Reply-To: <20240729022126.4450-1-neilb@suse.de>
If an svc thread needs to perform some initialisation that might fail,
it has no good way to handle the failure.
Before the thread can exit it must call svc_exit_thread(), but that
requires the service mutex to be held. The thread cannot simply take
the mutex as that could deadlock if there is a concurrent attempt to
shut down all threads (which is unlikely, but not impossible).
nfsd currently call svc_exit_thread() unprotected in the unlikely event
that unshare_fs_struct() fails.
We can clean this up by introducing svc_thread_init_status() by which an
svc thread can report whether initialisation has succeeded. If it has,
it continues normally into the action loop. If it has not,
svc_thread_init_status() immediately aborts the thread.
svc_start_kthread() waits for either of these to happen, and calls
svc_exit_thread() (under the mutex) if the thread aborted.
Signed-off-by: NeilBrown <neilb@suse.de>
---
fs/lockd/svc.c | 2 ++
fs/nfs/callback.c | 2 ++
fs/nfsd/nfssvc.c | 9 +++------
include/linux/sunrpc/svc.h | 28 ++++++++++++++++++++++++++++
net/sunrpc/svc.c | 11 +++++++++++
5 files changed, 46 insertions(+), 6 deletions(-)
diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
index ab8042a5b895..7d74858e1abe 100644
--- a/fs/lockd/svc.c
+++ b/fs/lockd/svc.c
@@ -125,6 +125,8 @@ lockd(void *vrqstp)
struct net *net = &init_net;
struct lockd_net *ln = net_generic(net, lockd_net_id);
+ svc_thread_init_status(rqstp, 0);
+
/* try_to_freeze() is called from svc_recv() */
set_freezable();
diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c
index 8adfcd4c8c1a..6cf92498a5ac 100644
--- a/fs/nfs/callback.c
+++ b/fs/nfs/callback.c
@@ -76,6 +76,8 @@ nfs4_callback_svc(void *vrqstp)
{
struct svc_rqst *rqstp = vrqstp;
+ svc_thread_init_status(rqstp, 0);
+
set_freezable();
while (!svc_thread_should_stop(rqstp))
diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index 89d7918de7b1..2a99787d6a86 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -915,11 +915,9 @@ nfsd(void *vrqstp)
/* At this point, the thread shares current->fs
* with the init process. We need to create files with the
- * umask as defined by the client instead of init's umask. */
- if (unshare_fs_struct() < 0) {
- printk("Unable to start nfsd thread: out of memory\n");
- goto out;
- }
+ * umask as defined by the client instead of init's umask.
+ */
+ svc_thread_init_status(rqstp, unshare_fs_struct());
current->fs->umask = 0;
@@ -941,7 +939,6 @@ nfsd(void *vrqstp)
atomic_dec(&nfsd_th_cnt);
-out:
/* Release the thread */
svc_exit_thread(rqstp);
return 0;
diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
index 01383ea077ad..2c4366afc9bd 100644
--- a/include/linux/sunrpc/svc.h
+++ b/include/linux/sunrpc/svc.h
@@ -21,6 +21,7 @@
#include <linux/wait.h>
#include <linux/mm.h>
#include <linux/pagevec.h>
+#include <linux/kthread.h>
/*
*
@@ -231,6 +232,11 @@ struct svc_rqst {
struct net *rq_bc_net; /* pointer to backchannel's
* net namespace
*/
+
+ int rq_err; /* Thread sets this to inidicate
+ * initialisation success.
+ */
+
unsigned long bc_to_initval;
unsigned int bc_to_retries;
void ** rq_lease_breaker; /* The v4 client breaking a lease */
@@ -304,6 +310,28 @@ static inline bool svc_thread_should_stop(struct svc_rqst *rqstp)
return test_bit(RQ_VICTIM, &rqstp->rq_flags);
}
+/**
+ * svc_thread_init_status - report whether thread has initialised successfully
+ * @rqstp: the thread in question
+ * @err: errno code
+ *
+ * After performing any initialisation that could fail, and before starting
+ * normal work, each sunrpc svc_thread must call svc_thread_init_status()
+ * with an appropriate error, or zero.
+ *
+ * If zero is passed, the thread is ready and must continue until
+ * svc_thread_should_stop() returns true. If a non-zero error is passed
+ * the call will not return - the thread will exit.
+ */
+static inline void svc_thread_init_status(struct svc_rqst *rqstp, int err)
+{
+ /* store_release ensures svc_start_kthreads() sees the error */
+ smp_store_release(&rqstp->rq_err, err);
+ wake_up_var(&rqstp->rq_err);
+ if (err)
+ kthread_exit(1);
+}
+
struct svc_deferred_req {
u32 prot; /* protocol (UDP or TCP) */
struct svc_xprt *xprt;
diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index c0e91bea72e7..0fc9c97ae419 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -662,6 +662,8 @@ svc_prepare_thread(struct svc_serv *serv, struct svc_pool *pool, int node)
if (!svc_init_buffer(rqstp, serv->sv_max_mesg, node))
goto out_enomem;
+ rqstp->rq_err = -EAGAIN; /* No error yet */
+
spin_lock_bh(&serv->sv_lock);
serv->sv_nrthreads += 1;
spin_unlock_bh(&serv->sv_lock);
@@ -760,6 +762,7 @@ svc_start_kthreads(struct svc_serv *serv, struct svc_pool *pool, int nrservs)
struct svc_pool *chosen_pool;
unsigned int state = serv->sv_nrthreads-1;
int node;
+ int err;
do {
nrservs--;
@@ -782,6 +785,14 @@ svc_start_kthreads(struct svc_serv *serv, struct svc_pool *pool, int nrservs)
svc_sock_update_bufs(serv);
wake_up_process(task);
+
+ /* load_acquire ensures we get value stored in svc_thread_init_status() */
+ wait_var_event(&rqstp->rq_err, smp_load_acquire(&rqstp->rq_err) != -EAGAIN);
+ err = rqstp->rq_err;
+ if (err) {
+ svc_exit_thread(rqstp);
+ return err;
+ }
} while (nrservs > 0);
return 0;
--
2.44.0
next prev parent reply other threads:[~2024-07-29 2:22 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-07-29 2:18 [PATCH 0/2] nfsd: fix handling of error from unshare_fs_struct() NeilBrown
2024-07-29 2:18 ` [PATCH 1/2] sunrpc: merge svc_rqst_alloc() into svc_prepare_thread() NeilBrown
2024-07-29 2:18 ` NeilBrown [this message]
2024-07-29 12:44 ` [PATCH 0/2] nfsd: fix handling of error from unshare_fs_struct() Jeff Layton
2024-07-29 14:36 ` Chuck Lever
2024-07-29 21:07 ` NeilBrown
2024-07-29 21:25 ` Chuck Lever III
2024-07-29 23:17 ` NeilBrown
2024-07-29 22:01 ` Christoph Hellwig
2024-07-30 0:43 ` NeilBrown
-- strict thread matches above, loose matches on Subject: below --
2024-07-29 21:19 [PATCH 0/2 v2] " NeilBrown
2024-07-29 21:19 ` [PATCH 2/2] sunrpc: allow svc threads to fail initialisation cleanly NeilBrown
2024-09-11 18:33 ` Jeff Layton
2024-09-13 16:14 ` Chuck Lever III
2024-09-15 23:47 ` 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=20240729022126.4450-3-neilb@suse.de \
--to=neilb@suse.de \
--cc=Dai.Ngo@oracle.com \
--cc=chuck.lever@oracle.com \
--cc=jlayton@kernel.org \
--cc=kolga@netapp.com \
--cc=linux-nfs@vger.kernel.org \
--cc=tom@talpey.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox