public inbox for linux-nfs@vger.kernel.org
 help / color / mirror / Atom feed
From: NeilBrown <neilb@suse.de>
To: Trond Myklebust <trondmy@kernel.org>, Anna Schumaker <anna@kernel.org>
Cc: linux-nfs@vger.kernel.org
Subject: [PATCH 02/11] sunrpc: use clear_and_wake_up_bit() for XPRT_LOCKED.
Date: Fri,  6 Dec 2024 13:15:28 +1100	[thread overview]
Message-ID: <20241206021830.3526922-3-neilb@suse.de> (raw)
In-Reply-To: <20241206021830.3526922-1-neilb@suse.de>

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 --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


  parent reply	other threads:[~2024-12-06  2:19 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-06  2:15 [PATCH 00/11] nfs: improve use of wake_up_bit and wake_up_var NeilBrown
2024-12-06  2:15 ` [PATCH 01/11] sunrpc: remove explicit barrier from rpc_make_runnable() NeilBrown
2024-12-06  2:15 ` NeilBrown [this message]
2024-12-06  2:15 ` [PATCH 03/11] nfs: use clear_and_wake_up_bit() NeilBrown
2024-12-06  2:15 ` [PATCH 04/11] nfs: combine NFS_LAYOUT_RETURN and NFS_LAYOUT_RETURN_LOCK NeilBrown
2025-01-22 20:45   ` Mike Snitzer
2025-01-22 21:25     ` NeilBrown
2024-12-06  2:15 ` [PATCH 05/11] nfs: use clear_and_wake_up_bit() in pnfs code NeilBrown
2024-12-06  2:15 ` [PATCH 06/11] nfs: use store_release_wake_up() for clearing d_fsdata NeilBrown
2024-12-06  2:15 ` [PATCH 07/11] sunrpc: discard rpc_wait_bit_killable() NeilBrown
2024-12-06  2:15 ` [PATCH 08/11] nfs: discard nfs_wait_bit_killable() NeilBrown
2024-12-06  2:15 ` [PATCH 09/11] nfs: add memory barrier before calling wake_up_var on cl_state NeilBrown
2024-12-06  2:15 ` [PATCH 10/11] nfs: use atomic_dec_and_wake_up() NeilBrown
2024-12-06  2:15 ` [PATCH 11/11] nfs: use wait_var_event_spinlock() to wait for nfsi->layout to change NeilBrown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20241206021830.3526922-3-neilb@suse.de \
    --to=neilb@suse.de \
    --cc=anna@kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=trondmy@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox