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@xxxxxxx>
---
net/sunrpc/xprt.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
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);
--
2.47.0
