[RFC PATCH 0/1] A rare failure to wake up sync tasks

[RFC PATCH 0/1] A rare failure to wake up sync tasks

[Benjamin Coddington]

Hey all - RFC patch here because this race is very hard to reproduce so I
will be testing this for a few days before feeling certain that this fix
does the job.

Meantime, I'd love some feedback from any one that has mastered load/store
races.  Do I really need a full smp_mb() here?  I think I do because the
race is not to the tk_runstate's ACTIVE bit which is set under the queue
lock, but rather to the wait_queue_head..  I'd love to be corrected.

Comments much appreciated.. this problem has been very elusive to catch.

Benjamin Coddington (1):
  SUNRPC: Fix a race to wake a sync task

 net/sunrpc/sched.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

-- 
2.44.0

[RFC PATCH 1/1] SUNRPC: Fix a race to wake a sync task

[Benjamin Coddington]

We've observed NFS clients with sync tasks sleeping in __rpc_execute
waiting on RPC_TASK_QUEUED that have not responded to a wake-up from
rpc_make_runnable().  I suspect this problem usually goes unnoticed,
because on a busy client the task will eventually be re-awoken by another
task completion or xprt event.  However, if the state manager is draining
the slot table, a sync task missing a wake-up can result in a hung client.

We've been able to prove that the waker in rpc_make_runnable() successfully
calls wake_up_bit() (ie- there's no race to tk_runstate), but the
wake_up_bit() call fails to wake the waiter.  I suspect the waker is
missing the load of the bit's wait_queue_head, so waitqueue_active() is
false.  There are some very helpful comments about this problem above
wake_up_bit(), prepare_to_wait(), and waitqueue_active().

Fix this by inserting smp_mb() before the wake_up_bit(), which pairs with
prepare_to_wait() calling set_current_state().

Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
---
 net/sunrpc/sched.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/sunrpc/sched.c b/net/sunrpc/sched.c
index 6debf4fd42d4..34b31be75497 100644
--- a/net/sunrpc/sched.c
+++ b/net/sunrpc/sched.c
@@ -369,8 +369,11 @@ static void rpc_make_runnable(struct workqueue_struct *wq,
 	if (RPC_IS_ASYNC(task)) {
 		INIT_WORK(&task->u.tk_work, rpc_async_schedule);
 		queue_work(wq, &task->u.tk_work);
-	} else
+	} else {
+		/* paired with set_current_state() in prepare_to_wait */
+		smp_mb();
 		wake_up_bit(&task->tk_runstate, RPC_TASK_QUEUED);
+	}
 }
 
 /*
-- 
2.44.0