On Fri, 2024-12-06 at 13:55 +1100, NeilBrown wrote:
> The wake_up_var interface is fragile as barriers are sometimes needed.
> There are now new interfaces so that most wake-ups can use an interface
> that is guaranteed to have all barriers needed.
>
> This patch changes the wake up on cl_cb_inflight to use
> atomic_dec_and_wake_up().
>
> It also changes the wake up on rp_locked to use store_release_wake_up().
> This involves changing rp_locked from atomic_t to int.
>
> Signed-off-by: NeilBrown <neilb@xxxxxxx>
> ---
> fs/nfsd/nfs4callback.c | 3 +--
> fs/nfsd/nfs4state.c | 16 ++++++----------
> fs/nfsd/state.h | 2 +-
> 3 files changed, 8 insertions(+), 13 deletions(-)
>
> diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
> index 3877b53e429f..a8dc9de2f7fb 100644
> --- a/fs/nfsd/nfs4callback.c
> +++ b/fs/nfsd/nfs4callback.c
> @@ -1036,8 +1036,7 @@ static void nfsd41_cb_inflight_begin(struct nfs4_client *clp)
> static void nfsd41_cb_inflight_end(struct nfs4_client *clp)
> {
>
> - if (atomic_dec_and_test(&clp->cl_cb_inflight))
> - wake_up_var(&clp->cl_cb_inflight);
> + atomic_dec_and_wake_up(&clp->cl_cb_inflight);
> }
>
> static void nfsd41_cb_inflight_wait_complete(struct nfs4_client *clp)
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 741b9449f727..9fbf7c8f0a3e 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -4739,7 +4739,7 @@ static void init_nfs4_replay(struct nfs4_replay *rp)
> rp->rp_status = nfserr_serverfault;
> rp->rp_buflen = 0;
> rp->rp_buf = rp->rp_ibuf;
> - atomic_set(&rp->rp_locked, RP_UNLOCKED);
> + rp->rp_locked = RP_UNLOCKED;
> }
>
> static int nfsd4_cstate_assign_replay(struct nfsd4_compound_state *cstate,
> @@ -4747,9 +4747,9 @@ static int nfsd4_cstate_assign_replay(struct nfsd4_compound_state *cstate,
> {
> if (!nfsd4_has_session(cstate)) {
> wait_var_event(&so->so_replay.rp_locked,
> - atomic_cmpxchg(&so->so_replay.rp_locked,
> - RP_UNLOCKED, RP_LOCKED) != RP_LOCKED);
> - if (atomic_read(&so->so_replay.rp_locked) == RP_UNHASHED)
> + cmpxchg(&so->so_replay.rp_locked,
> + RP_UNLOCKED, RP_LOCKED) != RP_LOCKED);
nit: try_cmpxchg() generates more efficient assembly. Can we switch to
that here too?
> + if (so->so_replay.rp_locked == RP_UNHASHED)
> return -EAGAIN;
> cstate->replay_owner = nfs4_get_stateowner(so);
> }
> @@ -4762,9 +4762,7 @@ void nfsd4_cstate_clear_replay(struct nfsd4_compound_state *cstate)
>
> if (so != NULL) {
> cstate->replay_owner = NULL;
> - atomic_set(&so->so_replay.rp_locked, RP_UNLOCKED);
> - smp_mb__after_atomic();
> - wake_up_var(&so->so_replay.rp_locked);
> + store_release_wake_up(&so->so_replay.rp_locked, RP_UNLOCKED);
> nfs4_put_stateowner(so);
> }
> }
> @@ -5069,9 +5067,7 @@ move_to_close_lru(struct nfs4_ol_stateid *s, struct net *net)
> * Some threads with a reference might be waiting for rp_locked,
> * so tell them to stop waiting.
> */
> - atomic_set(&oo->oo_owner.so_replay.rp_locked, RP_UNHASHED);
> - smp_mb__after_atomic();
> - wake_up_var(&oo->oo_owner.so_replay.rp_locked);
> + store_release_wake_up(&oo->oo_owner.so_replay.rp_locked, RP_UNHASHED);
> wait_event(close_wq, refcount_read(&s->st_stid.sc_count) == 2);
>
> release_all_access(s);
> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> index e16bb3717fb9..ba30b2335b66 100644
> --- a/fs/nfsd/state.h
> +++ b/fs/nfsd/state.h
> @@ -505,7 +505,7 @@ struct nfs4_replay {
> unsigned int rp_buflen;
> char *rp_buf;
> struct knfsd_fh rp_openfh;
> - atomic_t rp_locked;
> + int rp_locked;
> char rp_ibuf[NFSD4_REPLAY_ISIZE];
> };
>
Looks good otherwise.
Reviewed-by: Jeff Layton <jlayton@xxxxxxxxxx>
