Re: git: c52bcd09c2a6 - main - nfsd: Garbage collect stray NFSv4 state

On 7/11/26 20:58, A FreeBSD User wrote:
> Am Tage des Herren Sat, 11 Jul 2026 15:51:36 +0000
> Rick Macklem <rmacklem@FreeBSD.org> schrieb:
> 
>> The branch main has been updated by rmacklem:
>>
>> URL: https://cgit.FreeBSD.org/src/commit/?id=c52bcd09c2a6736fe841fd72e3cfb74de5a35b03
>>
>> commit c52bcd09c2a6736fe841fd72e3cfb74de5a35b03
>> Author:     Rick Macklem <rmacklem@FreeBSD.org>
>> AuthorDate: 2026-07-11 15:49:12 +0000
>> Commit:     Rick Macklem <rmacklem@FreeBSD.org>
>> CommitDate: 2026-07-11 15:49:12 +0000
>>
>>      nfsd: Garbage collect stray NFSv4 state
>>      
>>      When a file is deleted on the NFS server by another client,
>>      any NFSv4 state related to that file is left stranded.
>>      This happens because the NFSv4 operations that free the
>>      state use a CFH, which is set by a PutFH operation.
>>      However, the PutFH fails with ESTALE because the file has
>>      been deleted.
>>      
>>      This patch adds a function called nfsrv_freestrandedstate()
>>      that frees all the NFSv4 state related to a file and calls
>>      this function when PutFH will be replying ESTALE.
>>      
>>      While here, a helper function was defined to handle free'ng
>>      of the nfslockfile structure and replaces the two places
>>      where nearly identical code does this.
>>      
>>      Reported by:    Richard Purdie <richard.purdie@linuxfoundation.org>
>>      Tested by:      Michael Halstead <mhalstead@linuxfoundation.org>
>>      MFC after:      2 weeks
>> ---
>>   sys/fs/nfsserver/nfs_nfsdstate.c | 89 +++++++++++++++++++++++++++++++++-------
>>   1 file changed, 75 insertions(+), 14 deletions(-)
>>
>> diff --git a/sys/fs/nfsserver/nfs_nfsdstate.c b/sys/fs/nfsserver/nfs_nfsdstate.c
>> index 102af9bb2951..d790ab974d07 100644
>> --- a/sys/fs/nfsserver/nfs_nfsdstate.c
>> +++ b/sys/fs/nfsserver/nfs_nfsdstate.c
>> @@ -253,6 +253,7 @@ static void nfsrv_issuedelegation(struct vnode *vp, struct nfsclient
>> *clp, nfsv4stateid_t *delegstateidp);
>>   static void nfsrv_clientlock(bool mlocked);
>>   static void nfsrv_clientunlock(bool mlocked);
>> +static void nfsrv_freelockifnotinuse(struct nfslockfile *lfp);
>>   
>>   /*
>>    * Lock the client structure, either with the mutex or the exclusive nfsd lock.
>> @@ -1537,20 +1538,13 @@ nfsrv_freedeleglist(struct nfsstatehead *sthp)
>>   static void
>>   nfsrv_freedeleg(struct nfsstate *stp)
>>   {
>> -	struct nfslockfile *lfp;
>>   
>>   	LIST_REMOVE(stp, ls_hash);
>>   	LIST_REMOVE(stp, ls_list);
>>   	LIST_REMOVE(stp, ls_file);
>>   	if ((stp->ls_flags & NFSLCK_DELEGWRITE) != 0)
>>   		nfsrv_writedelegcnt--;
>> -	lfp = stp->ls_lfp;
>> -	if (LIST_EMPTY(&lfp->lf_open) &&
>> -	    LIST_EMPTY(&lfp->lf_lock) && LIST_EMPTY(&lfp->lf_deleg) &&
>> -	    LIST_EMPTY(&lfp->lf_locallock) && LIST_EMPTY(&lfp->lf_rollback) &&
>> -	    lfp->lf_usecount == 0 &&
>> -	    nfsv4_testlock(&lfp->lf_locallock_lck) == 0)
>> -		nfsrv_freenfslockfile(lfp);
>> +	nfsrv_freelockifnotinuse(stp->ls_lfp);
>>   	free(stp, M_NFSDSTATE);
>>   	VNET(nfsstatsv1_p)->srvdelegates--;
>>   	nfsrv_openpluslock--;
>> @@ -1632,12 +1626,7 @@ nfsrv_freeopen(struct nfsstate *stp, vnode_t vp, int cansleep,
>> NFSPROC_T *p)
>>   	 * If there are locks associated with the open, the
>>   	 * nfslockfile structure can be freed via nfsrv_freelockowner().
>>   	 */
>> -	if (lfp != NULL && LIST_EMPTY(&lfp->lf_open) &&
>> -	    LIST_EMPTY(&lfp->lf_deleg) && LIST_EMPTY(&lfp->lf_lock) &&
>> -	    LIST_EMPTY(&lfp->lf_locallock) && LIST_EMPTY(&lfp->lf_rollback) &&
>> -	    lfp->lf_usecount == 0 &&
>> -	    nfsv4_testlock(&lfp->lf_locallock_lck) == 0)
>> -		nfsrv_freenfslockfile(lfp);
>> +	nfsrv_freelockifnotinuse(lfp);
>>   	free(stp, M_NFSDSTATE);
>>   	VNET(nfsstatsv1_p)->srvopens--;
>>   	nfsrv_openpluslock--;
>> @@ -8881,3 +8870,75 @@ nfsrv_removedeleg(fhandle_t *fhp, struct nfsrv_descript *nd,
>> NFSPROC_T *p) }
>>   	NFSUNLOCKSTATE();
>>   }
>> +
>> +/*
>> + * Free the nfslockfile structure if not in use.
>> + */
>> +static void
>> +nfsrv_freelockifnotinuse(struct nfslockfile *lfp)
>> +{
>> +
>> +	/*
>> +	 * The nfslockfile is freed here if there are no locks
>> +	 * associated with the open.
>> +	 * If there are locks associated with the open, the
>> +	 * nfslockfile structure can be freed via nfsrv_freelockowner().
>> +	 */
>> +	if (lfp != NULL && LIST_EMPTY(&lfp->lf_open) &&
>> +	    LIST_EMPTY(&lfp->lf_deleg) && LIST_EMPTY(&lfp->lf_lock) &&
>> +	    LIST_EMPTY(&lfp->lf_locallock) && LIST_EMPTY(&lfp->lf_rollback) &&
>> +	    lfp->lf_usecount == 0 &&
>> +	    nfsv4_testlock(&lfp->lf_locallock_lck) == 0)
>> +		nfsrv_freenfslockfile(lfp);
>> +}
>> +
>> +/*
>> + * Free stranded open/lock/delegation/layouts.
>> + * (These become stranded if the file has been deleted.)
>> + */
>> +void
>> +nfsrv_freestrandedstate(struct nfsrvfh *nfp)
>> +{
>> +	struct nfslockfile *lfp;
>> +	struct nfsstate *stp, *nstp;
>> +	struct nfslayouthash *lhyp;
>> +	struct nfslayout *lyp, *nlyp;
>> +	fhandle_t *fhp;
>> +
>> +	if (nfp->nfsrvfh_len != NFSX_MYFH)
>> +		return;
>> +	fhp = (fhandle_t *)nfp->nfsrvfh_data;
>> +	NFSLOCKSTATE();
>> +	if (nfsrv_getlockfile(0, NULL, &lfp, fhp, 0) < 0) {
>> +		NFSUNLOCKSTATE();
>> +		return;
>> +	}
>> +	lfp->lf_usecount++;	/* So nfsrv_freeopen() does not free it. */
>> +	/* Note that nfsrv_freeopen() will also free the byte range locks. */
>> +	LIST_FOREACH_SAFE(stp, &lfp->lf_open, ls_file, nstp)
>> +		nfsrv_freeopen(stp, NULL, 0, curthread);
>> +
>> +	/*
>> +	 * Normally, a delegation will have been recalled when the file is
>> +	 * removed.  However, get rid of any that have somehow been
>> +	 * left stranded.
>> +	 */
>> +	LIST_FOREACH_SAFE(stp, &lfp->lf_deleg, ls_file, nstp)
>> +		nfsrv_freedeleg(stp);
>> +
>> +	/* Get rid of the nfslockfile, if no longer in use. */
>> +	lfp->lf_usecount--;
>> +	nfsrv_freelockifnotinuse(lfp);
>> +	NFSUNLOCKSTATE();
>> +
>> +	/* Free any layouts for the pNFS server case. */
>> +	if (nfsrv_devidcnt == 0)
>> +		return;
>> +	lhyp = NFSLAYOUTHASH(fhp);
>> +	NFSLOCKLAYOUT(lhyp);
>> +	TAILQ_FOREACH_SAFE(lyp, &lhyp->list, lay_list, nlyp) {
>> +		if (NFSBCMP(&lyp->lay_fh, fhp, sizeof(*fhp)) == 0)
>> +			nfsrv_freelayout(&lhyp->list, lyp);
>> +	}
>> +	NFSUNLOCKLAYOUT(lhyp);
>> +}
>>
> 
> It seems this commit breaks buildkernel for me:
> [...]
> /usr/src/sys/fs/nfsserver/nfs_nfsdstate.c:8900:1: error: no previous prototype for function
> 'nfsrv_freestrandedstate' [-Werror,-Wmissing-prototypes] 8900 | nfsrv_freestrandedstate(struct
> nfsrvfh *nfp

+1
Same here

> 
> Kind regards,
> 
> oh

-- 
Pouria