diff mbox series

[02/11] sunrpc: use clear_and_wake_up_bit() for XPRT_LOCKED.

Message ID 20241206021830.3526922-3-neilb@suse.de (mailing list archive)
State New
Headers show
Series nfs: improve use of wake_up_bit and wake_up_var | expand

Commit Message

NeilBrown Dec. 6, 2024, 2:15 a.m. UTC
wake_up_bit() requires a full memory barrier between the bit being
cleared and wake_up_bit() being called, else a race can result in wake
up not being sent despite another task preparing to wait.

Some paths between the clear_bit and the wake_up_bit do not have a full
barrier, such as when xprt_reserve_xprt() finds that XPRT_WRITE_SPACE is
set and clears XPRT_LOCKED before returning to xprt_release_write and
thence xprt_autoclose().

This doesn't appear to be a problem in practice as no failure reports
are known, but it seems prudent to send the wakeup immediately after the
bit is cleared and to use clear_and_wake_up_bit() which includes the
required barriers.

In most cases, if nothing is waiting for the bit the waitqueue_active()
test in wake_up_bit() will mean this does minimal extra work - though as
the waitqueue can be shared, this is not guaranteed.

Using clear_and_wake_up_bit() makes the code "obviously correct".

Signed-off-by: NeilBrown <neilb@suse.de>
---
 net/sunrpc/xprt.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)
diff mbox series

Patch

diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
index 09f245cda526..40385362e982 100644
--- a/net/sunrpc/xprt.c
+++ b/net/sunrpc/xprt.c
@@ -248,7 +248,7 @@  static void xprt_clear_locked(struct rpc_xprt *xprt)
 {
 	xprt->snd_task = NULL;
 	if (!test_bit(XPRT_CLOSE_WAIT, &xprt->state))
-		clear_bit_unlock(XPRT_LOCKED, &xprt->state);
+		clear_and_wake_up_bit(XPRT_LOCKED, &xprt->state);
 	else
 		queue_work(xprtiod_workqueue, &xprt->task_cleanup);
 }
@@ -744,7 +744,6 @@  static void xprt_autoclose(struct work_struct *work)
 	clear_bit(XPRT_CLOSE_WAIT, &xprt->state);
 	xprt->ops->close(xprt);
 	xprt_release_write(xprt, NULL);
-	wake_up_bit(&xprt->state, XPRT_LOCKED);
 	memalloc_nofs_restore(pflags);
 }
 
@@ -911,7 +910,6 @@  void xprt_unlock_connect(struct rpc_xprt *xprt, void *cookie)
 	xprt_schedule_autodisconnect(xprt);
 out:
 	spin_unlock(&xprt->transport_lock);
-	wake_up_bit(&xprt->state, XPRT_LOCKED);
 }
 EXPORT_SYMBOL_GPL(xprt_unlock_connect);