Re: [PATCH 12/13] nfsd: allow delegation state ids to be revoked and then freed

[Jeff Layton <jlayton@kernel.org>]

On Mon, 2024-01-29 at 14:29 +1100, NeilBrown wrote:
> Revoking state through 'unlock_filesystem' now revokes any delegation
> states found.  When the stateids are then freed by the client, the
> revoked stateids will be cleaned up correctly.
> 
> As there is already support for revoking delegations, we build on that
> for admin-revoking.
> 
> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
>  fs/nfsd/nfs4state.c | 28 +++++++++++++++++++++++++---
>  1 file changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 5dc8f60e18dc..e749d5c0e23a 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -1335,9 +1335,12 @@ unhash_delegation_locked(struct nfs4_delegation *dp, unsigned short statusmask)
>  	if (!delegation_hashed(dp))
>  		return false;
>  
> -	if (dp->dl_stid.sc_client->cl_minorversion == 0)
> +	if (statusmask == SC_STATUS_REVOKED &&
> +	    dp->dl_stid.sc_client->cl_minorversion == 0)
>  		statusmask = SC_STATUS_CLOSED;
>  	dp->dl_stid.sc_status |= statusmask;
> +	if (statusmask & SC_STATUS_ADMIN_REVOKED)
> +		atomic_inc(&dp->dl_stid.sc_client->cl_admin_revoked);
>  
>  	/* Ensure that deleg break won't try to requeue it */
>  	++dp->dl_time;
> @@ -1368,7 +1371,8 @@ static void revoke_delegation(struct nfs4_delegation *dp)
>  
>  	trace_nfsd_stid_revoke(&dp->dl_stid);
>  
> -	if (dp->dl_stid.sc_status & SC_STATUS_REVOKED) {
> +	if (dp->dl_stid.sc_status &
> +	    (SC_STATUS_REVOKED | SC_STATUS_ADMIN_REVOKED)) {
>  		spin_lock(&clp->cl_lock);
>  		refcount_inc(&dp->dl_stid.sc_count);
>  		list_add(&dp->dl_recall_lru, &clp->cl_revoked);
> @@ -1717,7 +1721,7 @@ void nfsd4_revoke_states(struct net *net, struct super_block *sb)
>  	unsigned int idhashval;
>  	unsigned int sc_types;
>  
> -	sc_types = SC_TYPE_OPEN | SC_TYPE_LOCK;
> +	sc_types = SC_TYPE_OPEN | SC_TYPE_LOCK | SC_TYPE_DELEG;
>  
>  	spin_lock(&nn->client_lock);
>  	for (idhashval = 0; idhashval < CLIENT_HASH_MASK; idhashval++) {
> @@ -1729,6 +1733,7 @@ void nfsd4_revoke_states(struct net *net, struct super_block *sb)
>  								  sc_types);
>  			if (stid) {
>  				struct nfs4_ol_stateid *stp;
> +				struct nfs4_delegation *dp;
>  
>  				spin_unlock(&nn->client_lock);
>  				switch (stid->sc_type) {
> @@ -1774,6 +1779,16 @@ void nfsd4_revoke_states(struct net *net, struct super_block *sb)
>  						spin_unlock(&clp->cl_lock);
>  					mutex_unlock(&stp->st_mutex);
>  					break;
> +				case SC_TYPE_DELEG:
> +					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;
>  				}
>  				nfs4_put_stid(stid);
>  				spin_lock(&nn->client_lock);
> @@ -4675,6 +4690,7 @@ static void nfsd4_drop_revoked_stid(struct nfs4_stid *s)
>  	struct nfs4_client *cl = s->sc_client;
>  	LIST_HEAD(reaplist);
>  	struct nfs4_ol_stateid *stp;
> +	struct nfs4_delegation *dp;
>  	bool unhashed;
>  
>  	switch (s->sc_type) {
> @@ -4692,6 +4708,12 @@ static void nfsd4_drop_revoked_stid(struct nfs4_stid *s)
>  		if (unhashed)
>  			nfs4_put_stid(s);
>  		break;
> +	case SC_TYPE_DELEG:
> +		dp = delegstateid(s);
> +		list_del_init(&dp->dl_recall_lru);
> +		spin_unlock(&cl->cl_lock);
> +		nfs4_put_stid(s);
> +		break;
>  	default:
>  		spin_unlock(&cl->cl_lock);
>  	}

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