Re: [PATCH 24/24] fs: remove simple_nosetlease()

Jeff Layton <[email protected]>
Newsgroups dev.linux.lists.gfs2,dev.linux.lists.ntfs3,dev.linux.lists.ocfs2-devel,dev.linux.lists.v9fs,net.sourceforge.lists.linux-f2fs-devel,org.infradead.lists.linux-mtd,org.kernel.vger.ceph-devel,org.kernel.vger.linux-btrfs,org.kernel.vger.linux-cifs,org.kernel.vger.linux-doc,org.kernel.vger.linux-ext4,org.kernel.vger.linux-fsdevel,org.kernel.vger.linux-kernel,org.kernel.vger.linux-nfs,org.kernel.vger.linux-nilfs,org.kernel.vger.linux-unionfs,org.kernel.vger.linux-xfs,org.kvack.linux-mm,org.ozlabs.lists.linux-erofs
Message-ID <[email protected]>
On Wed, 2026-02-25 at 12:58 -0500, Mike Snitzer wrote:
> On Thu, Jan 08, 2026 at 12:13:19PM -0500, Jeff Layton wrote:
> > Setting ->setlease() to a NULL pointer now has the same effect as
> > setting it to simple_nosetlease(). Remove all of the setlease
> > file_operations that are set to simple_nosetlease, and the function
> > itself.
> > 
> > Signed-off-by: Jeff Layton <[email protected]>
> > ---
> >  fs/9p/vfs_dir.c        |  2 --
> >  fs/9p/vfs_file.c       |  2 --
> >  fs/ceph/dir.c          |  2 --
> >  fs/ceph/file.c         |  1 -
> >  fs/fuse/dir.c          |  1 -
> >  fs/gfs2/file.c         |  2 --
> >  fs/libfs.c             | 18 ------------------
> >  fs/nfs/dir.c           |  1 -
> >  fs/nfs/file.c          |  1 -
> >  fs/smb/client/cifsfs.c |  1 -
> >  fs/vboxsf/dir.c        |  1 -
> >  fs/vboxsf/file.c       |  1 -
> >  include/linux/fs.h     |  1 -
> >  13 files changed, 34 deletions(-)
> > 
> 
> <snip>
> 
> > diff --git a/fs/libfs.c b/fs/libfs.c
> > index 697c6d5fc12786c036f0086886297fb5cd52ae00..f1860dff86f2703266beecf31e9d2667af7a9684 100644
> > --- a/fs/libfs.c
> > +++ b/fs/libfs.c
> > @@ -1699,24 +1699,6 @@ struct inode *alloc_anon_inode(struct super_block *s)
> >  }
> >  EXPORT_SYMBOL(alloc_anon_inode);
> >  
> > -/**
> > - * simple_nosetlease - generic helper for prohibiting leases
> > - * @filp: file pointer
> > - * @arg: type of lease to obtain
> > - * @flp: new lease supplied for insertion
> > - * @priv: private data for lm_setup operation
> > - *
> > - * Generic helper for filesystems that do not wish to allow leases to be set.
> > - * All arguments are ignored and it just returns -EINVAL.
> > - */
> > -int
> > -simple_nosetlease(struct file *filp, int arg, struct file_lease **flp,
> > -		  void **priv)
> > -{
> > -	return -EINVAL;
> > -}
> > -EXPORT_SYMBOL(simple_nosetlease);
> > -
> >  /**
> >   * simple_get_link - generic helper to get the target of "fast" symlinks
> >   * @dentry: not used here
> > diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
> > index 71df279febf797880ded19e45528c3df4cea2dde..23a78a742b619dea8b76ddf28f4f59a1c8a015e2 100644
> > --- a/fs/nfs/dir.c
> > +++ b/fs/nfs/dir.c
> > @@ -66,7 +66,6 @@ const struct file_operations nfs_dir_operations = {
> >  	.open		= nfs_opendir,
> >  	.release	= nfs_closedir,
> >  	.fsync		= nfs_fsync_dir,
> > -	.setlease	= simple_nosetlease,
> >  };
> >  
> >  const struct address_space_operations nfs_dir_aops = {
> > diff --git a/fs/nfs/file.c b/fs/nfs/file.c
> > index d020aab40c64ebda30d130b6acee1b9194621457..9d269561961825f88529551b0f0287920960ac62 100644
> > --- a/fs/nfs/file.c
> > +++ b/fs/nfs/file.c
> > @@ -962,7 +962,6 @@ const struct file_operations nfs_file_operations = {
> >  	.splice_read	= nfs_file_splice_read,
> >  	.splice_write	= iter_file_splice_write,
> >  	.check_flags	= nfs_check_flags,
> > -	.setlease	= simple_nosetlease,
> >  	.fop_flags	= FOP_DONTCACHE,
> >  };
> >  EXPORT_SYMBOL_GPL(nfs_file_operations);
> 
> Hey Jeff,
> 
> I've noticed an NFS reexport regression in v6.19 and now v7.0-rc1
> (similar but different due to your series that requires opt-in via
> .setlease).
> 
> Bisect first pointed out this commit:
> 10dcd5110678 nfs: properly disallow delegation requests on directories
> 
> And now with v7.0-rc1 its the fact that NFS doesn't provide .setlease
> so lstat() on parent dir (of file that I touch) gets -EINVAL.
> 
> So its a confluence of NFS's dir delegations and your setlease changes.
> 
> If I reexport NFSv4.2 filesystem in terms of NFSv4.1, the regression
> is seen by doing (lstat reproducer that gemini spit out for me is
> attached):
> 
> $ touch /mnt/share41/test
> $ strace ./lstat /mnt/share41
> ...
> lstat("/mnt/share41", 0x7ffec0d79920)   = -1 EINVAL (Invalid argument)
> 
> If I immediately re-run it works:
> ...
> lstat("/mnt/share41", {st_mode=S_IFDIR|0777, st_size=4096, ...}) = 0
> 
> I'm not sure what the proper fix is yet, but I feel like you've missed
> that NFS itself can be (re)exported?
> 
> 

My apologies. I missed seeing this last week.

That's a very simple reproducer! That's very strange behavior,
especially since NFS4 does provide a setlease operation:

const struct file_operations nfs4_file_operations = {
	[...]
	.setlease       = nfs4_setlease,
	[...]
};

I'm not sure why this would cause lstat() to return -EINVAL. What's
happening on the wire when this occurs?

I'll plan to take a look here soon either way.
-- 
Jeff Layton <[email protected]>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.