sunrpc: allow svc threads to fail initialisation cleanly

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>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>

diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
index 71713309967d3b7de19da9ab58951878c4add78f..4ec22c2f2ea3c30d87717e5e87a7be4f4a9440ee 100644 (file)
--- a/fs/lockd/svc.c
+++ b/fs/lockd/svc.c
@@ -124,6 +124,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 8adfcd4c8c1a0a5447634438b467dae84af69a4a..6cf92498a5ac6f24dc86d4f871c546d513fae05b 100644 (file)
--- 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 b1dc3404173b38cef07b020dd5d114cecb860f66..3fb6c8c9a2f0df11bc8b4e0e14974540ad8bdb74 100644 (file)
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -873,11 +873,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;
 
@@ -899,7 +897,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 99e9345d829e82c947e4c25588149b03d0ba91fc..c419a61f60e5365c351949a908c138599a962713 100644 (file)
--- 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>
 
 /*
  *
@@ -232,6 +233,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 */
@@ -305,6 +311,31 @@ 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)
+{
+       rqstp->rq_err = err;
+       /* memory barrier ensures assignment to error above is visible before
+        * waitqueue_active() test below completes.
+        */
+       smp_mb();
+       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 17f0f59c068ff2e405f9e2f21b64526fcc6612cb..9aff845196ce76b7a1dcc9a44612e5d0054c860f 100644 (file)
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -706,6 +706,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 */
+
        serv->sv_nrthreads += 1;
        pool->sp_nrthreads += 1;
 
@@ -792,6 +794,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--;
@@ -814,6 +817,13 @@ svc_start_kthreads(struct svc_serv *serv, struct svc_pool *pool, int nrservs)
 
                svc_sock_update_bufs(serv);
                wake_up_process(task);
+
+               wait_var_event(&rqstp->rq_err, rqstp->rq_err != -EAGAIN);
+               err = rqstp->rq_err;
+               if (err) {
+                       svc_exit_thread(rqstp);
+                       return err;
+               }
        } while (nrservs > 0);
 
        return 0;