On Fri, 2023-11-24 at 11:23 +1100, NeilBrown wrote:
> Code like:
>
> WARN_ON(foo())
>
> looks like an assertion and might not be expected to have any side
> effects.
> When testing if a function with side-effects fails a construct like
>
> if (foo())
> WARN_ON(1);
>
> makes the intent more obvious.
>
> nfsd has several WARN_ON calls where the test has side effects, so it
> would be good to change them. These cases don't really need the
> WARN_ON. They have never failed in 8 years of usage so let's just
> remove the WARN_ON wrapper.
>
> Suggested-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
> Signed-off-by: NeilBrown <neilb@xxxxxxx>
> ---
> fs/nfsd/nfs4state.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 042c7a50f425..dd01d0b9e21e 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -1605,7 +1605,7 @@ static void release_open_stateid_locks(struct nfs4_ol_stateid *open_stp,
> while (!list_empty(&open_stp->st_locks)) {
> stp = list_entry(open_stp->st_locks.next,
> struct nfs4_ol_stateid, st_locks);
> - WARN_ON(!unhash_lock_stateid(stp));
> + unhash_lock_stateid(stp);
> put_ol_stateid_locked(stp, reaplist);
> }
> }
> @@ -2234,7 +2234,7 @@ __destroy_client(struct nfs4_client *clp)
> spin_lock(&state_lock);
> while (!list_empty(&clp->cl_delegations)) {
> dp = list_entry(clp->cl_delegations.next, struct nfs4_delegation, dl_perclnt);
> - WARN_ON(!unhash_delegation_locked(dp));
> + unhash_delegation_locked(dp);
> list_add(&dp->dl_recall_lru, &reaplist);
> }
> spin_unlock(&state_lock);
> @@ -6234,7 +6234,7 @@ nfs4_laundromat(struct nfsd_net *nn)
> dp = list_entry (pos, struct nfs4_delegation, dl_recall_lru);
> if (!state_expired(<, dp->dl_time))
> break;
> - WARN_ON(!unhash_delegation_locked(dp));
> + unhash_delegation_locked(dp);
> list_add(&dp->dl_recall_lru, &reaplist);
> }
> spin_unlock(&state_lock);
> @@ -8060,7 +8060,7 @@ nfsd4_release_lockowner(struct svc_rqst *rqstp,
> stp = list_first_entry(&lo->lo_owner.so_stateids,
> struct nfs4_ol_stateid,
> st_perstateowner);
> - WARN_ON(!unhash_lock_stateid(stp));
> + unhash_lock_stateid(stp);
> put_ol_stateid_locked(stp, &reaplist);
> }
> spin_unlock(&clp->cl_lock);
> @@ -8353,7 +8353,7 @@ nfs4_state_shutdown_net(struct net *net)
> spin_lock(&state_lock);
> list_for_each_safe(pos, next, &nn->del_recall_lru) {
> dp = list_entry (pos, struct nfs4_delegation, dl_recall_lru);
> - WARN_ON(!unhash_delegation_locked(dp));
> + unhash_delegation_locked(dp);
> list_add(&dp->dl_recall_lru, &reaplist);
> }
> spin_unlock(&state_lock);
Reviewed-by: Jeff Layton <jlayton@xxxxxxxxxx>
