Re: [PATCH v4 1/6] NFSD: Extract revoke_one_stid() utility function

[Jeff Layton <jlayton@kernel.org>]

On Wed, 2026-03-18 at 10:15 -0400, Chuck Lever wrote:
> From: Chuck Lever <chuck.lever@oracle.com>
> 
> The per-stateid revocation logic in nfsd4_revoke_states() handles
> four stateid types in a deeply nested switch. Extract two helpers:
> 
> revoke_ol_stid() performs admin-revocation of an open or lock
> stateid with st_mutex already held: marks the stateid as
> SC_STATUS_ADMIN_REVOKED, closes POSIX locks for lock stateids,
> and releases file access.
> 
> revoke_one_stid() dispatches by sc_type, acquires st_mutex with
> the appropriate lockdep class for open and lock stateids, and
> handles delegation unhash and layout close inline.
> 
> No functional change. Preparation for adding export-scoped state
> revocation which reuses revoke_one_stid().
> 
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  fs/nfsd/nfs4state.c | 126 ++++++++++++++++++++++++++--------------------------
>  1 file changed, 63 insertions(+), 63 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 6b9c399b89df..d476192e4b27 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -1757,6 +1757,68 @@ static struct nfs4_stid *find_one_sb_stid(struct nfs4_client *clp,
>  	return stid;
>  }
>  
> +static void revoke_ol_stid(struct nfs4_client *clp,
> +			   struct nfs4_ol_stateid *stp)
> +{
> +	struct nfs4_stid *stid = &stp->st_stid;
> +
> +	lockdep_assert_held(&stp->st_mutex);
> +	spin_lock(&clp->cl_lock);
> +	if (stid->sc_status == 0) {
> +		stid->sc_status |= SC_STATUS_ADMIN_REVOKED;
> +		atomic_inc(&clp->cl_admin_revoked);
> +		spin_unlock(&clp->cl_lock);
> +		if (stid->sc_type == SC_TYPE_LOCK) {
> +			struct nfs4_lockowner *lo =
> +				lockowner(stp->st_stateowner);
> +			struct nfsd_file *nf;
> +
> +			nf = find_any_file(stp->st_stid.sc_file);
> +			if (nf) {
> +				get_file(nf->nf_file);
> +				filp_close(nf->nf_file, (fl_owner_t)lo);
> +				nfsd_file_put(nf);
> +			}
> +		}
> +		release_all_access(stp);
> +	} else
> +		spin_unlock(&clp->cl_lock);
> +}
> +
> +static void revoke_one_stid(struct nfs4_client *clp, struct nfs4_stid *stid)
> +{
> +	struct nfs4_ol_stateid *stp;
> +	struct nfs4_delegation *dp;
> +
> +	switch (stid->sc_type) {
> +	case SC_TYPE_OPEN:
> +		stp = openlockstateid(stid);
> +		mutex_lock_nested(&stp->st_mutex, OPEN_STATEID_MUTEX);
> +		revoke_ol_stid(clp, stp);
> +		mutex_unlock(&stp->st_mutex);
> +		break;
> +	case SC_TYPE_LOCK:
> +		stp = openlockstateid(stid);
> +		mutex_lock_nested(&stp->st_mutex, LOCK_STATEID_MUTEX);
> +		revoke_ol_stid(clp, stp);
> +		mutex_unlock(&stp->st_mutex);
> +		break;
> +	case SC_TYPE_DELEG:
> +		refcount_inc(&stid->sc_count);
> +		dp = delegstateid(stid);
> +		spin_lock(&state_lock);
> +		if (!unhash_delegation_locked(dp, SC_STATUS_ADMIN_REVOKED))
> +			dp = NULL;
> +		spin_unlock(&state_lock);
> +		if (dp)
> +			revoke_delegation(dp);
> +		break;
> +	case SC_TYPE_LAYOUT:
> +		nfsd4_close_layout(layoutstateid(stid));
> +		break;
> +	}
> +}
> +
>  /**
>   * nfsd4_revoke_states - revoke all nfsv4 states associated with given filesystem
>   * @nn:   used to identify instance of nfsd (there is one per net namespace)
> @@ -1787,70 +1849,8 @@ void nfsd4_revoke_states(struct nfsd_net *nn, struct super_block *sb)
>  			struct nfs4_stid *stid = find_one_sb_stid(clp, sb,
>  								  sc_types);
>  			if (stid) {
> -				struct nfs4_ol_stateid *stp;
> -				struct nfs4_delegation *dp;
> -				struct nfs4_layout_stateid *ls;
> -
>  				spin_unlock(&nn->client_lock);
> -				switch (stid->sc_type) {
> -				case SC_TYPE_OPEN:
> -					stp = openlockstateid(stid);
> -					mutex_lock_nested(&stp->st_mutex,
> -							  OPEN_STATEID_MUTEX);
> -
> -					spin_lock(&clp->cl_lock);
> -					if (stid->sc_status == 0) {
> -						stid->sc_status |=
> -							SC_STATUS_ADMIN_REVOKED;
> -						atomic_inc(&clp->cl_admin_revoked);
> -						spin_unlock(&clp->cl_lock);
> -						release_all_access(stp);
> -					} else
> -						spin_unlock(&clp->cl_lock);
> -					mutex_unlock(&stp->st_mutex);
> -					break;
> -				case SC_TYPE_LOCK:
> -					stp = openlockstateid(stid);
> -					mutex_lock_nested(&stp->st_mutex,
> -							  LOCK_STATEID_MUTEX);
> -					spin_lock(&clp->cl_lock);
> -					if (stid->sc_status == 0) {
> -						struct nfs4_lockowner *lo =
> -							lockowner(stp->st_stateowner);
> -						struct nfsd_file *nf;
> -
> -						stid->sc_status |=
> -							SC_STATUS_ADMIN_REVOKED;
> -						atomic_inc(&clp->cl_admin_revoked);
> -						spin_unlock(&clp->cl_lock);
> -						nf = find_any_file(stp->st_stid.sc_file);
> -						if (nf) {
> -							get_file(nf->nf_file);
> -							filp_close(nf->nf_file,
> -								   (fl_owner_t)lo);
> -							nfsd_file_put(nf);
> -						}
> -						release_all_access(stp);
> -					} else
> -						spin_unlock(&clp->cl_lock);
> -					mutex_unlock(&stp->st_mutex);
> -					break;
> -				case SC_TYPE_DELEG:
> -					refcount_inc(&stid->sc_count);
> -					dp = delegstateid(stid);
> -					spin_lock(&state_lock);
> -					if (!unhash_delegation_locked(
> -						    dp, SC_STATUS_ADMIN_REVOKED))
> -						dp = NULL;
> -					spin_unlock(&state_lock);
> -					if (dp)
> -						revoke_delegation(dp);
> -					break;
> -				case SC_TYPE_LAYOUT:
> -					ls = layoutstateid(stid);
> -					nfsd4_close_layout(ls);
> -					break;
> -				}
> +				revoke_one_stid(clp, stid);
>  				nfs4_put_stid(stid);
>  				spin_lock(&nn->client_lock);
>  				if (clp->cl_minorversion == 0)

Much nicer anyway.

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