Re: [PATCH 1/3] NFSD: Move clientid_hashval and same_clid to header files

["Chuck Lever" <cel@kernel.org>]

On Mon, Dec 15, 2025, at 1:13 PM, Dai Ngo wrote:
> As preparation for introducing infrastructure to track SCSI fencing events,
> relocate the clientid_hashval function from nfs4state.c to nfsd.h and the
> same_clid function from nfs4state.c to state.h.
>
> No functional changes are introduced in this commit—only movement of code
> to improve accessibility for forthcoming enhancements.
>
> Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
> ---
>  fs/nfsd/nfs4state.c | 15 ---------------
>  fs/nfsd/nfsd.h      |  5 +++++
>  fs/nfsd/state.h     |  5 +++++
>  3 files changed, 10 insertions(+), 15 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 35004568d43e..13b4dc89b1e8 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -1423,15 +1423,6 @@ static void revoke_delegation(struct nfs4_delegation *dp)
>  	destroy_unhashed_deleg(dp);
>  }
> 
> -/*
> - * SETCLIENTID state
> - */
> -
> -static unsigned int clientid_hashval(u32 id)
> -{
> -	return id & CLIENT_HASH_MASK;
> -}
> -
>  static unsigned int clientstr_hashval(struct xdr_netobj name)
>  {
>  	return opaque_hashval(name.data, 8) & CLIENT_HASH_MASK;
> @@ -2621,12 +2612,6 @@ same_verf(nfs4_verifier *v1, nfs4_verifier *v2)
>  	return 0 == memcmp(v1->data, v2->data, sizeof(v1->data));
>  }
> 
> -static int
> -same_clid(clientid_t *cl1, clientid_t *cl2)
> -{
> -	return (cl1->cl_boot == cl2->cl_boot) && (cl1->cl_id == cl2->cl_id);
> -}
> -
>  static bool groups_equal(struct group_info *g1, struct group_info *g2)
>  {
>  	int i;
> diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
> index 50be785f1d2c..369efe6ed665 100644
> --- a/fs/nfsd/nfsd.h
> +++ b/fs/nfsd/nfsd.h
> @@ -510,6 +510,11 @@ static inline bool nfsd_attrs_supported(u32 
> minorversion, const u32 *bmval)
>  	return bmval_is_subset(bmval, nfsd_suppattrs[minorversion]);
>  }
> 
> +static inline unsigned int clientid_hashval(u32 id)
> +{
> +	return id & CLIENT_HASH_MASK;
> +}
> +

I can't comment on the overall purpose of this series yet, but
there are one or two mechanical issues that I see already.

Let's not add NFSv4- or pNFS-specific functions to fs/nfsd/nfsd.h.
Same comment applies to the function declarations this series moves
in a subsequent patch.

Why can't clientid_hashval() go into fs/nfsd/state.h instead?

Also, when a function becomes accessible outside of one source
file (like a "static inline" function or a callback function),
it needs to get a kdoc comment that documents its API contract.


>  /* These will return ERR_INVAL if specified in GETATTR or READDIR. */
>  #define NFSD_WRITEONLY_ATTRS_WORD1 \
>  	(FATTR4_WORD1_TIME_ACCESS_SET   | FATTR4_WORD1_TIME_MODIFY_SET)
> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> index 1e736f402426..b737e8cfe6d5 100644
> --- a/fs/nfsd/state.h
> +++ b/fs/nfsd/state.h
> @@ -865,6 +865,11 @@ static inline bool try_to_expire_client(struct 
> nfs4_client *clp)
>  	return clp->cl_state == NFSD4_EXPIRABLE;
>  }
> 
> +static inline int same_clid(clientid_t *cl1, clientid_t *cl2)
> +{
> +	return (cl1->cl_boot == cl2->cl_boot) && (cl1->cl_id == cl2->cl_id);
> +}
> +
>  extern __be32 nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp,
>  		struct dentry *dentry, struct nfs4_delegation **pdp);
>  #endif   /* NFSD4_STATE_H */
> -- 
> 2.47.3

-- 
Chuck Lever