[PATCH 1/2] SUNRPC: maintain an rpc_task signalled flag

[Tim Menninger <tmenninger@everpuredata.com>]

Commit 5bbd6e863b15a ("SUNRPC: Prevent looping due to rpc_signal_task()
races") removed the RPC_TASK_SIGNALLED runstate flag because maintaining
it independently from the errno in tk_rpc_status introduced races
between an errno being recorded and the task being in a SIGNALLED state.
A signalled task is inferred by tk_rpc_status containing -ERESTARTSYS.

This reintroduces the concept by coupling a "task signalled" bit with
the errno. The tk_rpc_status int becomes a tk_rpc_error_state atomic64_t
whose low 32 bits are a signed errno representing the first observed RPC
error, as tk_rpc_status previously held, and bit 32 is 1 if the task is
signalled and 0 otherwise.

With this change, we can record an RPC status and later escalate the
status to "signalled" without overwriting (and losing) the first
recorded error.

Signed-off-by: Tim Menninger <tmenninger@everpuredata.com>
---
 fs/nfs/nfs4proc.c            | 12 ++++++++----
 fs/nfs/pnfs.c                | 11 +++++++----
 include/linux/sunrpc/sched.h | 21 +++++++++++++++++++--
 net/sunrpc/clnt.c            |  2 +-
 net/sunrpc/sched.c           | 34 +++++++++++++++++++++++++++-------
 5 files changed, 62 insertions(+), 18 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index e1018b03c1cbd..1face07f409b4 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -688,9 +688,10 @@ nfs4_async_handle_exception(struct rpc_task *task, struct nfs_server *server,
 {
 	struct nfs_client *clp = server->nfs_client;
 	int ret;
+	int task_rpc_status = rpc_task_rpc_status(task);
 
-	if ((task->tk_rpc_status == -ENETDOWN ||
-	     task->tk_rpc_status == -ENETUNREACH) &&
+	if ((task_rpc_status == -ENETDOWN ||
+	     task_rpc_status == -ENETUNREACH) &&
 	    task->tk_flags & RPC_TASK_NETUNREACH_FATAL) {
 		exception->delay = 0;
 		exception->recovering = 0;
@@ -9845,12 +9846,15 @@ static void nfs4_layoutreturn_done(struct rpc_task *task, void *calldata)
 {
 	struct nfs4_layoutreturn *lrp = calldata;
 	struct nfs_server *server;
+	int task_rpc_status;
 
 	if (!nfs41_sequence_process(task, &lrp->res.seq_res))
 		return;
 
-	if (task->tk_rpc_status < 0) {
-		switch (task->tk_rpc_status) {
+	task_rpc_status = rpc_task_rpc_status(task);
+
+	if (task_rpc_status < 0) {
+		switch (task_rpc_status) {
 		case -EACCES:
 		case -EIO:
 		case -EKEYEXPIRED:
diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index a057a06318c76..c51654e9d32eb 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -1687,6 +1687,7 @@ int pnfs_roc_done(struct rpc_task *task, struct nfs4_layoutreturn_args **argpp,
 {
 	struct nfs4_layoutreturn_args *arg = *argpp;
 	int retval = -EAGAIN;
+	int task_rpc_status;
 
 	if (!arg)
 		return 0;
@@ -1696,15 +1697,17 @@ int pnfs_roc_done(struct rpc_task *task, struct nfs4_layoutreturn_args **argpp,
 		retval = 0;
 		break;
 	case -NFS4ERR_NOMATCHING_LAYOUT:
+		task_rpc_status = rpc_task_rpc_status(task);
+
 		/* Was there an RPC level error? If not, retry */
-		if (task->tk_rpc_status == 0)
+		if (task_rpc_status == 0)
 			break;
 		/*
 		 * Is there a fatal network level error?
 		 * If so release the layout, but flag the error.
 		 */
-		if ((task->tk_rpc_status == -ENETDOWN ||
-		     task->tk_rpc_status == -ENETUNREACH) &&
+		if ((task_rpc_status == -ENETDOWN ||
+		     task_rpc_status == -ENETUNREACH) &&
 		    task->tk_flags & RPC_TASK_NETUNREACH_FATAL) {
 			*ret = 0;
 			(*respp)->lrs_present = 0;
@@ -1714,7 +1717,7 @@ int pnfs_roc_done(struct rpc_task *task, struct nfs4_layoutreturn_args **argpp,
 		/* If the call was not sent, let caller handle it */
 		if (!RPC_WAS_SENT(task))
 			return 0;
-		switch (task->tk_rpc_status) {
+		switch (task_rpc_status) {
 		default:
 			/*
 			 * Defer the layoutreturn if it was due
diff --git a/include/linux/sunrpc/sched.h b/include/linux/sunrpc/sched.h
index dbaad8b5f181c..db322e26703a4 100644
--- a/include/linux/sunrpc/sched.h
+++ b/include/linux/sunrpc/sched.h
@@ -10,6 +10,7 @@
 #ifndef _LINUX_SUNRPC_SCHED_H_
 #define _LINUX_SUNRPC_SCHED_H_
 
+#include <linux/atomic.h>
 #include <linux/timer.h>
 #include <linux/ktime.h>
 #include <linux/sunrpc/types.h>
@@ -92,7 +93,10 @@ struct rpc_task {
 
 	pid_t			tk_owner;	/* Process id for batching tasks */
 
-	int			tk_rpc_status;	/* Result of last RPC operation */
+	atomic64_t		tk_rpc_error_state;
+						/* Low 32 bits: first recorded RPC error.
+						 * Bit 32: signal requested.
+						 */
 	unsigned short		tk_flags;	/* misc flags */
 	unsigned short		tk_timeouts;	/* maj timeouts */
 	unsigned short		tk_pid;		/* debugging aid */
@@ -158,6 +162,18 @@ enum {
 	RPC_TASK_MSG_PIN_WAIT,
 };
 
+#define RPC_TASK_ERR_STATE_SIGNALLED	(1LL << 32)
+
+static inline s64 rpc_task_read_error_state(const struct rpc_task *task)
+{
+	return atomic64_read(&task->tk_rpc_error_state);
+}
+
+static inline int rpc_task_rpc_status(const struct rpc_task *task)
+{
+	return (int)(s32)rpc_task_read_error_state(task);
+}
+
 #define rpc_test_and_set_running(t) \
 				test_and_set_bit(RPC_TASK_RUNNING, &(t)->tk_runstate)
 #define rpc_clear_running(t)	clear_bit(RPC_TASK_RUNNING, &(t)->tk_runstate)
@@ -168,7 +184,8 @@ enum {
 
 #define RPC_IS_ACTIVATED(t)	test_bit(RPC_TASK_ACTIVE, &(t)->tk_runstate)
 
-#define RPC_SIGNALLED(t)	(READ_ONCE(task->tk_rpc_status) == -ERESTARTSYS)
+#define RPC_SIGNALLED(t)	(!!(atomic64_read(&(t)->tk_rpc_error_state) & \
+				    RPC_TASK_ERR_STATE_SIGNALLED))
 
 /*
  * Task priorities.
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index 5c33bf3fb4c34..cf17dfff57e90 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -1758,7 +1758,7 @@ static int
 __rpc_restart_call(struct rpc_task *task, void (*action)(struct rpc_task *))
 {
 	task->tk_status = 0;
-	task->tk_rpc_status = 0;
+	atomic64_set(&task->tk_rpc_error_state, 0);
 	task->tk_action = action;
 	return 1;
 }
diff --git a/net/sunrpc/sched.c b/net/sunrpc/sched.c
index 016f16ca57799..81890327d81a9 100644
--- a/net/sunrpc/sched.c
+++ b/net/sunrpc/sched.c
@@ -67,9 +67,17 @@ EXPORT_SYMBOL_GPL(rpc_task_gfp_mask);
 
 bool rpc_task_set_rpc_status(struct rpc_task *task, int rpc_status)
 {
-	if (cmpxchg(&task->tk_rpc_status, 0, rpc_status) == 0)
-		return true;
-	return false;
+	s64 old = atomic64_read(&task->tk_rpc_error_state);
+	s64 new;
+
+	for (;;) {
+		if ((s32)old)
+			return false;
+		new = (old & RPC_TASK_ERR_STATE_SIGNALLED) |
+		      ((s64)(u32)rpc_status);
+		if (atomic64_try_cmpxchg(&task->tk_rpc_error_state, &old, new))
+			return true;
+	}
 }
 
 unsigned long
@@ -857,12 +865,24 @@ void rpc_exit_task(struct rpc_task *task)
 void rpc_signal_task(struct rpc_task *task)
 {
 	struct rpc_wait_queue *queue;
+	s64 old, new;
 
 	if (!RPC_IS_ACTIVATED(task))
 		return;
 
-	if (!rpc_task_set_rpc_status(task, -ERESTARTSYS))
-		return;
+	old = atomic64_read(&task->tk_rpc_error_state);
+	for (;;) {
+		if (old & RPC_TASK_ERR_STATE_SIGNALLED)
+			return;
+
+		new = old | RPC_TASK_ERR_STATE_SIGNALLED;
+		if (!((s32)old))
+			new |= (s64)(u32)-ERESTARTSYS;
+
+		if (atomic64_try_cmpxchg(&task->tk_rpc_error_state, &old, new))
+			break;
+	}
+
 	trace_rpc_task_signalled(task, task->tk_action);
 	queue = READ_ONCE(task->tk_waitqueue);
 	if (queue)
@@ -928,7 +948,7 @@ static void __rpc_execute(struct rpc_task *task)
 		do_action = task->tk_action;
 		/* Tasks with an RPC error status should exit */
 		if (do_action && do_action != rpc_exit_task &&
-		    (status = READ_ONCE(task->tk_rpc_status)) != 0) {
+		    (status = rpc_task_rpc_status(task)) != 0) {
 			task->tk_status = status;
 			do_action = rpc_exit_task;
 		}
@@ -970,7 +990,7 @@ static void __rpc_execute(struct rpc_task *task)
 			continue;
 		}
 		/* Wake up any task that has an exit status */
-		if (READ_ONCE(task->tk_rpc_status) != 0) {
+		if (rpc_task_rpc_status(task) != 0) {
 			rpc_wake_up_task_queue_locked(queue, task);
 			spin_unlock(&queue->lock);
 			continue;
-- 
2.53.0