Re: [PATCH v5 05/21] nfsd: update the fsnotify mark when setting or removing a dir delegation

[Jeff Layton]

On Mon, 2026-06-08 at 12:38 -0400, Chuck Lever wrote:
> 
> On Fri, May 22, 2026, at 3:42 PM, Jeff Layton wrote:
> > Add a new helper function that will update the mask on the nfsd_file's
> > fsnotify_mark to be a union of all current directory delegations on an
> > inode. Call that when directory delegations are added or removed.
> 
> This commit message repeats what the diff below says. Can it instead
> explain why this change is necessary?
> 

The idea is that as new delegations are added or removed, the mask of
events that nfsd requires from the VFS layer can change, since clients
can request notifications of different events. I'll add that to the
changelog. 

> 
> > Reviewed-by: Jan Kara <jack@xxxxxxx>
> > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> > ---
> >  fs/nfsd/nfs4state.c | 34 ++++++++++++++++++++++++++++++++++
> >  1 file changed, 34 insertions(+)
> > 
> > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > index 2a34ba457b74..efbc99f0a965 100644
> > --- a/fs/nfsd/nfs4state.c
> > +++ b/fs/nfsd/nfs4state.c
> > @@ -1246,6 +1246,38 @@ static void 
> > nfsd4_finalize_deleg_timestamps(struct nfs4_delegation *dp, struct f
> >  	nfsd_update_cmtime_attr(f, ATTR_ATIME);
> >  }
> > 
> > +static void nfsd_fsnotify_recalc_mask(struct nfsd_file *nf)
> 
> Since nfsd_fsnotify_recalc_mask() takes a single struct nfsd_file
> as an argument, should this function reside in fs/nfsd/filecache.c
> instead? The question might reflect my misunderstanding of the
> new function's purpose.
> 

The only caller is in this file, so by keeping it here we can make it
static. I can change that if you'd prefer it be in filecache.c.

> 
> > +{
> > +	struct inode *inode = file_inode(nf->nf_file);
> > +	u32 lease_mask, set = 0, clear = 0;
> > +	struct fsnotify_mark *mark;
> > +
> > +	/* This is only needed when adding or removing dir delegs */
> > +	if (!S_ISDIR(inode->i_mode) || !nf->nf_mark)
> > +		return;
> > +
> > +	/* Set up notifications for any ignored delegation events */
> > +	lease_mask = inode_lease_ignore_mask(inode);
> > +	mark = &nf->nf_mark->nfm_mark;
> > +
> > +	if (lease_mask & FL_IGN_DIR_CREATE)
> > +		set |= FS_CREATE | FS_MOVED_TO;
> > +	else
> > +		clear |= FS_CREATE | FS_MOVED_TO;
> > +
> > +	if (lease_mask & FL_IGN_DIR_DELETE)
> > +		set |= FS_DELETE | FS_MOVED_FROM;
> > +	else
> > +		clear |= FS_DELETE | FS_MOVED_FROM;
> > +
> > +	if (lease_mask & FL_IGN_DIR_RENAME)
> > +		set |= FS_RENAME;
> > +	else
> > +		clear |= FS_RENAME;
> > +
> > +	fsnotify_modify_mark_mask(mark, set, clear);
> > +}
> > +
> >  static void nfs4_unlock_deleg_lease(struct nfs4_delegation *dp)
> >  {
> >  	struct nfs4_file *fp = dp->dl_stid.sc_file;
> > @@ -1255,6 +1287,7 @@ static void nfs4_unlock_deleg_lease(struct 
> > nfs4_delegation *dp)
> > 
> >  	nfsd4_finalize_deleg_timestamps(dp, nf->nf_file);
> >  	kernel_setlease(nf->nf_file, F_UNLCK, NULL, (void **)&dp);
> > +	nfsd_fsnotify_recalc_mask(nf);
> >  	put_deleg_file(fp);
> >  }
> > 
> 
> I added the following edit to this patch>
> 
> @@ -9597,8 +9629,7 @@ nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, struct dentry *dentry,
>   * @nf: nfsd_file opened on the directory
>   *
>   * Given a GET_DIR_DELEGATION request @gdd, attempt to acquire a delegation
> - * on the directory to which @nf refers. Note that this does not set up any
> - * sort of async notifications for the delegation.
> + * on the directory to which @nf refers.
>   */
>  struct nfs4_delegation *
>  nfsd_get_dir_deleg
> 
> The patch makes the above kerneldoc note ("does not set up any sort of async
> notifications") logically obsolete.
> 

Thanks. I folded that change into this patch.

> 
> > @@ -9682,6 +9715,7 @@ nfsd_get_dir_deleg(struct nfsd4_compound_state *cstate,
> > 
> >  	if (!status) {
> >  		put_nfs4_file(fp);
> > +		nfsd_fsnotify_recalc_mask(nf);
> >  		return dp;
> >  	}
> > 
> > 
> > -- 
> > 2.54.0

-- 
Jeff Layton <jlayton@xxxxxxxxxx>