Re: git: c52bcd09c2a6 - main - nfsd: Garbage collect stray NFSv4 state
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Sat, 11 Jul 2026 17:46:33 UTC
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