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

[Dai Ngo <dai.ngo@oracle.com>]

On 12/15/25 10:58 AM, Chuck Lever wrote:
>
> 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.

Fix in v2.

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

Fix in v2.

>
> 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.

Fix in v2.

Thanks,
-Dai

>
>
>>   /* 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