[PATCH 0/2] nfsd: use new wake_up_var interface

[PATCH 0/2] nfsd: use new wake_up_var interface

[NeilBrown]

wake_up_var() in a fragile interface as it sometimes needs a memory
barrier before it.  Recently new interfaces were added which include all
required barriers.

These patches update nfsd and svc parrts of sunrpc to use new interfaces
where appropriate.

NeilBrown

[PATCH 1/2] nfsd: use new wake_up_var interfaces.

[NeilBrown]

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@suse.de>
---
 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);
+		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];
 };
 
-- 
2.47.0

Re: [PATCH 1/2] nfsd: use new wake_up_var interfaces.

[Jeff Layton]

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@suse.de>
> ---
>  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@kernel.org>

Re: [PATCH 1/2] nfsd: use new wake_up_var interfaces.

[NeilBrown]

On Fri, 06 Dec 2024, Jeff Layton wrote:
> 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@suse.de>
> > ---
> >  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?

Does it?  try_cmpxchg() makes loops smaller (as described in
atomic_t.txt).  I think it wins when the "old" value has to be updated
each time around the loop.  In this case the "old" value is always the
same.


NeilBrown


> 
> > +		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@kernel.org>
> 

Re: [PATCH 1/2] nfsd: use new wake_up_var interfaces.

[Jeff Layton]

On Fri, 2024-12-06 at 17:10 +1100, NeilBrown wrote:
> On Fri, 06 Dec 2024, Jeff Layton wrote:
> > 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@suse.de>
> > > ---
> > >  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?
> 
> Does it?  try_cmpxchg() makes loops smaller (as described in
> atomic_t.txt).  I think it wins when the "old" value has to be updated
> each time around the loop.  In this case the "old" value is always the
> same.
> 
> 

In most cases, it does, because we have to return "old" in the case of
the traditional cmpxchg() operation. From atomic_t.txt:

  int atomic_cmpxchg(atomic_t *ptr, int old, int new)
  {
    (void)atomic_try_cmpxchg(ptr, &old, new);
    return old;
  }

That said, in this case it my not be a win. You need something to
return that value anyway, so it can properly act as a wait_var_event()
condition.

> > 
> > > +		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@kernel.org>
> > 
> 

-- 
Jeff Layton <jlayton@kernel.org>

[PATCH 2/2] sunrpc/svc: use store_release_wake_up()

[NeilBrown]

svc_thread_init_status() contains an open-coded
store_release_wake_up().  It is cleaner to use that function directly
rather than needing to remember the barrier.

Signed-off-by: NeilBrown <neilb@suse.de>
---
 include/linux/sunrpc/svc.h | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
index e68fecf6eab5..e4f09f58d58c 100644
--- a/include/linux/sunrpc/svc.h
+++ b/include/linux/sunrpc/svc.h
@@ -327,12 +327,7 @@ static inline bool svc_thread_should_stop(struct svc_rqst *rqstp)
  */
 static inline void svc_thread_init_status(struct svc_rqst *rqstp, int err)
 {
-	rqstp->rq_err = err;
-	/* memory barrier ensures assignment to error above is visible before
-	 * waitqueue_active() test below completes.
-	 */
-	smp_mb();
-	wake_up_var(&rqstp->rq_err);
+	store_release_wake_up(&rqstp->rq_err, err);
 	if (err)
 		kthread_exit(1);
 }
-- 
2.47.0

Re: [PATCH 2/2] sunrpc/svc: use store_release_wake_up()

[Jeff Layton]

On Fri, 2024-12-06 at 13:55 +1100, NeilBrown wrote:
> svc_thread_init_status() contains an open-coded
> store_release_wake_up().  It is cleaner to use that function directly
> rather than needing to remember the barrier.
> 
> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
>  include/linux/sunrpc/svc.h | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
> index e68fecf6eab5..e4f09f58d58c 100644
> --- a/include/linux/sunrpc/svc.h
> +++ b/include/linux/sunrpc/svc.h
> @@ -327,12 +327,7 @@ static inline bool svc_thread_should_stop(struct svc_rqst *rqstp)
>   */
>  static inline void svc_thread_init_status(struct svc_rqst *rqstp, int err)
>  {
> -	rqstp->rq_err = err;
> -	/* memory barrier ensures assignment to error above is visible before
> -	 * waitqueue_active() test below completes.
> -	 */
> -	smp_mb();
> -	wake_up_var(&rqstp->rq_err);
> +	store_release_wake_up(&rqstp->rq_err, err);
>  	if (err)
>  		kthread_exit(1);
>  }

Reviewed-by: Jeff Layton <jlayton@kernel.org>

Re: [PATCH 0/2] nfsd: use new wake_up_var interface

[cel]

From: Chuck Lever <chuck.lever@oracle.com>

On Fri, 06 Dec 2024 13:55:51 +1100, NeilBrown wrote:
> wake_up_var() in a fragile interface as it sometimes needs a memory
> barrier before it.  Recently new interfaces were added which include all
> required barriers.
> 
> These patches update nfsd and svc parrts of sunrpc to use new interfaces
> where appropriate.
> 
> [...]

Applied to nfsd-testing for v6.14, thanks!

[1/2] nfsd: use new wake_up_var interfaces.
      commit: 70b79974d849d8ff1f60295c03600f9c9db4da05
[2/2] sunrpc/svc: use store_release_wake_up()
      commit: 101543f023804bc0e62a2fdb15e143c022fedc7c

--
Chuck Lever