SUNRPC: Fix up task signalling

The RPC_TASK_KILLED flag should really not be set from another context
because it can clobber data in the struct task when task->tk_flags is
changed non-atomically.
Let's therefore swap out RPC_TASK_KILLED with an atomic flag, and add
a function to set that flag and safely wake up the task.

Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>

diff --git a/fs/lockd/clntproc.c b/fs/lockd/clntproc.c
index e8a004097d18754e932d26a52ed4ca993b671f95..d9c32d1a20c0bbeeecd65ecd5b0838b7641dd4c2 100644 (file)
--- a/fs/lockd/clntproc.c
+++ b/fs/lockd/clntproc.c
@@ -715,7 +715,7 @@ static void nlmclnt_unlock_callback(struct rpc_task *task, void *data)
        struct nlm_rqst *req = data;
        u32 status = ntohl(req->a_res.status);
 
-       if (RPC_ASSASSINATED(task))
+       if (RPC_SIGNALLED(task))
                goto die;
 
        if (task->tk_status < 0) {
@@ -783,7 +783,7 @@ static void nlmclnt_cancel_callback(struct rpc_task *task, void *data)
        struct nlm_rqst *req = data;
        u32 status = ntohl(req->a_res.status);
 
-       if (RPC_ASSASSINATED(task))
+       if (RPC_SIGNALLED(task))
                goto die;
 
        if (task->tk_status < 0) {

diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
index d219159b98afc54bda6d2efee824b41487db17c0..f7494be8dbe26e0b95ba8a7f3a71610d66fffb64 100644 (file)
--- a/fs/nfsd/nfs4callback.c
+++ b/fs/nfsd/nfs4callback.c
@@ -1032,7 +1032,7 @@ static bool nfsd4_cb_sequence_done(struct rpc_task *task, struct nfsd4_callback
                 * the submission code will error out, so we don't need to
                 * handle that case here.
                 */
-               if (task->tk_flags & RPC_TASK_KILLED)
+               if (RPC_SIGNALLED(task))
                        goto need_restart;
 
                return true;
@@ -1081,7 +1081,7 @@ static bool nfsd4_cb_sequence_done(struct rpc_task *task, struct nfsd4_callback
        dprintk("%s: freed slot, new seqid=%d\n", __func__,
                clp->cl_cb_session->se_cb_seq_nr);
 
-       if (task->tk_flags & RPC_TASK_KILLED)
+       if (RPC_SIGNALLED(task))
                goto need_restart;
 out:
        return ret;

diff --git a/include/linux/sunrpc/sched.h b/include/linux/sunrpc/sched.h
index 52d41d0c1ae1d54b6829a10134318ee2835b9fd5..90e06c67f455767d38814c11a4fa077ad8e10034 100644 (file)
--- a/include/linux/sunrpc/sched.h
+++ b/include/linux/sunrpc/sched.h
@@ -125,7 +125,6 @@ struct rpc_task_setup {
 #define RPC_CALL_MAJORSEEN     0x0020          /* major timeout seen */
 #define RPC_TASK_ROOTCREDS     0x0040          /* force root creds */
 #define RPC_TASK_DYNAMIC       0x0080          /* task was kmalloc'ed */
-#define RPC_TASK_KILLED                0x0100          /* task was killed */
 #define RPC_TASK_SOFT          0x0200          /* Use soft timeouts */
 #define RPC_TASK_SOFTCONN      0x0400          /* Fail if can't connect */
 #define RPC_TASK_SENT          0x0800          /* message was sent */
@@ -135,7 +134,6 @@ struct rpc_task_setup {
 
 #define RPC_IS_ASYNC(t)                ((t)->tk_flags & RPC_TASK_ASYNC)
 #define RPC_IS_SWAPPER(t)      ((t)->tk_flags & RPC_TASK_SWAPPER)
-#define RPC_ASSASSINATED(t)    ((t)->tk_flags & RPC_TASK_KILLED)
 #define RPC_IS_SOFT(t)         ((t)->tk_flags & (RPC_TASK_SOFT|RPC_TASK_TIMEOUT))
 #define RPC_IS_SOFTCONN(t)     ((t)->tk_flags & RPC_TASK_SOFTCONN)
 #define RPC_WAS_SENT(t)                ((t)->tk_flags & RPC_TASK_SENT)
@@ -146,6 +144,7 @@ struct rpc_task_setup {
 #define RPC_TASK_NEED_XMIT     3
 #define RPC_TASK_NEED_RECV     4
 #define RPC_TASK_MSG_PIN_WAIT  5
+#define RPC_TASK_SIGNALLED     6
 
 #define RPC_IS_RUNNING(t)      test_bit(RPC_TASK_RUNNING, &(t)->tk_runstate)
 #define rpc_set_running(t)     set_bit(RPC_TASK_RUNNING, &(t)->tk_runstate)
@@ -169,6 +168,8 @@ struct rpc_task_setup {
 
 #define RPC_IS_ACTIVATED(t)    test_bit(RPC_TASK_ACTIVE, &(t)->tk_runstate)
 
+#define RPC_SIGNALLED(t)       test_bit(RPC_TASK_SIGNALLED, &(t)->tk_runstate)
+
 /*
  * Task priorities.
  * Note: if you change these, you must also change
@@ -217,6 +218,7 @@ struct rpc_task *rpc_run_task(const struct rpc_task_setup *);
 struct rpc_task *rpc_run_bc_task(struct rpc_rqst *req);
 void           rpc_put_task(struct rpc_task *);
 void           rpc_put_task_async(struct rpc_task *);
+void           rpc_signal_task(struct rpc_task *);
 void           rpc_exit_task(struct rpc_task *);
 void           rpc_exit(struct rpc_task *, int);
 void           rpc_release_calldata(const struct rpc_call_ops *, void *);

diff --git a/include/trace/events/sunrpc.h b/include/trace/events/sunrpc.h
index 7e899e635d33abaecc009af281be99f7475f5d85..5e3b77d9daa72ca78dd25f00df1b7ba3ddc51df2 100644 (file)
--- a/include/trace/events/sunrpc.h
+++ b/include/trace/events/sunrpc.h
@@ -82,7 +82,6 @@ TRACE_DEFINE_ENUM(RPC_TASK_SWAPPER);
 TRACE_DEFINE_ENUM(RPC_CALL_MAJORSEEN);
 TRACE_DEFINE_ENUM(RPC_TASK_ROOTCREDS);
 TRACE_DEFINE_ENUM(RPC_TASK_DYNAMIC);
-TRACE_DEFINE_ENUM(RPC_TASK_KILLED);
 TRACE_DEFINE_ENUM(RPC_TASK_SOFT);
 TRACE_DEFINE_ENUM(RPC_TASK_SOFTCONN);
 TRACE_DEFINE_ENUM(RPC_TASK_SENT);
@@ -97,7 +96,6 @@ TRACE_DEFINE_ENUM(RPC_TASK_NO_RETRANS_TIMEOUT);
                { RPC_CALL_MAJORSEEN, "MAJORSEEN" },                    \
                { RPC_TASK_ROOTCREDS, "ROOTCREDS" },                    \
                { RPC_TASK_DYNAMIC, "DYNAMIC" },                        \
-               { RPC_TASK_KILLED, "KILLED" },                          \
                { RPC_TASK_SOFT, "SOFT" },                              \
                { RPC_TASK_SOFTCONN, "SOFTCONN" },                      \
                { RPC_TASK_SENT, "SENT" },                              \
@@ -111,6 +109,7 @@ TRACE_DEFINE_ENUM(RPC_TASK_ACTIVE);
 TRACE_DEFINE_ENUM(RPC_TASK_NEED_XMIT);
 TRACE_DEFINE_ENUM(RPC_TASK_NEED_RECV);
 TRACE_DEFINE_ENUM(RPC_TASK_MSG_PIN_WAIT);
+TRACE_DEFINE_ENUM(RPC_TASK_SIGNALLED);
 
 #define rpc_show_runstate(flags)                                       \
        __print_flags(flags, "|",                                       \
@@ -119,7 +118,8 @@ TRACE_DEFINE_ENUM(RPC_TASK_MSG_PIN_WAIT);
                { (1UL << RPC_TASK_ACTIVE), "ACTIVE" },                 \
                { (1UL << RPC_TASK_NEED_XMIT), "NEED_XMIT" },           \
                { (1UL << RPC_TASK_NEED_RECV), "NEED_RECV" },           \
-               { (1UL << RPC_TASK_MSG_PIN_WAIT), "MSG_PIN_WAIT" })
+               { (1UL << RPC_TASK_MSG_PIN_WAIT), "MSG_PIN_WAIT" },     \
+               { (1UL << RPC_TASK_SIGNALLED), "SIGNALLED" })
 
 DECLARE_EVENT_CLASS(rpc_task_running,
 

diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index 8ff11dc98d7f93fefeff6ecc53ff6d7815da47f8..18f5392aa550aa8a90fa5eeaeaedfb47eef89e83 100644 (file)
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -827,14 +827,8 @@ void rpc_killall_tasks(struct rpc_clnt *clnt)
         * Spin lock all_tasks to prevent changes...
         */
        spin_lock(&clnt->cl_lock);
-       list_for_each_entry(rovr, &clnt->cl_tasks, tk_task) {
-               if (!RPC_IS_ACTIVATED(rovr))
-                       continue;
-               if (!(rovr->tk_flags & RPC_TASK_KILLED)) {
-                       rovr->tk_flags |= RPC_TASK_KILLED;
-                       rpc_exit(rovr, -EIO);
-               }
-       }
+       list_for_each_entry(rovr, &clnt->cl_tasks, tk_task)
+               rpc_signal_task(rovr);
        spin_unlock(&clnt->cl_lock);
 }
 EXPORT_SYMBOL_GPL(rpc_killall_tasks);
@@ -1477,8 +1471,6 @@ EXPORT_SYMBOL_GPL(rpc_force_rebind);
 int
 rpc_restart_call_prepare(struct rpc_task *task)
 {
-       if (RPC_ASSASSINATED(task))
-               return 0;
        task->tk_action = call_start;
        task->tk_status = 0;
        if (task->tk_ops->rpc_call_prepare != NULL)
@@ -1494,8 +1486,6 @@ EXPORT_SYMBOL_GPL(rpc_restart_call_prepare);
 int
 rpc_restart_call(struct rpc_task *task)
 {
-       if (RPC_ASSASSINATED(task))
-               return 0;
        task->tk_action = call_start;
        task->tk_status = 0;
        return 1;

diff --git a/net/sunrpc/sched.c b/net/sunrpc/sched.c
index 28956c70100af03916fdcf19a520e7c784d1ba3b..3d6cb91ba598f444559cfa733e74d9a4b0f2bfeb 100644 (file)
--- a/net/sunrpc/sched.c
+++ b/net/sunrpc/sched.c
@@ -759,8 +759,7 @@ static void
 rpc_reset_task_statistics(struct rpc_task *task)
 {
        task->tk_timeouts = 0;
-       task->tk_flags &= ~(RPC_CALL_MAJORSEEN|RPC_TASK_KILLED|RPC_TASK_SENT);
-
+       task->tk_flags &= ~(RPC_CALL_MAJORSEEN|RPC_TASK_SENT);
        rpc_init_task_statistics(task);
 }
 
@@ -773,7 +772,6 @@ void rpc_exit_task(struct rpc_task *task)
        if (task->tk_ops->rpc_call_done != NULL) {
                task->tk_ops->rpc_call_done(task, task->tk_calldata);
                if (task->tk_action != NULL) {
-                       WARN_ON(RPC_ASSASSINATED(task));
                        /* Always release the RPC slot and buffer memory */
                        xprt_release(task);
                        rpc_reset_task_statistics(task);
@@ -781,6 +779,19 @@ void rpc_exit_task(struct rpc_task *task)
        }
 }
 
+void rpc_signal_task(struct rpc_task *task)
+{
+       struct rpc_wait_queue *queue;
+
+       if (!RPC_IS_ACTIVATED(task))
+               return;
+       set_bit(RPC_TASK_SIGNALLED, &task->tk_runstate);
+       smp_mb__after_atomic();
+       queue = READ_ONCE(task->tk_waitqueue);
+       if (queue)
+               rpc_wake_up_queued_task_set_status(queue, task, -ERESTARTSYS);
+}
+
 void rpc_exit(struct rpc_task *task, int status)
 {
        task->tk_status = status;
@@ -836,6 +847,13 @@ static void __rpc_execute(struct rpc_task *task)
                 */
                if (!RPC_IS_QUEUED(task))
                        continue;
+
+               /*
+                * Signalled tasks should exit rather than sleep.
+                */
+               if (RPC_SIGNALLED(task))
+                       rpc_exit(task, -ERESTARTSYS);
+
                /*
                 * The queue->lock protects against races with
                 * rpc_make_runnable().
@@ -861,7 +879,7 @@ static void __rpc_execute(struct rpc_task *task)
                status = out_of_line_wait_on_bit(&task->tk_runstate,
                                RPC_TASK_QUEUED, rpc_wait_bit_killable,
                                TASK_KILLABLE);
-               if (status == -ERESTARTSYS) {
+               if (status < 0) {
                        /*
                         * When a sync task receives a signal, it exits with
                         * -ERESTARTSYS. In order to catch any callbacks that
@@ -869,7 +887,7 @@ static void __rpc_execute(struct rpc_task *task)
                         * break the loop here, but go around once more.
                         */
                        dprintk("RPC: %5u got signal\n", task->tk_pid);
-                       task->tk_flags |= RPC_TASK_KILLED;
+                       set_bit(RPC_TASK_SIGNALLED, &task->tk_runstate);
                        rpc_exit(task, -ERESTARTSYS);
                }
                dprintk("RPC: %5u sync task resuming\n", task->tk_pid);

diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
index d7117d24146017e477f0e4ec39dbc6f2c08b11e1..3a4156cb0134522513abb5f82371dc7003c0dd2d 100644 (file)
--- a/net/sunrpc/xprt.c
+++ b/net/sunrpc/xprt.c
@@ -1337,6 +1337,10 @@ xprt_request_transmit(struct rpc_rqst *req, struct rpc_task *snd_task)
                        if (status < 0)
                                goto out_dequeue;
                }
+               if (RPC_SIGNALLED(task)) {
+                       status = -ERESTARTSYS;
+                       goto out_dequeue;
+               }
        }
 
        /*