linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 00/11] vfs: recall-only directory delegations for knfsd
@ 2025-10-17 11:31 Jeff Layton
  2025-10-17 11:31 ` [PATCH v2 01/11] filelock: push the S_ISREG check down to ->setlease handlers Jeff Layton
                   ` (12 more replies)
  0 siblings, 13 replies; 20+ messages in thread
From: Jeff Layton @ 2025-10-17 11:31 UTC (permalink / raw)
  To: Miklos Szeredi, Alexander Viro, Christian Brauner, Jan Kara,
	Chuck Lever, Alexander Aring, Trond Myklebust, Anna Schumaker,
	Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
	Tom Talpey, Bharath SM, Greg Kroah-Hartman, Rafael J. Wysocki,
	Danilo Krummrich, David Howells, Tyler Hicks, NeilBrown,
	Olga Kornievskaia, Dai Ngo, Amir Goldstein, Namjae Jeon,
	Steve French, Sergey Senozhatsky, Carlos Maiolino,
	Kuniyuki Iwashima, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman
  Cc: linux-fsdevel, linux-kernel, linux-nfs, linux-cifs,
	samba-technical, netfs, ecryptfs, linux-unionfs, linux-xfs,
	netdev, Jeff Layton

A smaller variation of the v1 patchset that I posted earlier this week.
Neil's review inspired me to get rid of the lm_may_setlease operation
and to do the conflict resolution internally inside of nfsd. That means
a smaller VFS-layer change, and an overall reduction in code.

This patchset adds support for directory delegations to nfsd. This
version only supports recallable delegations. There is no CB_NOTIFY
support yet. I have patches for those, but we've decided to add that
support in a later kernel once we get some experience with this part.
Anna is working on the client-side pieces.

It would be great if we could get into linux-next soon so that it can be
merged for v6.19. Christian, could you pick up the vfs/filelock patches,
and Chuck pick up the nfsd patches?

Thanks!
Jeff

[1]: https://lore.kernel.org/all/20240315-dir-deleg-v1-0-a1d6209a3654@kernel.org/

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
Changes in v2:
- handle lease conflict resolution inside of nfsd
- drop the lm_may_setlease lock_manager operation
- just add extra argument to vfs_create() instead of creating wrapper
- don't allocate fsnotify_mark for open directories
- Link to v1: https://lore.kernel.org/r/20251013-dir-deleg-ro-v1-0-406780a70e5e@kernel.org

---
Jeff Layton (11):
      filelock: push the S_ISREG check down to ->setlease handlers
      vfs: add try_break_deleg calls for parents to vfs_{link,rename,unlink}
      vfs: allow mkdir to wait for delegation break on parent
      vfs: allow rmdir to wait for delegation break on parent
      vfs: break parent dir delegations in open(..., O_CREAT) codepath
      vfs: make vfs_create break delegations on parent directory
      vfs: make vfs_mknod break delegations on parent directory
      filelock: lift the ban on directory leases in generic_setlease
      nfsd: allow filecache to hold S_IFDIR files
      nfsd: allow DELEGRETURN on directories
      nfsd: wire up GET_DIR_DELEGATION handling

 drivers/base/devtmpfs.c  |   6 +--
 fs/cachefiles/namei.c    |   2 +-
 fs/ecryptfs/inode.c      |   8 +--
 fs/fuse/dir.c            |   1 +
 fs/init.c                |   4 +-
 fs/locks.c               |  12 +++--
 fs/namei.c               | 134 ++++++++++++++++++++++++++++++++++++-----------
 fs/nfs/nfs4file.c        |   2 +
 fs/nfsd/filecache.c      |  57 +++++++++++++++-----
 fs/nfsd/filecache.h      |   2 +
 fs/nfsd/nfs3proc.c       |   2 +-
 fs/nfsd/nfs4proc.c       |  21 +++++++-
 fs/nfsd/nfs4recover.c    |   6 +--
 fs/nfsd/nfs4state.c      | 103 +++++++++++++++++++++++++++++++++++-
 fs/nfsd/state.h          |   5 ++
 fs/nfsd/vfs.c            |  14 ++---
 fs/nfsd/vfs.h            |   2 +-
 fs/open.c                |   2 +-
 fs/overlayfs/overlayfs.h |   8 +--
 fs/smb/client/cifsfs.c   |   3 ++
 fs/smb/server/vfs.c      |   8 +--
 fs/xfs/scrub/orphanage.c |   2 +-
 include/linux/fs.h       |  11 ++--
 net/unix/af_unix.c       |   2 +-
 24 files changed, 329 insertions(+), 88 deletions(-)
---
base-commit: 2c40814eb5ae104d3f898fd8b705ecad114105b5
change-id: 20251013-dir-deleg-ro-d0fe19823b21

Best regards,
-- 
Jeff Layton <jlayton@kernel.org>


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v2 01/11] filelock: push the S_ISREG check down to ->setlease handlers
  2025-10-17 11:31 [PATCH v2 00/11] vfs: recall-only directory delegations for knfsd Jeff Layton
@ 2025-10-17 11:31 ` Jeff Layton
  2025-10-17 11:31 ` [PATCH v2 02/11] vfs: add try_break_deleg calls for parents to vfs_{link,rename,unlink} Jeff Layton
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: Jeff Layton @ 2025-10-17 11:31 UTC (permalink / raw)
  To: Miklos Szeredi, Alexander Viro, Christian Brauner, Jan Kara,
	Chuck Lever, Alexander Aring, Trond Myklebust, Anna Schumaker,
	Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
	Tom Talpey, Bharath SM, Greg Kroah-Hartman, Rafael J. Wysocki,
	Danilo Krummrich, David Howells, Tyler Hicks, NeilBrown,
	Olga Kornievskaia, Dai Ngo, Amir Goldstein, Namjae Jeon,
	Steve French, Sergey Senozhatsky, Carlos Maiolino,
	Kuniyuki Iwashima, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman
  Cc: linux-fsdevel, linux-kernel, linux-nfs, linux-cifs,
	samba-technical, netfs, ecryptfs, linux-unionfs, linux-xfs,
	netdev, Jeff Layton

When nfsd starts requesting directory delegations, setlease handlers may
see requests for leases on directories. Push the !S_ISREG check down
into the non-trivial setlease handlers, so we can selectively enable
them where they're supported.

FUSE is special: It's the only filesystem that supports atomic_open and
allows kernel-internal leases. atomic_open is issued when the VFS
doesn't know the state of the dentry being opened. If the file doesn't
exist, it may be created, in which case the dir lease should be broken.

The existing kernel-internal lease implementation has no provision for
this. Ensure that we don't allow directory leases by default going
forward by explicitly disabling them there.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/fuse/dir.c          | 1 +
 fs/locks.c             | 5 +++--
 fs/nfs/nfs4file.c      | 2 ++
 fs/smb/client/cifsfs.c | 3 +++
 4 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index ecaec0fea3a132e7cbb88121e7db7fb504d57d3c..667774cc72a1d49796f531fcb342d2e4878beb85 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -2230,6 +2230,7 @@ static const struct file_operations fuse_dir_operations = {
 	.fsync		= fuse_dir_fsync,
 	.unlocked_ioctl	= fuse_dir_ioctl,
 	.compat_ioctl	= fuse_dir_compat_ioctl,
+	.setlease	= simple_nosetlease,
 };
 
 static const struct inode_operations fuse_common_inode_operations = {
diff --git a/fs/locks.c b/fs/locks.c
index 04a3f0e2072461b6e2d3d1cd12f2b089d69a7db3..0b16921fb52e602ea2e0c3de39d9d772af98ba7d 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -1929,6 +1929,9 @@ static int generic_delete_lease(struct file *filp, void *owner)
 int generic_setlease(struct file *filp, int arg, struct file_lease **flp,
 			void **priv)
 {
+	if (!S_ISREG(file_inode(filp)->i_mode))
+		return -EINVAL;
+
 	switch (arg) {
 	case F_UNLCK:
 		return generic_delete_lease(filp, *priv);
@@ -2018,8 +2021,6 @@ vfs_setlease(struct file *filp, int arg, struct file_lease **lease, void **priv)
 
 	if ((!vfsuid_eq_kuid(vfsuid, current_fsuid())) && !capable(CAP_LEASE))
 		return -EACCES;
-	if (!S_ISREG(inode->i_mode))
-		return -EINVAL;
 	error = security_file_lock(filp, arg);
 	if (error)
 		return error;
diff --git a/fs/nfs/nfs4file.c b/fs/nfs/nfs4file.c
index 7f43e890d3564a000dab9365048a3e17dc96395c..7317f26892c5782a39660cae87ec1afea24e36c0 100644
--- a/fs/nfs/nfs4file.c
+++ b/fs/nfs/nfs4file.c
@@ -431,6 +431,8 @@ void nfs42_ssc_unregister_ops(void)
 static int nfs4_setlease(struct file *file, int arg, struct file_lease **lease,
 			 void **priv)
 {
+	if (!S_ISREG(file_inode(file)->i_mode))
+		return -EINVAL;
 	return nfs4_proc_setlease(file, arg, lease, priv);
 }
 
diff --git a/fs/smb/client/cifsfs.c b/fs/smb/client/cifsfs.c
index 05b1fa76e8ccf1e86f0c174593cd6e1acb84608d..03c44c1d9bb631b87a8b67aa16e481d6bb3c7d14 100644
--- a/fs/smb/client/cifsfs.c
+++ b/fs/smb/client/cifsfs.c
@@ -1149,6 +1149,9 @@ cifs_setlease(struct file *file, int arg, struct file_lease **lease, void **priv
 	struct inode *inode = file_inode(file);
 	struct cifsFileInfo *cfile = file->private_data;
 
+	if (!S_ISREG(inode->i_mode))
+		return -EINVAL;
+
 	/* Check if file is oplocked if this is request for new lease */
 	if (arg == F_UNLCK ||
 	    ((arg == F_RDLCK) && CIFS_CACHE_READ(CIFS_I(inode))) ||

-- 
2.51.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 02/11] vfs: add try_break_deleg calls for parents to vfs_{link,rename,unlink}
  2025-10-17 11:31 [PATCH v2 00/11] vfs: recall-only directory delegations for knfsd Jeff Layton
  2025-10-17 11:31 ` [PATCH v2 01/11] filelock: push the S_ISREG check down to ->setlease handlers Jeff Layton
@ 2025-10-17 11:31 ` Jeff Layton
  2025-10-17 11:31 ` [PATCH v2 03/11] vfs: allow mkdir to wait for delegation break on parent Jeff Layton
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: Jeff Layton @ 2025-10-17 11:31 UTC (permalink / raw)
  To: Miklos Szeredi, Alexander Viro, Christian Brauner, Jan Kara,
	Chuck Lever, Alexander Aring, Trond Myklebust, Anna Schumaker,
	Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
	Tom Talpey, Bharath SM, Greg Kroah-Hartman, Rafael J. Wysocki,
	Danilo Krummrich, David Howells, Tyler Hicks, NeilBrown,
	Olga Kornievskaia, Dai Ngo, Amir Goldstein, Namjae Jeon,
	Steve French, Sergey Senozhatsky, Carlos Maiolino,
	Kuniyuki Iwashima, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman
  Cc: linux-fsdevel, linux-kernel, linux-nfs, linux-cifs,
	samba-technical, netfs, ecryptfs, linux-unionfs, linux-xfs,
	netdev, Jeff Layton

In order to add directory delegation support, we need to break
delegations on the parent whenever there is going to be a change in the
directory.

vfs_link, vfs_unlink, and vfs_rename all have existing delegation break
handling for the children in the rename. Add the necessary calls for
breaking delegations in the parent(s) as well.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/namei.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/fs/namei.c b/fs/namei.c
index 7377020a2cba02501483020e0fc93c279fb38d3e..6e61e0215b34134b1690f864e2719e3f82cf71a8 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -4667,6 +4667,9 @@ int vfs_unlink(struct mnt_idmap *idmap, struct inode *dir,
 	else {
 		error = security_inode_unlink(dir, dentry);
 		if (!error) {
+			error = try_break_deleg(dir, delegated_inode);
+			if (error)
+				goto out;
 			error = try_break_deleg(target, delegated_inode);
 			if (error)
 				goto out;
@@ -4936,7 +4939,9 @@ int vfs_link(struct dentry *old_dentry, struct mnt_idmap *idmap,
 	else if (max_links && inode->i_nlink >= max_links)
 		error = -EMLINK;
 	else {
-		error = try_break_deleg(inode, delegated_inode);
+		error = try_break_deleg(dir, delegated_inode);
+		if (!error)
+			error = try_break_deleg(inode, delegated_inode);
 		if (!error)
 			error = dir->i_op->link(old_dentry, dir, new_dentry);
 	}
@@ -5203,6 +5208,14 @@ int vfs_rename(struct renamedata *rd)
 		    old_dir->i_nlink >= max_links)
 			goto out;
 	}
+	error = try_break_deleg(old_dir, delegated_inode);
+	if (error)
+		goto out;
+	if (new_dir != old_dir) {
+		error = try_break_deleg(new_dir, delegated_inode);
+		if (error)
+			goto out;
+	}
 	if (!is_dir) {
 		error = try_break_deleg(source, delegated_inode);
 		if (error)

-- 
2.51.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 03/11] vfs: allow mkdir to wait for delegation break on parent
  2025-10-17 11:31 [PATCH v2 00/11] vfs: recall-only directory delegations for knfsd Jeff Layton
  2025-10-17 11:31 ` [PATCH v2 01/11] filelock: push the S_ISREG check down to ->setlease handlers Jeff Layton
  2025-10-17 11:31 ` [PATCH v2 02/11] vfs: add try_break_deleg calls for parents to vfs_{link,rename,unlink} Jeff Layton
@ 2025-10-17 11:31 ` Jeff Layton
  2025-10-17 11:31 ` [PATCH v2 04/11] vfs: allow rmdir " Jeff Layton
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: Jeff Layton @ 2025-10-17 11:31 UTC (permalink / raw)
  To: Miklos Szeredi, Alexander Viro, Christian Brauner, Jan Kara,
	Chuck Lever, Alexander Aring, Trond Myklebust, Anna Schumaker,
	Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
	Tom Talpey, Bharath SM, Greg Kroah-Hartman, Rafael J. Wysocki,
	Danilo Krummrich, David Howells, Tyler Hicks, NeilBrown,
	Olga Kornievskaia, Dai Ngo, Amir Goldstein, Namjae Jeon,
	Steve French, Sergey Senozhatsky, Carlos Maiolino,
	Kuniyuki Iwashima, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman
  Cc: linux-fsdevel, linux-kernel, linux-nfs, linux-cifs,
	samba-technical, netfs, ecryptfs, linux-unionfs, linux-xfs,
	netdev, Jeff Layton

In order to add directory delegation support, we need to break
delegations on the parent whenever there is going to be a change in the
directory.

Add a new delegated_inode parameter to vfs_mkdir. All of the existing
callers set that to NULL for now, except for do_mkdirat which will
properly block until the lease is gone.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 drivers/base/devtmpfs.c  |  2 +-
 fs/cachefiles/namei.c    |  2 +-
 fs/ecryptfs/inode.c      |  2 +-
 fs/init.c                |  2 +-
 fs/namei.c               | 24 ++++++++++++++++++------
 fs/nfsd/nfs4recover.c    |  2 +-
 fs/nfsd/vfs.c            |  2 +-
 fs/overlayfs/overlayfs.h |  2 +-
 fs/smb/server/vfs.c      |  2 +-
 fs/xfs/scrub/orphanage.c |  2 +-
 include/linux/fs.h       |  2 +-
 11 files changed, 28 insertions(+), 16 deletions(-)

diff --git a/drivers/base/devtmpfs.c b/drivers/base/devtmpfs.c
index 9d4e46ad8352257a6a65d85526ebdbf9bf2d4b19..0e79621cb0f79870003b867ca384199171ded4e0 100644
--- a/drivers/base/devtmpfs.c
+++ b/drivers/base/devtmpfs.c
@@ -180,7 +180,7 @@ static int dev_mkdir(const char *name, umode_t mode)
 	if (IS_ERR(dentry))
 		return PTR_ERR(dentry);
 
-	dentry = vfs_mkdir(&nop_mnt_idmap, d_inode(path.dentry), dentry, mode);
+	dentry = vfs_mkdir(&nop_mnt_idmap, d_inode(path.dentry), dentry, mode, NULL);
 	if (!IS_ERR(dentry))
 		/* mark as kernel-created inode */
 		d_inode(dentry)->i_private = &thread;
diff --git a/fs/cachefiles/namei.c b/fs/cachefiles/namei.c
index d1edb2ac38376c4f9d2a18026450bb3c774f7824..50c0f9c76d1fd4c05db90d7d0d1bad574523ead0 100644
--- a/fs/cachefiles/namei.c
+++ b/fs/cachefiles/namei.c
@@ -130,7 +130,7 @@ struct dentry *cachefiles_get_directory(struct cachefiles_cache *cache,
 			goto mkdir_error;
 		ret = cachefiles_inject_write_error();
 		if (ret == 0)
-			subdir = vfs_mkdir(&nop_mnt_idmap, d_inode(dir), subdir, 0700);
+			subdir = vfs_mkdir(&nop_mnt_idmap, d_inode(dir), subdir, 0700, NULL);
 		else
 			subdir = ERR_PTR(ret);
 		if (IS_ERR(subdir)) {
diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
index ed1394da8d6bd7065f2a074378331f13fcda17f9..35830b3144f8f71374a78b3e7463b864f4fc216e 100644
--- a/fs/ecryptfs/inode.c
+++ b/fs/ecryptfs/inode.c
@@ -508,7 +508,7 @@ static struct dentry *ecryptfs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
 		goto out;
 
 	lower_dentry = vfs_mkdir(&nop_mnt_idmap, lower_dir,
-				 lower_dentry, mode);
+				 lower_dentry, mode, NULL);
 	rc = PTR_ERR(lower_dentry);
 	if (IS_ERR(lower_dentry))
 		goto out;
diff --git a/fs/init.c b/fs/init.c
index 07f592ccdba868509d0f3aaf9936d8d890fdbec5..895f8a09a71acfd03e11164e3b441a7d4e2de146 100644
--- a/fs/init.c
+++ b/fs/init.c
@@ -233,7 +233,7 @@ int __init init_mkdir(const char *pathname, umode_t mode)
 	error = security_path_mkdir(&path, dentry, mode);
 	if (!error) {
 		dentry = vfs_mkdir(mnt_idmap(path.mnt), path.dentry->d_inode,
-				  dentry, mode);
+				  dentry, mode, NULL);
 		if (IS_ERR(dentry))
 			error = PTR_ERR(dentry);
 	}
diff --git a/fs/namei.c b/fs/namei.c
index 6e61e0215b34134b1690f864e2719e3f82cf71a8..86cf6eca1f485361c6732974e4103cf5ea721539 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -4407,10 +4407,11 @@ SYSCALL_DEFINE3(mknod, const char __user *, filename, umode_t, mode, unsigned, d
 
 /**
  * vfs_mkdir - create directory returning correct dentry if possible
- * @idmap:	idmap of the mount the inode was found from
- * @dir:	inode of the parent directory
- * @dentry:	dentry of the child directory
- * @mode:	mode of the child directory
+ * @idmap:		idmap of the mount the inode was found from
+ * @dir:		inode of the parent directory
+ * @dentry:		dentry of the child directory
+ * @mode:		mode of the child directory
+ * @delegated_inode:	returns parent inode, if the inode is delegated.
  *
  * Create a directory.
  *
@@ -4427,7 +4428,8 @@ SYSCALL_DEFINE3(mknod, const char __user *, filename, umode_t, mode, unsigned, d
  * In case of an error the dentry is dput() and an ERR_PTR() is returned.
  */
 struct dentry *vfs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
-			 struct dentry *dentry, umode_t mode)
+			 struct dentry *dentry, umode_t mode,
+			 struct inode **delegated_inode)
 {
 	int error;
 	unsigned max_links = dir->i_sb->s_max_links;
@@ -4450,6 +4452,10 @@ struct dentry *vfs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
 	if (max_links && dir->i_nlink >= max_links)
 		goto err;
 
+	error = try_break_deleg(dir, delegated_inode);
+	if (error)
+		goto err;
+
 	de = dir->i_op->mkdir(idmap, dir, dentry, mode);
 	error = PTR_ERR(de);
 	if (IS_ERR(de))
@@ -4473,6 +4479,7 @@ int do_mkdirat(int dfd, struct filename *name, umode_t mode)
 	struct path path;
 	int error;
 	unsigned int lookup_flags = LOOKUP_DIRECTORY;
+	struct inode *delegated_inode = NULL;
 
 retry:
 	dentry = filename_create(dfd, name, &path, lookup_flags);
@@ -4484,11 +4491,16 @@ int do_mkdirat(int dfd, struct filename *name, umode_t mode)
 			mode_strip_umask(path.dentry->d_inode, mode));
 	if (!error) {
 		dentry = vfs_mkdir(mnt_idmap(path.mnt), path.dentry->d_inode,
-				  dentry, mode);
+				   dentry, mode, &delegated_inode);
 		if (IS_ERR(dentry))
 			error = PTR_ERR(dentry);
 	}
 	end_creating_path(&path, dentry);
+	if (delegated_inode) {
+		error = break_deleg_wait(&delegated_inode);
+		if (!error)
+			goto retry;
+	}
 	if (retry_estale(error, lookup_flags)) {
 		lookup_flags |= LOOKUP_REVAL;
 		goto retry;
diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
index c41bb85e06822a82c86e2a33bd9a91e978c75965..3dfbb85c9a1166b56e56eb9f1d6bfd140584730b 100644
--- a/fs/nfsd/nfs4recover.c
+++ b/fs/nfsd/nfs4recover.c
@@ -162,7 +162,7 @@ nfsd4_create_clid_dir(struct nfs4_client *clp)
 		 * as well be forgiving and just succeed silently.
 		 */
 		goto out_put;
-	dentry = vfs_mkdir(&nop_mnt_idmap, d_inode(dir), dentry, S_IRWXU);
+	dentry = vfs_mkdir(&nop_mnt_idmap, d_inode(dir), dentry, 0700, NULL);
 	if (IS_ERR(dentry))
 		status = PTR_ERR(dentry);
 out_put:
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index f537a7b4ee01f49cfb864df9a92a9660b743a51e..447f5ab8e0b92288c9f220060ab15f32f2a84de9 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -1644,7 +1644,7 @@ nfsd_create_locked(struct svc_rqst *rqstp, struct svc_fh *fhp,
 			nfsd_check_ignore_resizing(iap);
 		break;
 	case S_IFDIR:
-		dchild = vfs_mkdir(&nop_mnt_idmap, dirp, dchild, iap->ia_mode);
+		dchild = vfs_mkdir(&nop_mnt_idmap, dirp, dchild, iap->ia_mode, NULL);
 		if (IS_ERR(dchild)) {
 			host_err = PTR_ERR(dchild);
 		} else if (d_is_negative(dchild)) {
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index c8fd5951fc5ece1ae6b3e2a0801ca15f9faf7d72..0f65f9a5d54d4786b39e4f4f30f416d5b9016e70 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -248,7 +248,7 @@ static inline struct dentry *ovl_do_mkdir(struct ovl_fs *ofs,
 {
 	struct dentry *ret;
 
-	ret = vfs_mkdir(ovl_upper_mnt_idmap(ofs), dir, dentry, mode);
+	ret = vfs_mkdir(ovl_upper_mnt_idmap(ofs), dir, dentry, mode, NULL);
 	pr_debug("mkdir(%pd2, 0%o) = %i\n", dentry, mode, PTR_ERR_OR_ZERO(ret));
 	return ret;
 }
diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c
index 891ed2dc2b7351a5cb14a2241d71095ffdd03f08..3d2190f26623b23ea79c63410905a3c3ad684048 100644
--- a/fs/smb/server/vfs.c
+++ b/fs/smb/server/vfs.c
@@ -230,7 +230,7 @@ int ksmbd_vfs_mkdir(struct ksmbd_work *work, const char *name, umode_t mode)
 	idmap = mnt_idmap(path.mnt);
 	mode |= S_IFDIR;
 	d = dentry;
-	dentry = vfs_mkdir(idmap, d_inode(path.dentry), dentry, mode);
+	dentry = vfs_mkdir(idmap, d_inode(path.dentry), dentry, mode, NULL);
 	if (IS_ERR(dentry))
 		err = PTR_ERR(dentry);
 	else if (d_is_negative(dentry))
diff --git a/fs/xfs/scrub/orphanage.c b/fs/xfs/scrub/orphanage.c
index 9c12cb8442311ca26b169e4d1567939ae44a5be0..91c9d07b97f306f57aebb9b69ba564b0c2cb8c17 100644
--- a/fs/xfs/scrub/orphanage.c
+++ b/fs/xfs/scrub/orphanage.c
@@ -167,7 +167,7 @@ xrep_orphanage_create(
 	 */
 	if (d_really_is_negative(orphanage_dentry)) {
 		orphanage_dentry = vfs_mkdir(&nop_mnt_idmap, root_inode,
-					     orphanage_dentry, 0750);
+					     orphanage_dentry, 0750, NULL);
 		error = PTR_ERR(orphanage_dentry);
 		if (IS_ERR(orphanage_dentry))
 			goto out_unlock_root;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index c895146c1444be36e0a779df55622cc38c9419ff..1040df3792794cd353b86558b41618294e25b8a6 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2113,7 +2113,7 @@ bool inode_owner_or_capable(struct mnt_idmap *idmap,
 int vfs_create(struct mnt_idmap *, struct inode *,
 	       struct dentry *, umode_t, bool);
 struct dentry *vfs_mkdir(struct mnt_idmap *, struct inode *,
-			 struct dentry *, umode_t);
+			 struct dentry *, umode_t, struct inode **);
 int vfs_mknod(struct mnt_idmap *, struct inode *, struct dentry *,
               umode_t, dev_t);
 int vfs_symlink(struct mnt_idmap *, struct inode *,

-- 
2.51.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 04/11] vfs: allow rmdir to wait for delegation break on parent
  2025-10-17 11:31 [PATCH v2 00/11] vfs: recall-only directory delegations for knfsd Jeff Layton
                   ` (2 preceding siblings ...)
  2025-10-17 11:31 ` [PATCH v2 03/11] vfs: allow mkdir to wait for delegation break on parent Jeff Layton
@ 2025-10-17 11:31 ` Jeff Layton
  2025-10-17 11:31 ` [PATCH v2 05/11] vfs: break parent dir delegations in open(..., O_CREAT) codepath Jeff Layton
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: Jeff Layton @ 2025-10-17 11:31 UTC (permalink / raw)
  To: Miklos Szeredi, Alexander Viro, Christian Brauner, Jan Kara,
	Chuck Lever, Alexander Aring, Trond Myklebust, Anna Schumaker,
	Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
	Tom Talpey, Bharath SM, Greg Kroah-Hartman, Rafael J. Wysocki,
	Danilo Krummrich, David Howells, Tyler Hicks, NeilBrown,
	Olga Kornievskaia, Dai Ngo, Amir Goldstein, Namjae Jeon,
	Steve French, Sergey Senozhatsky, Carlos Maiolino,
	Kuniyuki Iwashima, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman
  Cc: linux-fsdevel, linux-kernel, linux-nfs, linux-cifs,
	samba-technical, netfs, ecryptfs, linux-unionfs, linux-xfs,
	netdev, Jeff Layton

In order to add directory delegation support, we need to break
delegations on the parent whenever there is going to be a change in the
directory.

Add a "delegated_inode" return pointer to vfs_rmdir() and populate that
pointer with the parent inode if it's non-NULL. Most existing in-kernel
callers pass in a NULL pointer.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 drivers/base/devtmpfs.c  |  2 +-
 fs/ecryptfs/inode.c      |  2 +-
 fs/namei.c               | 22 +++++++++++++++++-----
 fs/nfsd/nfs4recover.c    |  4 ++--
 fs/nfsd/vfs.c            |  2 +-
 fs/overlayfs/overlayfs.h |  2 +-
 fs/smb/server/vfs.c      |  4 ++--
 include/linux/fs.h       |  3 ++-
 8 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/drivers/base/devtmpfs.c b/drivers/base/devtmpfs.c
index 0e79621cb0f79870003b867ca384199171ded4e0..104025104ef75381984fd94dfbd50feeaa8cdd22 100644
--- a/drivers/base/devtmpfs.c
+++ b/drivers/base/devtmpfs.c
@@ -261,7 +261,7 @@ static int dev_rmdir(const char *name)
 		return PTR_ERR(dentry);
 	if (d_inode(dentry)->i_private == &thread)
 		err = vfs_rmdir(&nop_mnt_idmap, d_inode(parent.dentry),
-				dentry);
+				dentry, NULL);
 	else
 		err = -EPERM;
 
diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
index 35830b3144f8f71374a78b3e7463b864f4fc216e..88631291b32535f623a3fbe4ea9b6ed48a306ca0 100644
--- a/fs/ecryptfs/inode.c
+++ b/fs/ecryptfs/inode.c
@@ -540,7 +540,7 @@ static int ecryptfs_rmdir(struct inode *dir, struct dentry *dentry)
 		if (d_unhashed(lower_dentry))
 			rc = -EINVAL;
 		else
-			rc = vfs_rmdir(&nop_mnt_idmap, lower_dir, lower_dentry);
+			rc = vfs_rmdir(&nop_mnt_idmap, lower_dir, lower_dentry, NULL);
 	}
 	if (!rc) {
 		clear_nlink(d_inode(dentry));
diff --git a/fs/namei.c b/fs/namei.c
index 86cf6eca1f485361c6732974e4103cf5ea721539..4b5a99653c558397e592715d9d4663cd4a63ef86 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -4522,9 +4522,10 @@ SYSCALL_DEFINE2(mkdir, const char __user *, pathname, umode_t, mode)
 
 /**
  * vfs_rmdir - remove directory
- * @idmap:	idmap of the mount the inode was found from
- * @dir:	inode of the parent directory
- * @dentry:	dentry of the child directory
+ * @idmap:		idmap of the mount the inode was found from
+ * @dir:		inode of the parent directory
+ * @dentry:		dentry of the child directory
+ * @delegated_inode:	returns parent inode, if it's delegated.
  *
  * Remove a directory.
  *
@@ -4535,7 +4536,7 @@ SYSCALL_DEFINE2(mkdir, const char __user *, pathname, umode_t, mode)
  * raw inode simply pass @nop_mnt_idmap.
  */
 int vfs_rmdir(struct mnt_idmap *idmap, struct inode *dir,
-		     struct dentry *dentry)
+	      struct dentry *dentry, struct inode **delegated_inode)
 {
 	int error = may_delete(idmap, dir, dentry, 1);
 
@@ -4557,6 +4558,10 @@ int vfs_rmdir(struct mnt_idmap *idmap, struct inode *dir,
 	if (error)
 		goto out;
 
+	error = try_break_deleg(dir, delegated_inode);
+	if (error)
+		goto out;
+
 	error = dir->i_op->rmdir(dir, dentry);
 	if (error)
 		goto out;
@@ -4583,6 +4588,7 @@ int do_rmdir(int dfd, struct filename *name)
 	struct qstr last;
 	int type;
 	unsigned int lookup_flags = 0;
+	struct inode *delegated_inode = NULL;
 retry:
 	error = filename_parentat(dfd, name, lookup_flags, &path, &last, &type);
 	if (error)
@@ -4612,7 +4618,8 @@ int do_rmdir(int dfd, struct filename *name)
 	error = security_path_rmdir(&path, dentry);
 	if (error)
 		goto exit4;
-	error = vfs_rmdir(mnt_idmap(path.mnt), path.dentry->d_inode, dentry);
+	error = vfs_rmdir(mnt_idmap(path.mnt), path.dentry->d_inode,
+			  dentry, &delegated_inode);
 exit4:
 	dput(dentry);
 exit3:
@@ -4620,6 +4627,11 @@ int do_rmdir(int dfd, struct filename *name)
 	mnt_drop_write(path.mnt);
 exit2:
 	path_put(&path);
+	if (delegated_inode) {
+		error = break_deleg_wait(&delegated_inode);
+		if (!error)
+			goto retry;
+	}
 	if (retry_estale(error, lookup_flags)) {
 		lookup_flags |= LOOKUP_REVAL;
 		goto retry;
diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
index 3dfbb85c9a1166b56e56eb9f1d6bfd140584730b..ad3acbb956d90cac88f74e5f598719af6f1f8328 100644
--- a/fs/nfsd/nfs4recover.c
+++ b/fs/nfsd/nfs4recover.c
@@ -275,7 +275,7 @@ nfsd4_unlink_clid_dir(char *name, struct nfsd_net *nn)
 	status = -ENOENT;
 	if (d_really_is_negative(dentry))
 		goto out;
-	status = vfs_rmdir(&nop_mnt_idmap, d_inode(dir), dentry);
+	status = vfs_rmdir(&nop_mnt_idmap, d_inode(dir), dentry, NULL);
 out:
 	dput(dentry);
 out_unlock:
@@ -367,7 +367,7 @@ purge_old(struct dentry *parent, char *cname, struct nfsd_net *nn)
 	inode_lock_nested(d_inode(parent), I_MUTEX_PARENT);
 	child = lookup_one(&nop_mnt_idmap, &QSTR(cname), parent);
 	if (!IS_ERR(child)) {
-		status = vfs_rmdir(&nop_mnt_idmap, d_inode(parent), child);
+		status = vfs_rmdir(&nop_mnt_idmap, d_inode(parent), child, NULL);
 		if (status)
 			printk("failed to remove client recovery directory %pd\n",
 			       child);
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 447f5ab8e0b92288c9f220060ab15f32f2a84de9..7d8cd2595f197be9741ee6320d43ed6651896647 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -2194,7 +2194,7 @@ nfsd_unlink(struct svc_rqst *rqstp, struct svc_fh *fhp, int type,
 				break;
 		}
 	} else {
-		host_err = vfs_rmdir(&nop_mnt_idmap, dirp, rdentry);
+		host_err = vfs_rmdir(&nop_mnt_idmap, dirp, rdentry, NULL);
 	}
 	fh_fill_post_attrs(fhp);
 
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index 0f65f9a5d54d4786b39e4f4f30f416d5b9016e70..d215d7349489686b66bb66e939b27046f7d836f6 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -206,7 +206,7 @@ static inline int ovl_do_notify_change(struct ovl_fs *ofs,
 static inline int ovl_do_rmdir(struct ovl_fs *ofs,
 			       struct inode *dir, struct dentry *dentry)
 {
-	int err = vfs_rmdir(ovl_upper_mnt_idmap(ofs), dir, dentry);
+	int err = vfs_rmdir(ovl_upper_mnt_idmap(ofs), dir, dentry, NULL);
 
 	pr_debug("rmdir(%pd2) = %i\n", dentry, err);
 	return err;
diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c
index 3d2190f26623b23ea79c63410905a3c3ad684048..c5f0f3170d586cb2dc4d416b80948c642797fb82 100644
--- a/fs/smb/server/vfs.c
+++ b/fs/smb/server/vfs.c
@@ -609,7 +609,7 @@ int ksmbd_vfs_remove_file(struct ksmbd_work *work, const struct path *path)
 
 	idmap = mnt_idmap(path->mnt);
 	if (S_ISDIR(d_inode(path->dentry)->i_mode)) {
-		err = vfs_rmdir(idmap, d_inode(parent), path->dentry);
+		err = vfs_rmdir(idmap, d_inode(parent), path->dentry, NULL);
 		if (err && err != -ENOTEMPTY)
 			ksmbd_debug(VFS, "rmdir failed, err %d\n", err);
 	} else {
@@ -1090,7 +1090,7 @@ int ksmbd_vfs_unlink(struct file *filp)
 	dget(dentry);
 
 	if (S_ISDIR(d_inode(dentry)->i_mode))
-		err = vfs_rmdir(idmap, d_inode(dir), dentry);
+		err = vfs_rmdir(idmap, d_inode(dir), dentry, NULL);
 	else
 		err = vfs_unlink(idmap, d_inode(dir), dentry, NULL);
 
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 1040df3792794cd353b86558b41618294e25b8a6..d8bdaf7c87502ff17775602f5391d375738b4ed8 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2120,7 +2120,8 @@ int vfs_symlink(struct mnt_idmap *, struct inode *,
 		struct dentry *, const char *);
 int vfs_link(struct dentry *, struct mnt_idmap *, struct inode *,
 	     struct dentry *, struct inode **);
-int vfs_rmdir(struct mnt_idmap *, struct inode *, struct dentry *);
+int vfs_rmdir(struct mnt_idmap *, struct inode *, struct dentry *,
+	      struct inode **);
 int vfs_unlink(struct mnt_idmap *, struct inode *, struct dentry *,
 	       struct inode **);
 

-- 
2.51.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 05/11] vfs: break parent dir delegations in open(..., O_CREAT) codepath
  2025-10-17 11:31 [PATCH v2 00/11] vfs: recall-only directory delegations for knfsd Jeff Layton
                   ` (3 preceding siblings ...)
  2025-10-17 11:31 ` [PATCH v2 04/11] vfs: allow rmdir " Jeff Layton
@ 2025-10-17 11:31 ` Jeff Layton
  2025-10-17 11:31 ` [PATCH v2 06/11] vfs: make vfs_create break delegations on parent directory Jeff Layton
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: Jeff Layton @ 2025-10-17 11:31 UTC (permalink / raw)
  To: Miklos Szeredi, Alexander Viro, Christian Brauner, Jan Kara,
	Chuck Lever, Alexander Aring, Trond Myklebust, Anna Schumaker,
	Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
	Tom Talpey, Bharath SM, Greg Kroah-Hartman, Rafael J. Wysocki,
	Danilo Krummrich, David Howells, Tyler Hicks, NeilBrown,
	Olga Kornievskaia, Dai Ngo, Amir Goldstein, Namjae Jeon,
	Steve French, Sergey Senozhatsky, Carlos Maiolino,
	Kuniyuki Iwashima, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman
  Cc: linux-fsdevel, linux-kernel, linux-nfs, linux-cifs,
	samba-technical, netfs, ecryptfs, linux-unionfs, linux-xfs,
	netdev, Jeff Layton

In order to add directory delegation support, we need to break
delegations on the parent whenever there is going to be a change in the
directory.

Add a delegated_inode parameter to lookup_open and have it break the
delegation. Then, open_last_lookups can wait for the delegation break
and retry the call to lookup_open once it's done.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/namei.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 4b5a99653c558397e592715d9d4663cd4a63ef86..786f42bd184b5dbf6d754fa1fb6c94c0f75429f2 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3697,7 +3697,7 @@ static struct dentry *atomic_open(struct nameidata *nd, struct dentry *dentry,
  */
 static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
 				  const struct open_flags *op,
-				  bool got_write)
+				  bool got_write, struct inode **delegated_inode)
 {
 	struct mnt_idmap *idmap;
 	struct dentry *dir = nd->path.dentry;
@@ -3786,6 +3786,11 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
 
 	/* Negative dentry, just create the file */
 	if (!dentry->d_inode && (open_flag & O_CREAT)) {
+		/* but break the directory lease first! */
+		error = try_break_deleg(dir_inode, delegated_inode);
+		if (error)
+			goto out_dput;
+
 		file->f_mode |= FMODE_CREATED;
 		audit_inode_child(dir_inode, dentry, AUDIT_TYPE_CHILD_CREATE);
 		if (!dir_inode->i_op->create) {
@@ -3849,6 +3854,7 @@ static const char *open_last_lookups(struct nameidata *nd,
 		   struct file *file, const struct open_flags *op)
 {
 	struct dentry *dir = nd->path.dentry;
+	struct inode *delegated_inode = NULL;
 	int open_flag = op->open_flag;
 	bool got_write = false;
 	struct dentry *dentry;
@@ -3879,7 +3885,7 @@ static const char *open_last_lookups(struct nameidata *nd,
 				return ERR_PTR(-ECHILD);
 		}
 	}
-
+retry:
 	if (open_flag & (O_CREAT | O_TRUNC | O_WRONLY | O_RDWR)) {
 		got_write = !mnt_want_write(nd->path.mnt);
 		/*
@@ -3892,7 +3898,7 @@ static const char *open_last_lookups(struct nameidata *nd,
 		inode_lock(dir->d_inode);
 	else
 		inode_lock_shared(dir->d_inode);
-	dentry = lookup_open(nd, file, op, got_write);
+	dentry = lookup_open(nd, file, op, got_write, &delegated_inode);
 	if (!IS_ERR(dentry)) {
 		if (file->f_mode & FMODE_CREATED)
 			fsnotify_create(dir->d_inode, dentry);
@@ -3907,8 +3913,16 @@ static const char *open_last_lookups(struct nameidata *nd,
 	if (got_write)
 		mnt_drop_write(nd->path.mnt);
 
-	if (IS_ERR(dentry))
+	if (IS_ERR(dentry)) {
+		if (delegated_inode) {
+			int error = break_deleg_wait(&delegated_inode);
+
+			if (!error)
+				goto retry;
+			return ERR_PTR(error);
+		}
 		return ERR_CAST(dentry);
+	}
 
 	if (file->f_mode & (FMODE_OPENED | FMODE_CREATED)) {
 		dput(nd->path.dentry);

-- 
2.51.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 06/11] vfs: make vfs_create break delegations on parent directory
  2025-10-17 11:31 [PATCH v2 00/11] vfs: recall-only directory delegations for knfsd Jeff Layton
                   ` (4 preceding siblings ...)
  2025-10-17 11:31 ` [PATCH v2 05/11] vfs: break parent dir delegations in open(..., O_CREAT) codepath Jeff Layton
@ 2025-10-17 11:31 ` Jeff Layton
  2025-10-17 11:31 ` [PATCH v2 07/11] vfs: make vfs_mknod " Jeff Layton
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: Jeff Layton @ 2025-10-17 11:31 UTC (permalink / raw)
  To: Miklos Szeredi, Alexander Viro, Christian Brauner, Jan Kara,
	Chuck Lever, Alexander Aring, Trond Myklebust, Anna Schumaker,
	Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
	Tom Talpey, Bharath SM, Greg Kroah-Hartman, Rafael J. Wysocki,
	Danilo Krummrich, David Howells, Tyler Hicks, NeilBrown,
	Olga Kornievskaia, Dai Ngo, Amir Goldstein, Namjae Jeon,
	Steve French, Sergey Senozhatsky, Carlos Maiolino,
	Kuniyuki Iwashima, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman
  Cc: linux-fsdevel, linux-kernel, linux-nfs, linux-cifs,
	samba-technical, netfs, ecryptfs, linux-unionfs, linux-xfs,
	netdev, Jeff Layton

In order to add directory delegation support, we need to break
delegations on the parent whenever there is going to be a change in the
directory.

Add a delegated_inode parameter to vfs_create. Most callers are
converted to pass in NULL, but do_mknodat() is changed to wait for a
delegation break if there is one.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/ecryptfs/inode.c      |  2 +-
 fs/namei.c               | 26 +++++++++++++++++++-------
 fs/nfsd/nfs3proc.c       |  2 +-
 fs/nfsd/vfs.c            |  3 +--
 fs/open.c                |  2 +-
 fs/overlayfs/overlayfs.h |  2 +-
 fs/smb/server/vfs.c      |  2 +-
 include/linux/fs.h       |  2 +-
 8 files changed, 26 insertions(+), 15 deletions(-)

diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
index 88631291b32535f623a3fbe4ea9b6ed48a306ca0..661709b157ce854c3bfdfdb13f7c10435fad9756 100644
--- a/fs/ecryptfs/inode.c
+++ b/fs/ecryptfs/inode.c
@@ -189,7 +189,7 @@ ecryptfs_do_create(struct inode *directory_inode,
 	rc = lock_parent(ecryptfs_dentry, &lower_dentry, &lower_dir);
 	if (!rc)
 		rc = vfs_create(&nop_mnt_idmap, lower_dir,
-				lower_dentry, mode, true);
+				lower_dentry, mode, true, NULL);
 	if (rc) {
 		printk(KERN_ERR "%s: Failure to create dentry in lower fs; "
 		       "rc = [%d]\n", __func__, rc);
diff --git a/fs/namei.c b/fs/namei.c
index 786f42bd184b5dbf6d754fa1fb6c94c0f75429f2..7510942e0249de19df4363b92f813b3acdfc2254 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3460,11 +3460,12 @@ static inline umode_t vfs_prepare_mode(struct mnt_idmap *idmap,
 
 /**
  * vfs_create - create new file
- * @idmap:	idmap of the mount the inode was found from
- * @dir:	inode of the parent directory
- * @dentry:	dentry of the child file
- * @mode:	mode of the child file
- * @want_excl:	whether the file must not yet exist
+ * @idmap:		idmap of the mount the inode was found from
+ * @dir:		inode of the parent directory
+ * @dentry:		dentry of the child file
+ * @mode:		mode of the child file
+ * @want_excl:		whether the file must not yet exist
+ * @delegated_inode:	returns parent inode, if the inode is delegated.
  *
  * Create a new file.
  *
@@ -3475,7 +3476,8 @@ static inline umode_t vfs_prepare_mode(struct mnt_idmap *idmap,
  * raw inode simply pass @nop_mnt_idmap.
  */
 int vfs_create(struct mnt_idmap *idmap, struct inode *dir,
-	       struct dentry *dentry, umode_t mode, bool want_excl)
+	       struct dentry *dentry, umode_t mode, bool want_excl,
+	       struct inode **delegated_inode)
 {
 	int error;
 
@@ -3488,6 +3490,9 @@ int vfs_create(struct mnt_idmap *idmap, struct inode *dir,
 
 	mode = vfs_prepare_mode(idmap, dir, mode, S_IALLUGO, S_IFREG);
 	error = security_inode_create(dir, dentry, mode);
+	if (error)
+		return error;
+	error = try_break_deleg(dir, delegated_inode);
 	if (error)
 		return error;
 	error = dir->i_op->create(idmap, dir, dentry, mode, want_excl);
@@ -4365,6 +4370,7 @@ static int do_mknodat(int dfd, struct filename *name, umode_t mode,
 	struct path path;
 	int error;
 	unsigned int lookup_flags = 0;
+	struct inode *delegated_inode = NULL;
 
 	error = may_mknod(mode);
 	if (error)
@@ -4384,7 +4390,8 @@ static int do_mknodat(int dfd, struct filename *name, umode_t mode,
 	switch (mode & S_IFMT) {
 		case 0: case S_IFREG:
 			error = vfs_create(idmap, path.dentry->d_inode,
-					   dentry, mode, true);
+					   dentry, mode, true,
+					   &delegated_inode);
 			if (!error)
 				security_path_post_mknod(idmap, dentry);
 			break;
@@ -4399,6 +4406,11 @@ static int do_mknodat(int dfd, struct filename *name, umode_t mode,
 	}
 out2:
 	end_creating_path(&path, dentry);
+	if (delegated_inode) {
+		error = break_deleg_wait(&delegated_inode);
+		if (!error)
+			goto retry;
+	}
 	if (retry_estale(error, lookup_flags)) {
 		lookup_flags |= LOOKUP_REVAL;
 		goto retry;
diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c
index b6d03e1ef5f7a5e8dd111b0d56c061f1e91abff7..f3977c52fe27b4ff0c0e2d62cf8c14b49bbd8b13 100644
--- a/fs/nfsd/nfs3proc.c
+++ b/fs/nfsd/nfs3proc.c
@@ -344,7 +344,7 @@ nfsd3_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
 	status = fh_fill_pre_attrs(fhp);
 	if (status != nfs_ok)
 		goto out;
-	host_err = vfs_create(&nop_mnt_idmap, inode, child, iap->ia_mode, true);
+	host_err = vfs_create(&nop_mnt_idmap, inode, child, iap->ia_mode, true, NULL);
 	if (host_err < 0) {
 		status = nfserrno(host_err);
 		goto out;
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 7d8cd2595f197be9741ee6320d43ed6651896647..8834bc59f5cfcc88797eb09189b6c12d29e98d10 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -1638,8 +1638,7 @@ nfsd_create_locked(struct svc_rqst *rqstp, struct svc_fh *fhp,
 	err = 0;
 	switch (type) {
 	case S_IFREG:
-		host_err = vfs_create(&nop_mnt_idmap, dirp, dchild,
-				      iap->ia_mode, true);
+		host_err = vfs_create(&nop_mnt_idmap, dirp, dchild, iap->ia_mode, true, NULL);
 		if (!host_err)
 			nfsd_check_ignore_resizing(iap);
 		break;
diff --git a/fs/open.c b/fs/open.c
index 3d64372ecc675e4795eb0a0deda10f8f67b95640..4d98f8b52b98bc95e52cb247d14871ff6e4a1b5c 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -1173,7 +1173,7 @@ struct file *dentry_create(const struct path *path, int flags, umode_t mode,
 
 	error = vfs_create(mnt_idmap(path->mnt),
 			   d_inode(path->dentry->d_parent),
-			   path->dentry, mode, true);
+			   path->dentry, mode, true, NULL);
 	if (!error)
 		error = vfs_open(path, f);
 
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index d215d7349489686b66bb66e939b27046f7d836f6..d3123f5d97e86b58e4c9608cf6ef2abd1fcddbcd 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -235,7 +235,7 @@ static inline int ovl_do_create(struct ovl_fs *ofs,
 				struct inode *dir, struct dentry *dentry,
 				umode_t mode)
 {
-	int err = vfs_create(ovl_upper_mnt_idmap(ofs), dir, dentry, mode, true);
+	int err = vfs_create(ovl_upper_mnt_idmap(ofs), dir, dentry, mode, true, NULL);
 
 	pr_debug("create(%pd2, 0%o) = %i\n", dentry, mode, err);
 	return err;
diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c
index c5f0f3170d586cb2dc4d416b80948c642797fb82..be278bb6b71bab8aa41aed06a8806e7bc2de4cd3 100644
--- a/fs/smb/server/vfs.c
+++ b/fs/smb/server/vfs.c
@@ -189,7 +189,7 @@ int ksmbd_vfs_create(struct ksmbd_work *work, const char *name, umode_t mode)
 
 	mode |= S_IFREG;
 	err = vfs_create(mnt_idmap(path.mnt), d_inode(path.dentry),
-			 dentry, mode, true);
+			 dentry, mode, true, NULL);
 	if (!err) {
 		ksmbd_vfs_inherit_owner(work, d_inode(path.dentry),
 					d_inode(dentry));
diff --git a/include/linux/fs.h b/include/linux/fs.h
index d8bdaf7c87502ff17775602f5391d375738b4ed8..5fcf64d9cf42ce135c0fbcbf6dfbf8816ae0bcb1 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2111,7 +2111,7 @@ bool inode_owner_or_capable(struct mnt_idmap *idmap,
  * VFS helper functions..
  */
 int vfs_create(struct mnt_idmap *, struct inode *,
-	       struct dentry *, umode_t, bool);
+	       struct dentry *, umode_t, bool, struct inode **);
 struct dentry *vfs_mkdir(struct mnt_idmap *, struct inode *,
 			 struct dentry *, umode_t, struct inode **);
 int vfs_mknod(struct mnt_idmap *, struct inode *, struct dentry *,

-- 
2.51.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 07/11] vfs: make vfs_mknod break delegations on parent directory
  2025-10-17 11:31 [PATCH v2 00/11] vfs: recall-only directory delegations for knfsd Jeff Layton
                   ` (5 preceding siblings ...)
  2025-10-17 11:31 ` [PATCH v2 06/11] vfs: make vfs_create break delegations on parent directory Jeff Layton
@ 2025-10-17 11:31 ` Jeff Layton
  2025-10-17 11:32 ` [PATCH v2 08/11] filelock: lift the ban on directory leases in generic_setlease Jeff Layton
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: Jeff Layton @ 2025-10-17 11:31 UTC (permalink / raw)
  To: Miklos Szeredi, Alexander Viro, Christian Brauner, Jan Kara,
	Chuck Lever, Alexander Aring, Trond Myklebust, Anna Schumaker,
	Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
	Tom Talpey, Bharath SM, Greg Kroah-Hartman, Rafael J. Wysocki,
	Danilo Krummrich, David Howells, Tyler Hicks, NeilBrown,
	Olga Kornievskaia, Dai Ngo, Amir Goldstein, Namjae Jeon,
	Steve French, Sergey Senozhatsky, Carlos Maiolino,
	Kuniyuki Iwashima, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman
  Cc: linux-fsdevel, linux-kernel, linux-nfs, linux-cifs,
	samba-technical, netfs, ecryptfs, linux-unionfs, linux-xfs,
	netdev, Jeff Layton

In order to add directory delegation support, we need to break
delegations on the parent whenever there is going to be a change in the
directory.

Add a new delegated_inode return pointer to vfs_mknod() and have the
appropriate callers wait when there is an outstanding delegation. All
other callers just set the pointer to NULL.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 drivers/base/devtmpfs.c  |  2 +-
 fs/ecryptfs/inode.c      |  2 +-
 fs/init.c                |  2 +-
 fs/namei.c               | 25 +++++++++++++++++--------
 fs/nfsd/vfs.c            |  2 +-
 fs/overlayfs/overlayfs.h |  2 +-
 include/linux/fs.h       |  4 ++--
 net/unix/af_unix.c       |  2 +-
 8 files changed, 25 insertions(+), 16 deletions(-)

diff --git a/drivers/base/devtmpfs.c b/drivers/base/devtmpfs.c
index 104025104ef75381984fd94dfbd50feeaa8cdd22..2f576ecf18324f767cd5ac6cbd28adbf9f46b958 100644
--- a/drivers/base/devtmpfs.c
+++ b/drivers/base/devtmpfs.c
@@ -231,7 +231,7 @@ static int handle_create(const char *nodename, umode_t mode, kuid_t uid,
 		return PTR_ERR(dentry);
 
 	err = vfs_mknod(&nop_mnt_idmap, d_inode(path.dentry), dentry, mode,
-			dev->devt);
+			dev->devt, NULL);
 	if (!err) {
 		struct iattr newattrs;
 
diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
index 661709b157ce854c3bfdfdb13f7c10435fad9756..639ae42bcd56890d04592f7269e4ffc099b44f09 100644
--- a/fs/ecryptfs/inode.c
+++ b/fs/ecryptfs/inode.c
@@ -565,7 +565,7 @@ ecryptfs_mknod(struct mnt_idmap *idmap, struct inode *dir,
 	rc = lock_parent(dentry, &lower_dentry, &lower_dir);
 	if (!rc)
 		rc = vfs_mknod(&nop_mnt_idmap, lower_dir,
-			       lower_dentry, mode, dev);
+			       lower_dentry, mode, dev, NULL);
 	if (rc || d_really_is_negative(lower_dentry))
 		goto out;
 	rc = ecryptfs_interpose(lower_dentry, dentry, dir->i_sb);
diff --git a/fs/init.c b/fs/init.c
index 895f8a09a71acfd03e11164e3b441a7d4e2de146..4f02260dd65b0dfcbfbf5812d2ec6a33444a3b1f 100644
--- a/fs/init.c
+++ b/fs/init.c
@@ -157,7 +157,7 @@ int __init init_mknod(const char *filename, umode_t mode, unsigned int dev)
 	error = security_path_mknod(&path, dentry, mode, dev);
 	if (!error)
 		error = vfs_mknod(mnt_idmap(path.mnt), path.dentry->d_inode,
-				  dentry, mode, new_decode_dev(dev));
+				  dentry, mode, new_decode_dev(dev), NULL);
 	end_creating_path(&path, dentry);
 	return error;
 }
diff --git a/fs/namei.c b/fs/namei.c
index 7510942e0249de19df4363b92f813b3acdfc2254..7e400cbdbc6af1c72eb684f051d0571e944a27d7 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -4297,13 +4297,15 @@ inline struct dentry *start_creating_user_path(
 }
 EXPORT_SYMBOL(start_creating_user_path);
 
+
 /**
  * vfs_mknod - create device node or file
- * @idmap:	idmap of the mount the inode was found from
- * @dir:	inode of the parent directory
- * @dentry:	dentry of the child device node
- * @mode:	mode of the child device node
- * @dev:	device number of device to create
+ * @idmap:		idmap of the mount the inode was found from
+ * @dir:		inode of the parent directory
+ * @dentry:		dentry of the child device node
+ * @mode:		mode of the child device node
+ * @dev:		device number of device to create
+ * @delegated_inode:	returns parent inode, if the inode is delegated.
  *
  * Create a device node or file.
  *
@@ -4314,7 +4316,8 @@ EXPORT_SYMBOL(start_creating_user_path);
  * raw inode simply pass @nop_mnt_idmap.
  */
 int vfs_mknod(struct mnt_idmap *idmap, struct inode *dir,
-	      struct dentry *dentry, umode_t mode, dev_t dev)
+	      struct dentry *dentry, umode_t mode, dev_t dev,
+	      struct inode **delegated_inode)
 {
 	bool is_whiteout = S_ISCHR(mode) && dev == WHITEOUT_DEV;
 	int error = may_create(idmap, dir, dentry);
@@ -4338,6 +4341,10 @@ int vfs_mknod(struct mnt_idmap *idmap, struct inode *dir,
 	if (error)
 		return error;
 
+	error = try_break_deleg(dir, delegated_inode);
+	if (error)
+		return error;
+
 	error = dir->i_op->mknod(idmap, dir, dentry, mode, dev);
 	if (!error)
 		fsnotify_create(dir, dentry);
@@ -4397,11 +4404,13 @@ static int do_mknodat(int dfd, struct filename *name, umode_t mode,
 			break;
 		case S_IFCHR: case S_IFBLK:
 			error = vfs_mknod(idmap, path.dentry->d_inode,
-					  dentry, mode, new_decode_dev(dev));
+					  dentry, mode, new_decode_dev(dev),
+					  &delegated_inode);
 			break;
 		case S_IFIFO: case S_IFSOCK:
 			error = vfs_mknod(idmap, path.dentry->d_inode,
-					  dentry, mode, 0);
+					  dentry, mode, 0,
+					  &delegated_inode);
 			break;
 	}
 out2:
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 8834bc59f5cfcc88797eb09189b6c12d29e98d10..eeb138569eba5df6de361cf6ba29604722e14af9 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -1659,7 +1659,7 @@ nfsd_create_locked(struct svc_rqst *rqstp, struct svc_fh *fhp,
 	case S_IFIFO:
 	case S_IFSOCK:
 		host_err = vfs_mknod(&nop_mnt_idmap, dirp, dchild,
-				     iap->ia_mode, rdev);
+				     iap->ia_mode, rdev, NULL);
 		break;
 	default:
 		printk(KERN_WARNING "nfsd: bad file type %o in nfsd_create\n",
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index d3123f5d97e86b58e4c9608cf6ef2abd1fcddbcd..87b82dada7ec1b8429299c68078cda24176c5607 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -257,7 +257,7 @@ static inline int ovl_do_mknod(struct ovl_fs *ofs,
 			       struct inode *dir, struct dentry *dentry,
 			       umode_t mode, dev_t dev)
 {
-	int err = vfs_mknod(ovl_upper_mnt_idmap(ofs), dir, dentry, mode, dev);
+	int err = vfs_mknod(ovl_upper_mnt_idmap(ofs), dir, dentry, mode, dev, NULL);
 
 	pr_debug("mknod(%pd2, 0%o, 0%o) = %i\n", dentry, mode, dev, err);
 	return err;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 5fcf64d9cf42ce135c0fbcbf6dfbf8816ae0bcb1..a1e1afe39e01a46bf0a81e241b92690947402851 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2115,7 +2115,7 @@ int vfs_create(struct mnt_idmap *, struct inode *,
 struct dentry *vfs_mkdir(struct mnt_idmap *, struct inode *,
 			 struct dentry *, umode_t, struct inode **);
 int vfs_mknod(struct mnt_idmap *, struct inode *, struct dentry *,
-              umode_t, dev_t);
+	      umode_t, dev_t, struct inode **);
 int vfs_symlink(struct mnt_idmap *, struct inode *,
 		struct dentry *, const char *);
 int vfs_link(struct dentry *, struct mnt_idmap *, struct inode *,
@@ -2151,7 +2151,7 @@ static inline int vfs_whiteout(struct mnt_idmap *idmap,
 			       struct inode *dir, struct dentry *dentry)
 {
 	return vfs_mknod(idmap, dir, dentry, S_IFCHR | WHITEOUT_MODE,
-			 WHITEOUT_DEV);
+			 WHITEOUT_DEV, NULL);
 }
 
 struct file *kernel_tmpfile_open(struct mnt_idmap *idmap,
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 768098dec2310008632558ae928703b37c3cc8ef..db1fd8d6a84c2c7c0d45b43d9c5a936b3d491b7b 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -1399,7 +1399,7 @@ static int unix_bind_bsd(struct sock *sk, struct sockaddr_un *sunaddr,
 	idmap = mnt_idmap(parent.mnt);
 	err = security_path_mknod(&parent, dentry, mode, 0);
 	if (!err)
-		err = vfs_mknod(idmap, d_inode(parent.dentry), dentry, mode, 0);
+		err = vfs_mknod(idmap, d_inode(parent.dentry), dentry, mode, 0, NULL);
 	if (err)
 		goto out_path;
 	err = mutex_lock_interruptible(&u->bindlock);

-- 
2.51.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 08/11] filelock: lift the ban on directory leases in generic_setlease
  2025-10-17 11:31 [PATCH v2 00/11] vfs: recall-only directory delegations for knfsd Jeff Layton
                   ` (6 preceding siblings ...)
  2025-10-17 11:31 ` [PATCH v2 07/11] vfs: make vfs_mknod " Jeff Layton
@ 2025-10-17 11:32 ` Jeff Layton
  2025-10-17 11:32 ` [PATCH v2 09/11] nfsd: allow filecache to hold S_IFDIR files Jeff Layton
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: Jeff Layton @ 2025-10-17 11:32 UTC (permalink / raw)
  To: Miklos Szeredi, Alexander Viro, Christian Brauner, Jan Kara,
	Chuck Lever, Alexander Aring, Trond Myklebust, Anna Schumaker,
	Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
	Tom Talpey, Bharath SM, Greg Kroah-Hartman, Rafael J. Wysocki,
	Danilo Krummrich, David Howells, Tyler Hicks, NeilBrown,
	Olga Kornievskaia, Dai Ngo, Amir Goldstein, Namjae Jeon,
	Steve French, Sergey Senozhatsky, Carlos Maiolino,
	Kuniyuki Iwashima, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman
  Cc: linux-fsdevel, linux-kernel, linux-nfs, linux-cifs,
	samba-technical, netfs, ecryptfs, linux-unionfs, linux-xfs,
	netdev, Jeff Layton

With the addition of the try_break_lease calls in directory changing
operations, allow generic_setlease to hand them out.

Note that this also makes directory leases available to userland via
fcntl().

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/locks.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/fs/locks.c b/fs/locks.c
index 0b16921fb52e602ea2e0c3de39d9d772af98ba7d..b137cab18d362130cda5d1f0cdf50ba7e9aa2e12 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -1929,14 +1929,19 @@ static int generic_delete_lease(struct file *filp, void *owner)
 int generic_setlease(struct file *filp, int arg, struct file_lease **flp,
 			void **priv)
 {
-	if (!S_ISREG(file_inode(filp)->i_mode))
+	struct inode *inode = file_inode(filp);
+
+	if (!S_ISREG(inode->i_mode) && !S_ISDIR(inode->i_mode))
 		return -EINVAL;
 
 	switch (arg) {
 	case F_UNLCK:
 		return generic_delete_lease(filp, *priv);
-	case F_RDLCK:
 	case F_WRLCK:
+		if (S_ISDIR(inode->i_mode))
+			return -EINVAL;
+		fallthrough;
+	case F_RDLCK:
 		if (!(*flp)->fl_lmops->lm_break) {
 			WARN_ON_ONCE(1);
 			return -ENOLCK;

-- 
2.51.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 09/11] nfsd: allow filecache to hold S_IFDIR files
  2025-10-17 11:31 [PATCH v2 00/11] vfs: recall-only directory delegations for knfsd Jeff Layton
                   ` (7 preceding siblings ...)
  2025-10-17 11:32 ` [PATCH v2 08/11] filelock: lift the ban on directory leases in generic_setlease Jeff Layton
@ 2025-10-17 11:32 ` Jeff Layton
  2025-10-18 19:33   ` Chuck Lever
  2025-10-17 11:32 ` [PATCH v2 10/11] nfsd: allow DELEGRETURN on directories Jeff Layton
                   ` (3 subsequent siblings)
  12 siblings, 1 reply; 20+ messages in thread
From: Jeff Layton @ 2025-10-17 11:32 UTC (permalink / raw)
  To: Miklos Szeredi, Alexander Viro, Christian Brauner, Jan Kara,
	Chuck Lever, Alexander Aring, Trond Myklebust, Anna Schumaker,
	Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
	Tom Talpey, Bharath SM, Greg Kroah-Hartman, Rafael J. Wysocki,
	Danilo Krummrich, David Howells, Tyler Hicks, NeilBrown,
	Olga Kornievskaia, Dai Ngo, Amir Goldstein, Namjae Jeon,
	Steve French, Sergey Senozhatsky, Carlos Maiolino,
	Kuniyuki Iwashima, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman
  Cc: linux-fsdevel, linux-kernel, linux-nfs, linux-cifs,
	samba-technical, netfs, ecryptfs, linux-unionfs, linux-xfs,
	netdev, Jeff Layton

The filecache infrastructure will only handle S_IFREG files at the
moment. Directory delegations will require adding support for opening
S_IFDIR inodes.

Plumb a "type" argument into nfsd_file_do_acquire() and have all of the
existing callers set it to S_IFREG. Add a new nfsd_file_acquire_dir()
wrapper that nfsd can call to request a nfsd_file that holds a directory
open.

For now, there is no need for a fsnotify_mark for directories, as
CB_NOTIFY is not yet supported. Change nfsd_file_do_acquire() to avoid
allocating one for non-S_IFREG inodes.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/filecache.c | 57 ++++++++++++++++++++++++++++++++++++++++-------------
 fs/nfsd/filecache.h |  2 ++
 fs/nfsd/vfs.c       |  5 +++--
 fs/nfsd/vfs.h       |  2 +-
 4 files changed, 49 insertions(+), 17 deletions(-)

diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c
index a238b6725008a5c2988bd3da874d1f34ee778437..93798575b8075c63f95cd415b6d24df706ada0f6 100644
--- a/fs/nfsd/filecache.c
+++ b/fs/nfsd/filecache.c
@@ -1086,7 +1086,7 @@ nfsd_file_do_acquire(struct svc_rqst *rqstp, struct net *net,
 		     struct auth_domain *client,
 		     struct svc_fh *fhp,
 		     unsigned int may_flags, struct file *file,
-		     struct nfsd_file **pnf, bool want_gc)
+		     umode_t type, bool want_gc, struct nfsd_file **pnf)
 {
 	unsigned char need = may_flags & NFSD_FILE_MAY_MASK;
 	struct nfsd_file *new, *nf;
@@ -1097,13 +1097,13 @@ nfsd_file_do_acquire(struct svc_rqst *rqstp, struct net *net,
 	int ret;
 
 retry:
-	if (rqstp) {
-		status = fh_verify(rqstp, fhp, S_IFREG,
+	if (rqstp)
+		status = fh_verify(rqstp, fhp, type,
 				   may_flags|NFSD_MAY_OWNER_OVERRIDE);
-	} else {
-		status = fh_verify_local(net, cred, client, fhp, S_IFREG,
+	else
+		status = fh_verify_local(net, cred, client, fhp, type,
 					 may_flags|NFSD_MAY_OWNER_OVERRIDE);
-	}
+
 	if (status != nfs_ok)
 		return status;
 	inode = d_inode(fhp->fh_dentry);
@@ -1176,15 +1176,18 @@ nfsd_file_do_acquire(struct svc_rqst *rqstp, struct net *net,
 
 open_file:
 	trace_nfsd_file_alloc(nf);
-	nf->nf_mark = nfsd_file_mark_find_or_create(inode);
-	if (nf->nf_mark) {
+
+	if (type == S_IFREG)
+		nf->nf_mark = nfsd_file_mark_find_or_create(inode);
+
+	if (type != S_IFREG || nf->nf_mark) {
 		if (file) {
 			get_file(file);
 			nf->nf_file = file;
 			status = nfs_ok;
 			trace_nfsd_file_opened(nf, status);
 		} else {
-			ret = nfsd_open_verified(fhp, may_flags, &nf->nf_file);
+			ret = nfsd_open_verified(fhp, type, may_flags, &nf->nf_file);
 			if (ret == -EOPENSTALE && stale_retry) {
 				stale_retry = false;
 				nfsd_file_unhash(nf);
@@ -1246,7 +1249,7 @@ nfsd_file_acquire_gc(struct svc_rqst *rqstp, struct svc_fh *fhp,
 		     unsigned int may_flags, struct nfsd_file **pnf)
 {
 	return nfsd_file_do_acquire(rqstp, SVC_NET(rqstp), NULL, NULL,
-				    fhp, may_flags, NULL, pnf, true);
+				    fhp, may_flags, NULL, S_IFREG, true, pnf);
 }
 
 /**
@@ -1271,7 +1274,7 @@ nfsd_file_acquire(struct svc_rqst *rqstp, struct svc_fh *fhp,
 		  unsigned int may_flags, struct nfsd_file **pnf)
 {
 	return nfsd_file_do_acquire(rqstp, SVC_NET(rqstp), NULL, NULL,
-				    fhp, may_flags, NULL, pnf, false);
+				    fhp, may_flags, NULL, S_IFREG, false, pnf);
 }
 
 /**
@@ -1314,8 +1317,8 @@ nfsd_file_acquire_local(struct net *net, struct svc_cred *cred,
 	const struct cred *save_cred = get_current_cred();
 	__be32 beres;
 
-	beres = nfsd_file_do_acquire(NULL, net, cred, client,
-				     fhp, may_flags, NULL, pnf, false);
+	beres = nfsd_file_do_acquire(NULL, net, cred, client, fhp, may_flags,
+				     NULL, S_IFREG, false, pnf);
 	put_cred(revert_creds(save_cred));
 	return beres;
 }
@@ -1344,7 +1347,33 @@ nfsd_file_acquire_opened(struct svc_rqst *rqstp, struct svc_fh *fhp,
 			 struct nfsd_file **pnf)
 {
 	return nfsd_file_do_acquire(rqstp, SVC_NET(rqstp), NULL, NULL,
-				    fhp, may_flags, file, pnf, false);
+				    fhp, may_flags, file, S_IFREG, false, pnf);
+}
+
+/**
+ * nfsd_file_acquire_dir - Get a struct nfsd_file with an open directory
+ * @rqstp: the RPC transaction being executed
+ * @fhp: the NFS filehandle of the file to be opened
+ * @pnf: OUT: new or found "struct nfsd_file" object
+ *
+ * The nfsd_file_object returned by this API is reference-counted
+ * but not garbage-collected. The object is unhashed after the
+ * final nfsd_file_put(). This opens directories only, and only
+ * in O_RDONLY mode.
+ *
+ * Return values:
+ *   %nfs_ok - @pnf points to an nfsd_file with its reference
+ *   count boosted.
+ *
+ * On error, an nfsstat value in network byte order is returned.
+ */
+__be32
+nfsd_file_acquire_dir(struct svc_rqst *rqstp, struct svc_fh *fhp,
+		      struct nfsd_file **pnf)
+{
+	return nfsd_file_do_acquire(rqstp, SVC_NET(rqstp), NULL, NULL, fhp,
+				    NFSD_MAY_READ|NFSD_MAY_64BIT_COOKIE,
+				    NULL, S_IFDIR, false, pnf);
 }
 
 /*
diff --git a/fs/nfsd/filecache.h b/fs/nfsd/filecache.h
index e3d6ca2b60308e5e91ba4bb32d935f54527d8bda..b383dbc5b9218d21a29b852572f80fab08de9fa9 100644
--- a/fs/nfsd/filecache.h
+++ b/fs/nfsd/filecache.h
@@ -82,5 +82,7 @@ __be32 nfsd_file_acquire_opened(struct svc_rqst *rqstp, struct svc_fh *fhp,
 __be32 nfsd_file_acquire_local(struct net *net, struct svc_cred *cred,
 			       struct auth_domain *client, struct svc_fh *fhp,
 			       unsigned int may_flags, struct nfsd_file **pnf);
+__be32 nfsd_file_acquire_dir(struct svc_rqst *rqstp, struct svc_fh *fhp,
+		  struct nfsd_file **pnf);
 int nfsd_file_cache_stats_show(struct seq_file *m, void *v);
 #endif /* _FS_NFSD_FILECACHE_H */
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index eeb138569eba5df6de361cf6ba29604722e14af9..12c33223b612664dbb3b18b591e97fc708165763 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -959,15 +959,16 @@ nfsd_open(struct svc_rqst *rqstp, struct svc_fh *fhp, umode_t type,
 /**
  * nfsd_open_verified - Open a regular file for the filecache
  * @fhp: NFS filehandle of the file to open
+ * @type: S_IFMT inode type allowed (0 means any type is allowed)
  * @may_flags: internal permission flags
  * @filp: OUT: open "struct file *"
  *
  * Returns zero on success, or a negative errno value.
  */
 int
-nfsd_open_verified(struct svc_fh *fhp, int may_flags, struct file **filp)
+nfsd_open_verified(struct svc_fh *fhp, umode_t type, int may_flags, struct file **filp)
 {
-	return __nfsd_open(fhp, S_IFREG, may_flags, filp);
+	return __nfsd_open(fhp, type, may_flags, filp);
 }
 
 /*
diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
index fa46f8b5f132079e3a2c45e71ecf9cc43181f6b0..ded2900d423f80d33fb6c8b809bc5d9fc842ebfd 100644
--- a/fs/nfsd/vfs.h
+++ b/fs/nfsd/vfs.h
@@ -114,7 +114,7 @@ __be32		nfsd_setxattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
 int 		nfsd_open_break_lease(struct inode *, int);
 __be32		nfsd_open(struct svc_rqst *, struct svc_fh *, umode_t,
 				int, struct file **);
-int		nfsd_open_verified(struct svc_fh *fhp, int may_flags,
+int		nfsd_open_verified(struct svc_fh *fhp, umode_t type, int may_flags,
 				struct file **filp);
 __be32		nfsd_splice_read(struct svc_rqst *rqstp, struct svc_fh *fhp,
 				struct file *file, loff_t offset,

-- 
2.51.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 10/11] nfsd: allow DELEGRETURN on directories
  2025-10-17 11:31 [PATCH v2 00/11] vfs: recall-only directory delegations for knfsd Jeff Layton
                   ` (8 preceding siblings ...)
  2025-10-17 11:32 ` [PATCH v2 09/11] nfsd: allow filecache to hold S_IFDIR files Jeff Layton
@ 2025-10-17 11:32 ` Jeff Layton
  2025-10-18 19:37   ` Chuck Lever
  2025-10-17 11:32 ` [PATCH v2 11/11] nfsd: wire up GET_DIR_DELEGATION handling Jeff Layton
                   ` (2 subsequent siblings)
  12 siblings, 1 reply; 20+ messages in thread
From: Jeff Layton @ 2025-10-17 11:32 UTC (permalink / raw)
  To: Miklos Szeredi, Alexander Viro, Christian Brauner, Jan Kara,
	Chuck Lever, Alexander Aring, Trond Myklebust, Anna Schumaker,
	Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
	Tom Talpey, Bharath SM, Greg Kroah-Hartman, Rafael J. Wysocki,
	Danilo Krummrich, David Howells, Tyler Hicks, NeilBrown,
	Olga Kornievskaia, Dai Ngo, Amir Goldstein, Namjae Jeon,
	Steve French, Sergey Senozhatsky, Carlos Maiolino,
	Kuniyuki Iwashima, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman
  Cc: linux-fsdevel, linux-kernel, linux-nfs, linux-cifs,
	samba-technical, netfs, ecryptfs, linux-unionfs, linux-xfs,
	netdev, Jeff Layton

As Trond pointed out: "...provided that the presented stateid is
actually valid, it is also sufficient to uniquely identify the file to
which it is associated (see RFC8881 Section 8.2.4), so the filehandle
should be considered mostly irrelevant for operations like DELEGRETURN."

Don't ask fh_verify to filter on file type.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/nfs4state.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index c9053ef4d79f074f49ecaf0c7a3db78ec147136e..b06591f154aa372db710e071c69260f4639956d7 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -7824,7 +7824,8 @@ nfsd4_delegreturn(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	__be32 status;
 	struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
 
-	if ((status = fh_verify(rqstp, &cstate->current_fh, S_IFREG, 0)))
+	status = fh_verify(rqstp, &cstate->current_fh, 0, 0);
+	if (status)
 		return status;
 
 	status = nfsd4_lookup_stateid(cstate, stateid, SC_TYPE_DELEG, SC_STATUS_REVOKED, &s, nn);

-- 
2.51.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 11/11] nfsd: wire up GET_DIR_DELEGATION handling
  2025-10-17 11:31 [PATCH v2 00/11] vfs: recall-only directory delegations for knfsd Jeff Layton
                   ` (9 preceding siblings ...)
  2025-10-17 11:32 ` [PATCH v2 10/11] nfsd: allow DELEGRETURN on directories Jeff Layton
@ 2025-10-17 11:32 ` Jeff Layton
  2025-10-18 19:46   ` Chuck Lever
  2025-10-17 23:44 ` [PATCH v2 00/11] vfs: recall-only directory delegations for knfsd NeilBrown
  2025-10-21 13:09 ` Christian Brauner
  12 siblings, 1 reply; 20+ messages in thread
From: Jeff Layton @ 2025-10-17 11:32 UTC (permalink / raw)
  To: Miklos Szeredi, Alexander Viro, Christian Brauner, Jan Kara,
	Chuck Lever, Alexander Aring, Trond Myklebust, Anna Schumaker,
	Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
	Tom Talpey, Bharath SM, Greg Kroah-Hartman, Rafael J. Wysocki,
	Danilo Krummrich, David Howells, Tyler Hicks, NeilBrown,
	Olga Kornievskaia, Dai Ngo, Amir Goldstein, Namjae Jeon,
	Steve French, Sergey Senozhatsky, Carlos Maiolino,
	Kuniyuki Iwashima, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman
  Cc: linux-fsdevel, linux-kernel, linux-nfs, linux-cifs,
	samba-technical, netfs, ecryptfs, linux-unionfs, linux-xfs,
	netdev, Jeff Layton

Add a new routine for acquiring a read delegation on a directory. These
are recallable-only delegations with no support for CB_NOTIFY. That will
be added in a later phase.

Since the same CB_RECALL/DELEGRETURN infrastrure is used for regular and
directory delegations, a normal nfs4_delegation is used to represent a
directory delegation.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/nfs4proc.c  |  21 ++++++++++-
 fs/nfsd/nfs4state.c | 100 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/nfsd/state.h     |   5 +++
 3 files changed, 125 insertions(+), 1 deletion(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 7f7e6bb23a90d9a1cafd154c0f09e236df75b083..527f8dc52159803770964700170473509ec328ed 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -2342,6 +2342,13 @@ nfsd4_get_dir_delegation(struct svc_rqst *rqstp,
 			 union nfsd4_op_u *u)
 {
 	struct nfsd4_get_dir_delegation *gdd = &u->get_dir_delegation;
+	struct nfs4_delegation *dd;
+	struct nfsd_file *nf;
+	__be32 status;
+
+	status = nfsd_file_acquire_dir(rqstp, &cstate->current_fh, &nf);
+	if (status != nfs_ok)
+		return status;
 
 	/*
 	 * RFC 8881, section 18.39.3 says:
@@ -2355,7 +2362,19 @@ nfsd4_get_dir_delegation(struct svc_rqst *rqstp,
 	 * return NFS4_OK with a non-fatal status of GDD4_UNAVAIL in this
 	 * situation.
 	 */
-	gdd->gddrnf_status = GDD4_UNAVAIL;
+	dd = nfsd_get_dir_deleg(cstate, gdd, nf);
+	if (IS_ERR(dd)) {
+		int err = PTR_ERR(dd);
+
+		if (err != -EAGAIN)
+			return nfserrno(err);
+		gdd->gddrnf_status = GDD4_UNAVAIL;
+		return nfs_ok;
+	}
+
+	gdd->gddrnf_status = GDD4_OK;
+	memcpy(&gdd->gddr_stateid, &dd->dl_stid.sc_stateid, sizeof(gdd->gddr_stateid));
+	nfs4_put_stid(&dd->dl_stid);
 	return nfs_ok;
 }
 
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index b06591f154aa372db710e071c69260f4639956d7..a63e8c885291fc377163f3255f26f5f693704437 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -9359,3 +9359,103 @@ nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, struct dentry *dentry,
 	nfs4_put_stid(&dp->dl_stid);
 	return status;
 }
+
+/**
+ * nfsd_get_dir_deleg - attempt to get a directory delegation
+ * @cstate: compound state
+ * @gdd: GET_DIR_DELEGATION arg/resp structure
+ * @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.
+ */
+struct nfs4_delegation *
+nfsd_get_dir_deleg(struct nfsd4_compound_state *cstate,
+		   struct nfsd4_get_dir_delegation *gdd,
+		   struct nfsd_file *nf)
+{
+	struct nfs4_client *clp = cstate->clp;
+	struct nfs4_delegation *dp;
+	struct file_lease *fl;
+	struct nfs4_file *fp, *rfp;
+	int status = 0;
+
+	fp = nfsd4_alloc_file();
+	if (!fp)
+		return ERR_PTR(-ENOMEM);
+
+	nfsd4_file_init(&cstate->current_fh, fp);
+
+	rfp = nfsd4_file_hash_insert(fp, &cstate->current_fh);
+	if (unlikely(!rfp)) {
+		put_nfs4_file(fp);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	if (rfp != fp) {
+		put_nfs4_file(fp);
+		fp = rfp;
+	}
+
+	/* if this client already has one, return that it's unavailable */
+	spin_lock(&state_lock);
+	spin_lock(&fp->fi_lock);
+	/* existing delegation? */
+	if (nfs4_delegation_exists(clp, fp)) {
+		status = -EAGAIN;
+	} else if (!fp->fi_deleg_file) {
+		fp->fi_deleg_file = nf;
+		fp->fi_delegees = 1;
+	} else {
+		++fp->fi_delegees;
+	}
+	spin_unlock(&fp->fi_lock);
+	spin_unlock(&state_lock);
+
+	if (status) {
+		put_nfs4_file(fp);
+		return ERR_PTR(status);
+	}
+
+	/* Try to set up the lease */
+	status = -ENOMEM;
+	dp = alloc_init_deleg(clp, fp, NULL, NFS4_OPEN_DELEGATE_READ);
+	if (!dp)
+		goto out_delegees;
+
+	fl = nfs4_alloc_init_lease(dp);
+	if (!fl)
+		goto out_put_stid;
+
+	status = kernel_setlease(nf->nf_file,
+				 fl->c.flc_type, &fl, NULL);
+	if (fl)
+		locks_free_lease(fl);
+	if (status)
+		goto out_put_stid;
+
+	/*
+	 * Now, try to hash it. This can fail if we race another nfsd task
+	 * trying to set a delegation on the same file. If that happens,
+	 * then just say UNAVAIL.
+	 */
+	spin_lock(&state_lock);
+	spin_lock(&clp->cl_lock);
+	spin_lock(&fp->fi_lock);
+	status = hash_delegation_locked(dp, fp);
+	spin_unlock(&fp->fi_lock);
+	spin_unlock(&clp->cl_lock);
+	spin_unlock(&state_lock);
+
+	if (!status)
+		return dp;
+
+	/* Something failed. Drop the lease and clean up the stid */
+	kernel_setlease(fp->fi_deleg_file->nf_file, F_UNLCK, NULL, (void **)&dp);
+out_put_stid:
+	nfs4_put_stid(&dp->dl_stid);
+out_delegees:
+	put_deleg_file(fp);
+	return ERR_PTR(status);
+}
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 1e736f4024263ffa9c93bcc9ec48f44566a8cc77..b052c1effdc5356487c610db9728df8ecfe851d4 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -867,4 +867,9 @@ static inline bool try_to_expire_client(struct nfs4_client *clp)
 
 extern __be32 nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp,
 		struct dentry *dentry, struct nfs4_delegation **pdp);
+
+struct nfsd4_get_dir_delegation;
+struct nfs4_delegation *nfsd_get_dir_deleg(struct nfsd4_compound_state *cstate,
+						struct nfsd4_get_dir_delegation *gdd,
+						struct nfsd_file *nf);
 #endif   /* NFSD4_STATE_H */

-- 
2.51.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 00/11] vfs: recall-only directory delegations for knfsd
  2025-10-17 11:31 [PATCH v2 00/11] vfs: recall-only directory delegations for knfsd Jeff Layton
                   ` (10 preceding siblings ...)
  2025-10-17 11:32 ` [PATCH v2 11/11] nfsd: wire up GET_DIR_DELEGATION handling Jeff Layton
@ 2025-10-17 23:44 ` NeilBrown
  2025-10-19 14:17   ` Jeff Layton
  2025-10-21 13:09 ` Christian Brauner
  12 siblings, 1 reply; 20+ messages in thread
From: NeilBrown @ 2025-10-17 23:44 UTC (permalink / raw)
  To: Jeff Layton
  Cc: Miklos Szeredi, Alexander Viro, Christian Brauner, Jan Kara,
	Chuck Lever, Alexander Aring, Trond Myklebust, Anna Schumaker,
	Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
	Tom Talpey, Bharath SM, Greg Kroah-Hartman, Rafael J. Wysocki,
	Danilo Krummrich, David Howells, Tyler Hicks, Olga Kornievskaia,
	Dai Ngo, Amir Goldstein, Namjae Jeon, Steve French,
	Sergey Senozhatsky, Carlos Maiolino, Kuniyuki Iwashima,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, linux-fsdevel, linux-kernel, linux-nfs, linux-cifs,
	samba-technical, netfs, ecryptfs, linux-unionfs, linux-xfs,
	netdev, Jeff Layton

On Fri, 17 Oct 2025, Jeff Layton wrote:
> A smaller variation of the v1 patchset that I posted earlier this week.
> Neil's review inspired me to get rid of the lm_may_setlease operation
> and to do the conflict resolution internally inside of nfsd. That means
> a smaller VFS-layer change, and an overall reduction in code.
> 
> This patchset adds support for directory delegations to nfsd. This
> version only supports recallable delegations. There is no CB_NOTIFY
> support yet. I have patches for those, but we've decided to add that
> support in a later kernel once we get some experience with this part.
> Anna is working on the client-side pieces.
> 
> It would be great if we could get into linux-next soon so that it can be
> merged for v6.19. Christian, could you pick up the vfs/filelock patches,
> and Chuck pick up the nfsd patches?
> 
> Thanks!
> Jeff
> 
> [1]: https://lore.kernel.org/all/20240315-dir-deleg-v1-0-a1d6209a3654@kernel.org/
> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
> Changes in v2:
> - handle lease conflict resolution inside of nfsd
> - drop the lm_may_setlease lock_manager operation
> - just add extra argument to vfs_create() instead of creating wrapper
> - don't allocate fsnotify_mark for open directories
> - Link to v1: https://lore.kernel.org/r/20251013-dir-deleg-ro-v1-0-406780a70e5e@kernel.org
> 
> ---
> Jeff Layton (11):
>       filelock: push the S_ISREG check down to ->setlease handlers
>       vfs: add try_break_deleg calls for parents to vfs_{link,rename,unlink}
>       vfs: allow mkdir to wait for delegation break on parent
>       vfs: allow rmdir to wait for delegation break on parent
>       vfs: break parent dir delegations in open(..., O_CREAT) codepath
>       vfs: make vfs_create break delegations on parent directory
>       vfs: make vfs_mknod break delegations on parent directory
>       filelock: lift the ban on directory leases in generic_setlease
>       nfsd: allow filecache to hold S_IFDIR files
>       nfsd: allow DELEGRETURN on directories
>       nfsd: wire up GET_DIR_DELEGATION handling

vfs_symlink() is missing from the updated APIs.  Surely that needs to be
able to wait for a delegation to break.

vfs_mkobj() maybe does too, but I could easily turn a blind eye to that.

I haven't looked properly at the last patch but all the other could have
 Reviewed-by: NeilBrown <neil@brown.name>

once the vfs_symlink() omission is fixed.

NeilBrown

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 09/11] nfsd: allow filecache to hold S_IFDIR files
  2025-10-17 11:32 ` [PATCH v2 09/11] nfsd: allow filecache to hold S_IFDIR files Jeff Layton
@ 2025-10-18 19:33   ` Chuck Lever
  2025-10-19 13:41     ` Jeff Layton
  0 siblings, 1 reply; 20+ messages in thread
From: Chuck Lever @ 2025-10-18 19:33 UTC (permalink / raw)
  To: Jeff Layton, Miklos Szeredi, Alexander Viro, Christian Brauner,
	Jan Kara, Alexander Aring, Trond Myklebust, Anna Schumaker,
	Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
	Tom Talpey, Bharath SM, Greg Kroah-Hartman, Rafael J. Wysocki,
	Danilo Krummrich, David Howells, Tyler Hicks, NeilBrown,
	Olga Kornievskaia, Dai Ngo, Amir Goldstein, Namjae Jeon,
	Steve French, Sergey Senozhatsky, Carlos Maiolino,
	Kuniyuki Iwashima, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman
  Cc: linux-fsdevel, linux-kernel, linux-nfs, linux-cifs,
	samba-technical, netfs, ecryptfs, linux-unionfs, linux-xfs,
	netdev

On 10/17/25 7:32 AM, Jeff Layton wrote:
> The filecache infrastructure will only handle S_IFREG files at the
> moment. Directory delegations will require adding support for opening
> S_IFDIR inodes.
> 
> Plumb a "type" argument into nfsd_file_do_acquire() and have all of the
> existing callers set it to S_IFREG. Add a new nfsd_file_acquire_dir()
> wrapper that nfsd can call to request a nfsd_file that holds a directory
> open.
> 
> For now, there is no need for a fsnotify_mark for directories, as
> CB_NOTIFY is not yet supported. Change nfsd_file_do_acquire() to avoid
> allocating one for non-S_IFREG inodes.
> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  fs/nfsd/filecache.c | 57 ++++++++++++++++++++++++++++++++++++++++-------------
>  fs/nfsd/filecache.h |  2 ++
>  fs/nfsd/vfs.c       |  5 +++--
>  fs/nfsd/vfs.h       |  2 +-
>  4 files changed, 49 insertions(+), 17 deletions(-)
> 
> diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c
> index a238b6725008a5c2988bd3da874d1f34ee778437..93798575b8075c63f95cd415b6d24df706ada0f6 100644
> --- a/fs/nfsd/filecache.c
> +++ b/fs/nfsd/filecache.c
> @@ -1086,7 +1086,7 @@ nfsd_file_do_acquire(struct svc_rqst *rqstp, struct net *net,
>  		     struct auth_domain *client,
>  		     struct svc_fh *fhp,
>  		     unsigned int may_flags, struct file *file,
> -		     struct nfsd_file **pnf, bool want_gc)
> +		     umode_t type, bool want_gc, struct nfsd_file **pnf)
>  {
>  	unsigned char need = may_flags & NFSD_FILE_MAY_MASK;
>  	struct nfsd_file *new, *nf;
> @@ -1097,13 +1097,13 @@ nfsd_file_do_acquire(struct svc_rqst *rqstp, struct net *net,
>  	int ret;
>  
>  retry:
> -	if (rqstp) {
> -		status = fh_verify(rqstp, fhp, S_IFREG,
> +	if (rqstp)
> +		status = fh_verify(rqstp, fhp, type,
>  				   may_flags|NFSD_MAY_OWNER_OVERRIDE);
> -	} else {
> -		status = fh_verify_local(net, cred, client, fhp, S_IFREG,
> +	else
> +		status = fh_verify_local(net, cred, client, fhp, type,
>  					 may_flags|NFSD_MAY_OWNER_OVERRIDE);
> -	}
> +
>  	if (status != nfs_ok)
>  		return status;
>  	inode = d_inode(fhp->fh_dentry);
> @@ -1176,15 +1176,18 @@ nfsd_file_do_acquire(struct svc_rqst *rqstp, struct net *net,
>  
>  open_file:
>  	trace_nfsd_file_alloc(nf);
> -	nf->nf_mark = nfsd_file_mark_find_or_create(inode);
> -	if (nf->nf_mark) {
> +
> +	if (type == S_IFREG)
> +		nf->nf_mark = nfsd_file_mark_find_or_create(inode);
> +
> +	if (type != S_IFREG || nf->nf_mark) {
>  		if (file) {
>  			get_file(file);
>  			nf->nf_file = file;
>  			status = nfs_ok;
>  			trace_nfsd_file_opened(nf, status);
>  		} else {
> -			ret = nfsd_open_verified(fhp, may_flags, &nf->nf_file);
> +			ret = nfsd_open_verified(fhp, type, may_flags, &nf->nf_file);
>  			if (ret == -EOPENSTALE && stale_retry) {
>  				stale_retry = false;
>  				nfsd_file_unhash(nf);
> @@ -1246,7 +1249,7 @@ nfsd_file_acquire_gc(struct svc_rqst *rqstp, struct svc_fh *fhp,
>  		     unsigned int may_flags, struct nfsd_file **pnf)
>  {
>  	return nfsd_file_do_acquire(rqstp, SVC_NET(rqstp), NULL, NULL,
> -				    fhp, may_flags, NULL, pnf, true);
> +				    fhp, may_flags, NULL, S_IFREG, true, pnf);
>  }
>  
>  /**
> @@ -1271,7 +1274,7 @@ nfsd_file_acquire(struct svc_rqst *rqstp, struct svc_fh *fhp,
>  		  unsigned int may_flags, struct nfsd_file **pnf)
>  {
>  	return nfsd_file_do_acquire(rqstp, SVC_NET(rqstp), NULL, NULL,
> -				    fhp, may_flags, NULL, pnf, false);
> +				    fhp, may_flags, NULL, S_IFREG, false, pnf);
>  }
>  
>  /**
> @@ -1314,8 +1317,8 @@ nfsd_file_acquire_local(struct net *net, struct svc_cred *cred,
>  	const struct cred *save_cred = get_current_cred();
>  	__be32 beres;
>  
> -	beres = nfsd_file_do_acquire(NULL, net, cred, client,
> -				     fhp, may_flags, NULL, pnf, false);
> +	beres = nfsd_file_do_acquire(NULL, net, cred, client, fhp, may_flags,
> +				     NULL, S_IFREG, false, pnf);
>  	put_cred(revert_creds(save_cred));
>  	return beres;
>  }
> @@ -1344,7 +1347,33 @@ nfsd_file_acquire_opened(struct svc_rqst *rqstp, struct svc_fh *fhp,
>  			 struct nfsd_file **pnf)
>  {
>  	return nfsd_file_do_acquire(rqstp, SVC_NET(rqstp), NULL, NULL,
> -				    fhp, may_flags, file, pnf, false);
> +				    fhp, may_flags, file, S_IFREG, false, pnf);
> +}
> +
> +/**
> + * nfsd_file_acquire_dir - Get a struct nfsd_file with an open directory
> + * @rqstp: the RPC transaction being executed
> + * @fhp: the NFS filehandle of the file to be opened
> + * @pnf: OUT: new or found "struct nfsd_file" object
> + *
> + * The nfsd_file_object returned by this API is reference-counted
> + * but not garbage-collected. The object is unhashed after the
> + * final nfsd_file_put(). This opens directories only, and only
> + * in O_RDONLY mode.
> + *
> + * Return values:
> + *   %nfs_ok - @pnf points to an nfsd_file with its reference
> + *   count boosted.
> + *
> + * On error, an nfsstat value in network byte order is returned.
> + */
> +__be32
> +nfsd_file_acquire_dir(struct svc_rqst *rqstp, struct svc_fh *fhp,
> +		      struct nfsd_file **pnf)
> +{
> +	return nfsd_file_do_acquire(rqstp, SVC_NET(rqstp), NULL, NULL, fhp,
> +				    NFSD_MAY_READ|NFSD_MAY_64BIT_COOKIE,
> +				    NULL, S_IFDIR, false, pnf);
>  }
>  
>  /*
> diff --git a/fs/nfsd/filecache.h b/fs/nfsd/filecache.h
> index e3d6ca2b60308e5e91ba4bb32d935f54527d8bda..b383dbc5b9218d21a29b852572f80fab08de9fa9 100644
> --- a/fs/nfsd/filecache.h
> +++ b/fs/nfsd/filecache.h
> @@ -82,5 +82,7 @@ __be32 nfsd_file_acquire_opened(struct svc_rqst *rqstp, struct svc_fh *fhp,
>  __be32 nfsd_file_acquire_local(struct net *net, struct svc_cred *cred,
>  			       struct auth_domain *client, struct svc_fh *fhp,
>  			       unsigned int may_flags, struct nfsd_file **pnf);
> +__be32 nfsd_file_acquire_dir(struct svc_rqst *rqstp, struct svc_fh *fhp,
> +		  struct nfsd_file **pnf);
>  int nfsd_file_cache_stats_show(struct seq_file *m, void *v);
>  #endif /* _FS_NFSD_FILECACHE_H */
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index eeb138569eba5df6de361cf6ba29604722e14af9..12c33223b612664dbb3b18b591e97fc708165763 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -959,15 +959,16 @@ nfsd_open(struct svc_rqst *rqstp, struct svc_fh *fhp, umode_t type,
>  /**
>   * nfsd_open_verified - Open a regular file for the filecache
>   * @fhp: NFS filehandle of the file to open
> + * @type: S_IFMT inode type allowed (0 means any type is allowed)
>   * @may_flags: internal permission flags
>   * @filp: OUT: open "struct file *"
>   *
>   * Returns zero on success, or a negative errno value.
>   */
>  int
> -nfsd_open_verified(struct svc_fh *fhp, int may_flags, struct file **filp)
> +nfsd_open_verified(struct svc_fh *fhp, umode_t type, int may_flags, struct file **filp)
>  {
> -	return __nfsd_open(fhp, S_IFREG, may_flags, filp);
> +	return __nfsd_open(fhp, type, may_flags, filp);
>  }
>  
>  /*
> diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
> index fa46f8b5f132079e3a2c45e71ecf9cc43181f6b0..ded2900d423f80d33fb6c8b809bc5d9fc842ebfd 100644
> --- a/fs/nfsd/vfs.h
> +++ b/fs/nfsd/vfs.h
> @@ -114,7 +114,7 @@ __be32		nfsd_setxattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
>  int 		nfsd_open_break_lease(struct inode *, int);
>  __be32		nfsd_open(struct svc_rqst *, struct svc_fh *, umode_t,
>  				int, struct file **);
> -int		nfsd_open_verified(struct svc_fh *fhp, int may_flags,
> +int		nfsd_open_verified(struct svc_fh *fhp, umode_t type, int may_flags,
>  				struct file **filp);
>  __be32		nfsd_splice_read(struct svc_rqst *rqstp, struct svc_fh *fhp,
>  				struct file *file, loff_t offset,
> 

Reviewed-by: Chuck Lever <chuck.lever@oracle.com>

These can probably go in via Christian's tree. I don't think there
are going to be conflicts between these and what's in nfsd-testing
now.

-- 
Chuck Lever

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 10/11] nfsd: allow DELEGRETURN on directories
  2025-10-17 11:32 ` [PATCH v2 10/11] nfsd: allow DELEGRETURN on directories Jeff Layton
@ 2025-10-18 19:37   ` Chuck Lever
  0 siblings, 0 replies; 20+ messages in thread
From: Chuck Lever @ 2025-10-18 19:37 UTC (permalink / raw)
  To: Jeff Layton, Miklos Szeredi, Alexander Viro, Christian Brauner,
	Jan Kara, Alexander Aring, Trond Myklebust, Anna Schumaker,
	Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
	Tom Talpey, Bharath SM, Greg Kroah-Hartman, Rafael J. Wysocki,
	Danilo Krummrich, David Howells, Tyler Hicks, NeilBrown,
	Olga Kornievskaia, Dai Ngo, Amir Goldstein, Namjae Jeon,
	Steve French, Sergey Senozhatsky, Carlos Maiolino,
	Kuniyuki Iwashima, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman
  Cc: linux-fsdevel, linux-kernel, linux-nfs, linux-cifs,
	samba-technical, netfs, ecryptfs, linux-unionfs, linux-xfs,
	netdev

On 10/17/25 7:32 AM, Jeff Layton wrote:
> As Trond pointed out: "...provided that the presented stateid is
> actually valid, it is also sufficient to uniquely identify the file to
> which it is associated (see RFC8881 Section 8.2.4), so the filehandle
> should be considered mostly irrelevant for operations like DELEGRETURN."
> 
> Don't ask fh_verify to filter on file type.
> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  fs/nfsd/nfs4state.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index c9053ef4d79f074f49ecaf0c7a3db78ec147136e..b06591f154aa372db710e071c69260f4639956d7 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -7824,7 +7824,8 @@ nfsd4_delegreturn(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  	__be32 status;
>  	struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
>  
> -	if ((status = fh_verify(rqstp, &cstate->current_fh, S_IFREG, 0)))
> +	status = fh_verify(rqstp, &cstate->current_fh, 0, 0);
> +	if (status)
>  		return status;
>  
>  	status = nfsd4_lookup_stateid(cstate, stateid, SC_TYPE_DELEG, SC_STATUS_REVOKED, &s, nn);
> 

Reviewed-by: Chuck Lever <chuck.lever@oracle.com>

-- 
Chuck Lever

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 11/11] nfsd: wire up GET_DIR_DELEGATION handling
  2025-10-17 11:32 ` [PATCH v2 11/11] nfsd: wire up GET_DIR_DELEGATION handling Jeff Layton
@ 2025-10-18 19:46   ` Chuck Lever
  2025-10-19 13:46     ` Jeff Layton
  0 siblings, 1 reply; 20+ messages in thread
From: Chuck Lever @ 2025-10-18 19:46 UTC (permalink / raw)
  To: Jeff Layton, Miklos Szeredi, Alexander Viro, Christian Brauner,
	Jan Kara, Alexander Aring, Trond Myklebust, Anna Schumaker,
	Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
	Tom Talpey, Bharath SM, Greg Kroah-Hartman, Rafael J. Wysocki,
	Danilo Krummrich, David Howells, Tyler Hicks, NeilBrown,
	Olga Kornievskaia, Dai Ngo, Amir Goldstein, Namjae Jeon,
	Steve French, Sergey Senozhatsky, Carlos Maiolino,
	Kuniyuki Iwashima, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman
  Cc: linux-fsdevel, linux-kernel, linux-nfs, linux-cifs,
	samba-technical, netfs, ecryptfs, linux-unionfs, linux-xfs,
	netdev

On 10/17/25 7:32 AM, Jeff Layton wrote:
> Add a new routine for acquiring a read delegation on a directory. These
> are recallable-only delegations with no support for CB_NOTIFY. That will
> be added in a later phase.
> 
> Since the same CB_RECALL/DELEGRETURN infrastrure is used for regular and
> directory delegations, a normal nfs4_delegation is used to represent a
> directory delegation.

s/infrastrure/infrastructure/


> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  fs/nfsd/nfs4proc.c  |  21 ++++++++++-
>  fs/nfsd/nfs4state.c | 100 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  fs/nfsd/state.h     |   5 +++
>  3 files changed, 125 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 7f7e6bb23a90d9a1cafd154c0f09e236df75b083..527f8dc52159803770964700170473509ec328ed 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -2342,6 +2342,13 @@ nfsd4_get_dir_delegation(struct svc_rqst *rqstp,
>  			 union nfsd4_op_u *u)
>  {
>  	struct nfsd4_get_dir_delegation *gdd = &u->get_dir_delegation;
> +	struct nfs4_delegation *dd;
> +	struct nfsd_file *nf;
> +	__be32 status;
> +
> +	status = nfsd_file_acquire_dir(rqstp, &cstate->current_fh, &nf);
> +	if (status != nfs_ok)
> +		return status;
>  
>  	/*
>  	 * RFC 8881, section 18.39.3 says:
> @@ -2355,7 +2362,19 @@ nfsd4_get_dir_delegation(struct svc_rqst *rqstp,
>  	 * return NFS4_OK with a non-fatal status of GDD4_UNAVAIL in this
>  	 * situation.
>  	 */
> -	gdd->gddrnf_status = GDD4_UNAVAIL;
> +	dd = nfsd_get_dir_deleg(cstate, gdd, nf);
> +	if (IS_ERR(dd)) {
> +		int err = PTR_ERR(dd);
> +
> +		if (err != -EAGAIN)
> +			return nfserrno(err);
> +		gdd->gddrnf_status = GDD4_UNAVAIL;
> +		return nfs_ok;
> +	}

These error flows might leak the nf acquired just above.


> +
> +	gdd->gddrnf_status = GDD4_OK;
> +	memcpy(&gdd->gddr_stateid, &dd->dl_stid.sc_stateid, sizeof(gdd->gddr_stateid));
> +	nfs4_put_stid(&dd->dl_stid);
>  	return nfs_ok;
>  }
>  
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index b06591f154aa372db710e071c69260f4639956d7..a63e8c885291fc377163f3255f26f5f693704437 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -9359,3 +9359,103 @@ nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, struct dentry *dentry,
>  	nfs4_put_stid(&dp->dl_stid);
>  	return status;
>  }
> +
> +/**
> + * nfsd_get_dir_deleg - attempt to get a directory delegation
> + * @cstate: compound state
> + * @gdd: GET_DIR_DELEGATION arg/resp structure
> + * @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.
> + */
> +struct nfs4_delegation *
> +nfsd_get_dir_deleg(struct nfsd4_compound_state *cstate,
> +		   struct nfsd4_get_dir_delegation *gdd,
> +		   struct nfsd_file *nf)
> +{
> +	struct nfs4_client *clp = cstate->clp;
> +	struct nfs4_delegation *dp;
> +	struct file_lease *fl;
> +	struct nfs4_file *fp, *rfp;
> +	int status = 0;
> +
> +	fp = nfsd4_alloc_file();
> +	if (!fp)
> +		return ERR_PTR(-ENOMEM);
> +
> +	nfsd4_file_init(&cstate->current_fh, fp);
> +
> +	rfp = nfsd4_file_hash_insert(fp, &cstate->current_fh);
> +	if (unlikely(!rfp)) {
> +		put_nfs4_file(fp);
> +		return ERR_PTR(-ENOMEM);
> +	}
> +
> +	if (rfp != fp) {
> +		put_nfs4_file(fp);
> +		fp = rfp;
> +	}
> +
> +	/* if this client already has one, return that it's unavailable */
> +	spin_lock(&state_lock);
> +	spin_lock(&fp->fi_lock);
> +	/* existing delegation? */
> +	if (nfs4_delegation_exists(clp, fp)) {
> +		status = -EAGAIN;
> +	} else if (!fp->fi_deleg_file) {
> +		fp->fi_deleg_file = nf;
> +		fp->fi_delegees = 1;
> +	} else {
> +		++fp->fi_delegees;

The new nf is unused in this arm. Does it need to be released?


> +	}
> +	spin_unlock(&fp->fi_lock);
> +	spin_unlock(&state_lock);
> +
> +	if (status) {
> +		put_nfs4_file(fp);
> +		return ERR_PTR(status);
> +	}
> +
> +	/* Try to set up the lease */
> +	status = -ENOMEM;
> +	dp = alloc_init_deleg(clp, fp, NULL, NFS4_OPEN_DELEGATE_READ);
> +	if (!dp)
> +		goto out_delegees;
> +
> +	fl = nfs4_alloc_init_lease(dp);
> +	if (!fl)
> +		goto out_put_stid;
> +
> +	status = kernel_setlease(nf->nf_file,
> +				 fl->c.flc_type, &fl, NULL);
> +	if (fl)
> +		locks_free_lease(fl);
> +	if (status)
> +		goto out_put_stid;
> +
> +	/*
> +	 * Now, try to hash it. This can fail if we race another nfsd task
> +	 * trying to set a delegation on the same file. If that happens,
> +	 * then just say UNAVAIL.
> +	 */
> +	spin_lock(&state_lock);
> +	spin_lock(&clp->cl_lock);
> +	spin_lock(&fp->fi_lock);
> +	status = hash_delegation_locked(dp, fp);
> +	spin_unlock(&fp->fi_lock);
> +	spin_unlock(&clp->cl_lock);
> +	spin_unlock(&state_lock);
> +
> +	if (!status)
> +		return dp;
> +
> +	/* Something failed. Drop the lease and clean up the stid */
> +	kernel_setlease(fp->fi_deleg_file->nf_file, F_UNLCK, NULL, (void **)&dp);
> +out_put_stid:
> +	nfs4_put_stid(&dp->dl_stid);
> +out_delegees:
> +	put_deleg_file(fp);
> +	return ERR_PTR(status);
> +}
> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> index 1e736f4024263ffa9c93bcc9ec48f44566a8cc77..b052c1effdc5356487c610db9728df8ecfe851d4 100644
> --- a/fs/nfsd/state.h
> +++ b/fs/nfsd/state.h
> @@ -867,4 +867,9 @@ static inline bool try_to_expire_client(struct nfs4_client *clp)
>  
>  extern __be32 nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp,
>  		struct dentry *dentry, struct nfs4_delegation **pdp);
> +
> +struct nfsd4_get_dir_delegation;
> +struct nfs4_delegation *nfsd_get_dir_deleg(struct nfsd4_compound_state *cstate,
> +						struct nfsd4_get_dir_delegation *gdd,
> +						struct nfsd_file *nf);
>  #endif   /* NFSD4_STATE_H */
> 


-- 
Chuck Lever

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 09/11] nfsd: allow filecache to hold S_IFDIR files
  2025-10-18 19:33   ` Chuck Lever
@ 2025-10-19 13:41     ` Jeff Layton
  0 siblings, 0 replies; 20+ messages in thread
From: Jeff Layton @ 2025-10-19 13:41 UTC (permalink / raw)
  To: Chuck Lever, Miklos Szeredi, Alexander Viro, Christian Brauner,
	Jan Kara, Alexander Aring, Trond Myklebust, Anna Schumaker,
	Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
	Tom Talpey, Bharath SM, Greg Kroah-Hartman, Rafael J. Wysocki,
	Danilo Krummrich, David Howells, Tyler Hicks, NeilBrown,
	Olga Kornievskaia, Dai Ngo, Amir Goldstein, Namjae Jeon,
	Steve French, Sergey Senozhatsky, Carlos Maiolino,
	Kuniyuki Iwashima, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman
  Cc: linux-fsdevel, linux-kernel, linux-nfs, linux-cifs,
	samba-technical, netfs, ecryptfs, linux-unionfs, linux-xfs,
	netdev

On Sat, 2025-10-18 at 15:33 -0400, Chuck Lever wrote:
> On 10/17/25 7:32 AM, Jeff Layton wrote:
> > The filecache infrastructure will only handle S_IFREG files at the
> > moment. Directory delegations will require adding support for opening
> > S_IFDIR inodes.
> > 
> > Plumb a "type" argument into nfsd_file_do_acquire() and have all of the
> > existing callers set it to S_IFREG. Add a new nfsd_file_acquire_dir()
> > wrapper that nfsd can call to request a nfsd_file that holds a directory
> > open.
> > 
> > For now, there is no need for a fsnotify_mark for directories, as
> > CB_NOTIFY is not yet supported. Change nfsd_file_do_acquire() to avoid
> > allocating one for non-S_IFREG inodes.
> > 
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> >  fs/nfsd/filecache.c | 57 ++++++++++++++++++++++++++++++++++++++++-------------
> >  fs/nfsd/filecache.h |  2 ++
> >  fs/nfsd/vfs.c       |  5 +++--
> >  fs/nfsd/vfs.h       |  2 +-
> >  4 files changed, 49 insertions(+), 17 deletions(-)
> > 
> > diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c
> > index a238b6725008a5c2988bd3da874d1f34ee778437..93798575b8075c63f95cd415b6d24df706ada0f6 100644
> > --- a/fs/nfsd/filecache.c
> > +++ b/fs/nfsd/filecache.c
> > @@ -1086,7 +1086,7 @@ nfsd_file_do_acquire(struct svc_rqst *rqstp, struct net *net,
> >  		     struct auth_domain *client,
> >  		     struct svc_fh *fhp,
> >  		     unsigned int may_flags, struct file *file,
> > -		     struct nfsd_file **pnf, bool want_gc)
> > +		     umode_t type, bool want_gc, struct nfsd_file **pnf)
> >  {
> >  	unsigned char need = may_flags & NFSD_FILE_MAY_MASK;
> >  	struct nfsd_file *new, *nf;
> > @@ -1097,13 +1097,13 @@ nfsd_file_do_acquire(struct svc_rqst *rqstp, struct net *net,
> >  	int ret;
> >  
> >  retry:
> > -	if (rqstp) {
> > -		status = fh_verify(rqstp, fhp, S_IFREG,
> > +	if (rqstp)
> > +		status = fh_verify(rqstp, fhp, type,
> >  				   may_flags|NFSD_MAY_OWNER_OVERRIDE);
> > -	} else {
> > -		status = fh_verify_local(net, cred, client, fhp, S_IFREG,
> > +	else
> > +		status = fh_verify_local(net, cred, client, fhp, type,
> >  					 may_flags|NFSD_MAY_OWNER_OVERRIDE);
> > -	}
> > +
> >  	if (status != nfs_ok)
> >  		return status;
> >  	inode = d_inode(fhp->fh_dentry);
> > @@ -1176,15 +1176,18 @@ nfsd_file_do_acquire(struct svc_rqst *rqstp, struct net *net,
> >  
> >  open_file:
> >  	trace_nfsd_file_alloc(nf);
> > -	nf->nf_mark = nfsd_file_mark_find_or_create(inode);
> > -	if (nf->nf_mark) {
> > +
> > +	if (type == S_IFREG)
> > +		nf->nf_mark = nfsd_file_mark_find_or_create(inode);
> > +
> > +	if (type != S_IFREG || nf->nf_mark) {
> >  		if (file) {
> >  			get_file(file);
> >  			nf->nf_file = file;
> >  			status = nfs_ok;
> >  			trace_nfsd_file_opened(nf, status);
> >  		} else {
> > -			ret = nfsd_open_verified(fhp, may_flags, &nf->nf_file);
> > +			ret = nfsd_open_verified(fhp, type, may_flags, &nf->nf_file);
> >  			if (ret == -EOPENSTALE && stale_retry) {
> >  				stale_retry = false;
> >  				nfsd_file_unhash(nf);
> > @@ -1246,7 +1249,7 @@ nfsd_file_acquire_gc(struct svc_rqst *rqstp, struct svc_fh *fhp,
> >  		     unsigned int may_flags, struct nfsd_file **pnf)
> >  {
> >  	return nfsd_file_do_acquire(rqstp, SVC_NET(rqstp), NULL, NULL,
> > -				    fhp, may_flags, NULL, pnf, true);
> > +				    fhp, may_flags, NULL, S_IFREG, true, pnf);
> >  }
> >  
> >  /**
> > @@ -1271,7 +1274,7 @@ nfsd_file_acquire(struct svc_rqst *rqstp, struct svc_fh *fhp,
> >  		  unsigned int may_flags, struct nfsd_file **pnf)
> >  {
> >  	return nfsd_file_do_acquire(rqstp, SVC_NET(rqstp), NULL, NULL,
> > -				    fhp, may_flags, NULL, pnf, false);
> > +				    fhp, may_flags, NULL, S_IFREG, false, pnf);
> >  }
> >  
> >  /**
> > @@ -1314,8 +1317,8 @@ nfsd_file_acquire_local(struct net *net, struct svc_cred *cred,
> >  	const struct cred *save_cred = get_current_cred();
> >  	__be32 beres;
> >  
> > -	beres = nfsd_file_do_acquire(NULL, net, cred, client,
> > -				     fhp, may_flags, NULL, pnf, false);
> > +	beres = nfsd_file_do_acquire(NULL, net, cred, client, fhp, may_flags,
> > +				     NULL, S_IFREG, false, pnf);
> >  	put_cred(revert_creds(save_cred));
> >  	return beres;
> >  }
> > @@ -1344,7 +1347,33 @@ nfsd_file_acquire_opened(struct svc_rqst *rqstp, struct svc_fh *fhp,
> >  			 struct nfsd_file **pnf)
> >  {
> >  	return nfsd_file_do_acquire(rqstp, SVC_NET(rqstp), NULL, NULL,
> > -				    fhp, may_flags, file, pnf, false);
> > +				    fhp, may_flags, file, S_IFREG, false, pnf);
> > +}
> > +
> > +/**
> > + * nfsd_file_acquire_dir - Get a struct nfsd_file with an open directory
> > + * @rqstp: the RPC transaction being executed
> > + * @fhp: the NFS filehandle of the file to be opened
> > + * @pnf: OUT: new or found "struct nfsd_file" object
> > + *
> > + * The nfsd_file_object returned by this API is reference-counted
> > + * but not garbage-collected. The object is unhashed after the
> > + * final nfsd_file_put(). This opens directories only, and only
> > + * in O_RDONLY mode.
> > + *
> > + * Return values:
> > + *   %nfs_ok - @pnf points to an nfsd_file with its reference
> > + *   count boosted.
> > + *
> > + * On error, an nfsstat value in network byte order is returned.
> > + */
> > +__be32
> > +nfsd_file_acquire_dir(struct svc_rqst *rqstp, struct svc_fh *fhp,
> > +		      struct nfsd_file **pnf)
> > +{
> > +	return nfsd_file_do_acquire(rqstp, SVC_NET(rqstp), NULL, NULL, fhp,
> > +				    NFSD_MAY_READ|NFSD_MAY_64BIT_COOKIE,
> > +				    NULL, S_IFDIR, false, pnf);
> >  }
> >  
> >  /*
> > diff --git a/fs/nfsd/filecache.h b/fs/nfsd/filecache.h
> > index e3d6ca2b60308e5e91ba4bb32d935f54527d8bda..b383dbc5b9218d21a29b852572f80fab08de9fa9 100644
> > --- a/fs/nfsd/filecache.h
> > +++ b/fs/nfsd/filecache.h
> > @@ -82,5 +82,7 @@ __be32 nfsd_file_acquire_opened(struct svc_rqst *rqstp, struct svc_fh *fhp,
> >  __be32 nfsd_file_acquire_local(struct net *net, struct svc_cred *cred,
> >  			       struct auth_domain *client, struct svc_fh *fhp,
> >  			       unsigned int may_flags, struct nfsd_file **pnf);
> > +__be32 nfsd_file_acquire_dir(struct svc_rqst *rqstp, struct svc_fh *fhp,
> > +		  struct nfsd_file **pnf);
> >  int nfsd_file_cache_stats_show(struct seq_file *m, void *v);
> >  #endif /* _FS_NFSD_FILECACHE_H */
> > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> > index eeb138569eba5df6de361cf6ba29604722e14af9..12c33223b612664dbb3b18b591e97fc708165763 100644
> > --- a/fs/nfsd/vfs.c
> > +++ b/fs/nfsd/vfs.c
> > @@ -959,15 +959,16 @@ nfsd_open(struct svc_rqst *rqstp, struct svc_fh *fhp, umode_t type,
> >  /**
> >   * nfsd_open_verified - Open a regular file for the filecache
> >   * @fhp: NFS filehandle of the file to open
> > + * @type: S_IFMT inode type allowed (0 means any type is allowed)
> >   * @may_flags: internal permission flags
> >   * @filp: OUT: open "struct file *"
> >   *
> >   * Returns zero on success, or a negative errno value.
> >   */
> >  int
> > -nfsd_open_verified(struct svc_fh *fhp, int may_flags, struct file **filp)
> > +nfsd_open_verified(struct svc_fh *fhp, umode_t type, int may_flags, struct file **filp)
> >  {
> > -	return __nfsd_open(fhp, S_IFREG, may_flags, filp);
> > +	return __nfsd_open(fhp, type, may_flags, filp);
> >  }
> >  
> >  /*
> > diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
> > index fa46f8b5f132079e3a2c45e71ecf9cc43181f6b0..ded2900d423f80d33fb6c8b809bc5d9fc842ebfd 100644
> > --- a/fs/nfsd/vfs.h
> > +++ b/fs/nfsd/vfs.h
> > @@ -114,7 +114,7 @@ __be32		nfsd_setxattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
> >  int 		nfsd_open_break_lease(struct inode *, int);
> >  __be32		nfsd_open(struct svc_rqst *, struct svc_fh *, umode_t,
> >  				int, struct file **);
> > -int		nfsd_open_verified(struct svc_fh *fhp, int may_flags,
> > +int		nfsd_open_verified(struct svc_fh *fhp, umode_t type, int may_flags,
> >  				struct file **filp);
> >  __be32		nfsd_splice_read(struct svc_rqst *rqstp, struct svc_fh *fhp,
> >  				struct file *file, loff_t offset,
> > 
> 
> Reviewed-by: Chuck Lever <chuck.lever@oracle.com>
> 
> These can probably go in via Christian's tree. I don't think there
> are going to be conflicts between these and what's in nfsd-testing
> now.

That would work. I think I have at least one more respin of the VFS
bits before we merge anything anyway.
-- 
Jeff Layton <jlayton@kernel.org>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 11/11] nfsd: wire up GET_DIR_DELEGATION handling
  2025-10-18 19:46   ` Chuck Lever
@ 2025-10-19 13:46     ` Jeff Layton
  0 siblings, 0 replies; 20+ messages in thread
From: Jeff Layton @ 2025-10-19 13:46 UTC (permalink / raw)
  To: Chuck Lever, Miklos Szeredi, Alexander Viro, Christian Brauner,
	Jan Kara, Alexander Aring, Trond Myklebust, Anna Schumaker,
	Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
	Tom Talpey, Bharath SM, Greg Kroah-Hartman, Rafael J. Wysocki,
	Danilo Krummrich, David Howells, Tyler Hicks, NeilBrown,
	Olga Kornievskaia, Dai Ngo, Amir Goldstein, Namjae Jeon,
	Steve French, Sergey Senozhatsky, Carlos Maiolino,
	Kuniyuki Iwashima, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman
  Cc: linux-fsdevel, linux-kernel, linux-nfs, linux-cifs,
	samba-technical, netfs, ecryptfs, linux-unionfs, linux-xfs,
	netdev

On Sat, 2025-10-18 at 15:46 -0400, Chuck Lever wrote:
> On 10/17/25 7:32 AM, Jeff Layton wrote:
> > Add a new routine for acquiring a read delegation on a directory. These
> > are recallable-only delegations with no support for CB_NOTIFY. That will
> > be added in a later phase.
> > 
> > Since the same CB_RECALL/DELEGRETURN infrastrure is used for regular and
> > directory delegations, a normal nfs4_delegation is used to represent a
> > directory delegation.
> 
> s/infrastrure/infrastructure/
> 
> 
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> >  fs/nfsd/nfs4proc.c  |  21 ++++++++++-
> >  fs/nfsd/nfs4state.c | 100 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  fs/nfsd/state.h     |   5 +++
> >  3 files changed, 125 insertions(+), 1 deletion(-)
> > 
> > diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> > index 7f7e6bb23a90d9a1cafd154c0f09e236df75b083..527f8dc52159803770964700170473509ec328ed 100644
> > --- a/fs/nfsd/nfs4proc.c
> > +++ b/fs/nfsd/nfs4proc.c
> > @@ -2342,6 +2342,13 @@ nfsd4_get_dir_delegation(struct svc_rqst *rqstp,
> >  			 union nfsd4_op_u *u)
> >  {
> >  	struct nfsd4_get_dir_delegation *gdd = &u->get_dir_delegation;
> > +	struct nfs4_delegation *dd;
> > +	struct nfsd_file *nf;
> > +	__be32 status;
> > +
> > +	status = nfsd_file_acquire_dir(rqstp, &cstate->current_fh, &nf);
> > +	if (status != nfs_ok)
> > +		return status;
> >  
> >  	/*
> >  	 * RFC 8881, section 18.39.3 says:
> > @@ -2355,7 +2362,19 @@ nfsd4_get_dir_delegation(struct svc_rqst *rqstp,
> >  	 * return NFS4_OK with a non-fatal status of GDD4_UNAVAIL in this
> >  	 * situation.
> >  	 */
> > -	gdd->gddrnf_status = GDD4_UNAVAIL;
> > +	dd = nfsd_get_dir_deleg(cstate, gdd, nf);
> > +	if (IS_ERR(dd)) {
> > +		int err = PTR_ERR(dd);
> > +
> > +		if (err != -EAGAIN)
> > +			return nfserrno(err);
> > +		gdd->gddrnf_status = GDD4_UNAVAIL;
> > +		return nfs_ok;
> > +	}
> 
> These error flows might leak the nf acquired just above.
> 

Good catch. I'll fix that.

> 
> > +
> > +	gdd->gddrnf_status = GDD4_OK;
> > +	memcpy(&gdd->gddr_stateid, &dd->dl_stid.sc_stateid, sizeof(gdd->gddr_stateid));
> > +	nfs4_put_stid(&dd->dl_stid);
> >  	return nfs_ok;
> >  }
> >  
> > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > index b06591f154aa372db710e071c69260f4639956d7..a63e8c885291fc377163f3255f26f5f693704437 100644
> > --- a/fs/nfsd/nfs4state.c
> > +++ b/fs/nfsd/nfs4state.c
> > @@ -9359,3 +9359,103 @@ nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, struct dentry *dentry,
> >  	nfs4_put_stid(&dp->dl_stid);
> >  	return status;
> >  }
> > +
> > +/**
> > + * nfsd_get_dir_deleg - attempt to get a directory delegation
> > + * @cstate: compound state
> > + * @gdd: GET_DIR_DELEGATION arg/resp structure
> > + * @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.
> > + */
> > +struct nfs4_delegation *
> > +nfsd_get_dir_deleg(struct nfsd4_compound_state *cstate,
> > +		   struct nfsd4_get_dir_delegation *gdd,
> > +		   struct nfsd_file *nf)
> > +{
> > +	struct nfs4_client *clp = cstate->clp;
> > +	struct nfs4_delegation *dp;
> > +	struct file_lease *fl;
> > +	struct nfs4_file *fp, *rfp;
> > +	int status = 0;
> > +
> > +	fp = nfsd4_alloc_file();
> > +	if (!fp)
> > +		return ERR_PTR(-ENOMEM);
> > +
> > +	nfsd4_file_init(&cstate->current_fh, fp);
> > +
> > +	rfp = nfsd4_file_hash_insert(fp, &cstate->current_fh);
> > +	if (unlikely(!rfp)) {
> > +		put_nfs4_file(fp);
> > +		return ERR_PTR(-ENOMEM);
> > +	}
> > +
> > +	if (rfp != fp) {
> > +		put_nfs4_file(fp);
> > +		fp = rfp;
> > +	}
> > +
> > +	/* if this client already has one, return that it's unavailable */
> > +	spin_lock(&state_lock);
> > +	spin_lock(&fp->fi_lock);
> > +	/* existing delegation? */
> > +	if (nfs4_delegation_exists(clp, fp)) {
> > +		status = -EAGAIN;
> > +	} else if (!fp->fi_deleg_file) {
> > +		fp->fi_deleg_file = nf;
> > +		fp->fi_delegees = 1;
> > +	} else {
> > +		++fp->fi_delegees;
> 
> The new nf is unused in this arm. Does it need to be released?
> 

Yes. Good catch!

> 
> > +	}
> > +	spin_unlock(&fp->fi_lock);
> > +	spin_unlock(&state_lock);
> > +
> > +	if (status) {
> > +		put_nfs4_file(fp);
> > +		return ERR_PTR(status);
> > +	}
> > +
> > +	/* Try to set up the lease */
> > +	status = -ENOMEM;
> > +	dp = alloc_init_deleg(clp, fp, NULL, NFS4_OPEN_DELEGATE_READ);
> > +	if (!dp)
> > +		goto out_delegees;
> > +
> > +	fl = nfs4_alloc_init_lease(dp);
> > +	if (!fl)
> > +		goto out_put_stid;
> > +
> > +	status = kernel_setlease(nf->nf_file,
> > +				 fl->c.flc_type, &fl, NULL);
> > +	if (fl)
> > +		locks_free_lease(fl);
> > +	if (status)
> > +		goto out_put_stid;
> > +
> > +	/*
> > +	 * Now, try to hash it. This can fail if we race another nfsd task
> > +	 * trying to set a delegation on the same file. If that happens,
> > +	 * then just say UNAVAIL.
> > +	 */
> > +	spin_lock(&state_lock);
> > +	spin_lock(&clp->cl_lock);
> > +	spin_lock(&fp->fi_lock);
> > +	status = hash_delegation_locked(dp, fp);
> > +	spin_unlock(&fp->fi_lock);
> > +	spin_unlock(&clp->cl_lock);
> > +	spin_unlock(&state_lock);
> > +
> > +	if (!status)
> > +		return dp;
> > +
> > +	/* Something failed. Drop the lease and clean up the stid */
> > +	kernel_setlease(fp->fi_deleg_file->nf_file, F_UNLCK, NULL, (void **)&dp);
> > +out_put_stid:
> > +	nfs4_put_stid(&dp->dl_stid);
> > +out_delegees:
> > +	put_deleg_file(fp);
> > +	return ERR_PTR(status);
> > +}
> > diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> > index 1e736f4024263ffa9c93bcc9ec48f44566a8cc77..b052c1effdc5356487c610db9728df8ecfe851d4 100644
> > --- a/fs/nfsd/state.h
> > +++ b/fs/nfsd/state.h
> > @@ -867,4 +867,9 @@ static inline bool try_to_expire_client(struct nfs4_client *clp)
> >  
> >  extern __be32 nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp,
> >  		struct dentry *dentry, struct nfs4_delegation **pdp);
> > +
> > +struct nfsd4_get_dir_delegation;
> > +struct nfs4_delegation *nfsd_get_dir_deleg(struct nfsd4_compound_state *cstate,
> > +						struct nfsd4_get_dir_delegation *gdd,
> > +						struct nfsd_file *nf);
> >  #endif   /* NFSD4_STATE_H */
> > 
> 

-- 
Jeff Layton <jlayton@kernel.org>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 00/11] vfs: recall-only directory delegations for knfsd
  2025-10-17 23:44 ` [PATCH v2 00/11] vfs: recall-only directory delegations for knfsd NeilBrown
@ 2025-10-19 14:17   ` Jeff Layton
  0 siblings, 0 replies; 20+ messages in thread
From: Jeff Layton @ 2025-10-19 14:17 UTC (permalink / raw)
  To: NeilBrown
  Cc: Miklos Szeredi, Alexander Viro, Christian Brauner, Jan Kara,
	Chuck Lever, Alexander Aring, Trond Myklebust, Anna Schumaker,
	Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
	Tom Talpey, Bharath SM, Greg Kroah-Hartman, Rafael J. Wysocki,
	Danilo Krummrich, David Howells, Tyler Hicks, Olga Kornievskaia,
	Dai Ngo, Amir Goldstein, Namjae Jeon, Steve French,
	Sergey Senozhatsky, Carlos Maiolino, Kuniyuki Iwashima,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, linux-fsdevel, linux-kernel, linux-nfs, linux-cifs,
	samba-technical, netfs, ecryptfs, linux-unionfs, linux-xfs,
	netdev

On Sat, 2025-10-18 at 10:44 +1100, NeilBrown wrote:
> On Fri, 17 Oct 2025, Jeff Layton wrote:
> > A smaller variation of the v1 patchset that I posted earlier this week.
> > Neil's review inspired me to get rid of the lm_may_setlease operation
> > and to do the conflict resolution internally inside of nfsd. That means
> > a smaller VFS-layer change, and an overall reduction in code.
> > 
> > This patchset adds support for directory delegations to nfsd. This
> > version only supports recallable delegations. There is no CB_NOTIFY
> > support yet. I have patches for those, but we've decided to add that
> > support in a later kernel once we get some experience with this part.
> > Anna is working on the client-side pieces.
> > 
> > It would be great if we could get into linux-next soon so that it can be
> > merged for v6.19. Christian, could you pick up the vfs/filelock patches,
> > and Chuck pick up the nfsd patches?
> > 
> > Thanks!
> > Jeff
> > 
> > [1]: https://lore.kernel.org/all/20240315-dir-deleg-v1-0-a1d6209a3654@kernel.org/
> > 
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> > Changes in v2:
> > - handle lease conflict resolution inside of nfsd
> > - drop the lm_may_setlease lock_manager operation
> > - just add extra argument to vfs_create() instead of creating wrapper
> > - don't allocate fsnotify_mark for open directories
> > - Link to v1: https://lore.kernel.org/r/20251013-dir-deleg-ro-v1-0-406780a70e5e@kernel.org
> > 
> > ---
> > Jeff Layton (11):
> >       filelock: push the S_ISREG check down to ->setlease handlers
> >       vfs: add try_break_deleg calls for parents to vfs_{link,rename,unlink}
> >       vfs: allow mkdir to wait for delegation break on parent
> >       vfs: allow rmdir to wait for delegation break on parent
> >       vfs: break parent dir delegations in open(..., O_CREAT) codepath
> >       vfs: make vfs_create break delegations on parent directory
> >       vfs: make vfs_mknod break delegations on parent directory
> >       filelock: lift the ban on directory leases in generic_setlease
> >       nfsd: allow filecache to hold S_IFDIR files
> >       nfsd: allow DELEGRETURN on directories
> >       nfsd: wire up GET_DIR_DELEGATION handling
> 
> vfs_symlink() is missing from the updated APIs.  Surely that needs to be
> able to wait for a delegation to break.
> 

Ouch! That's a major oversight. I'll fix that up.

> vfs_mkobj() maybe does too, but I could easily turn a blind eye to that.
> 
> I haven't looked properly at the last patch but all the other could have
>  Reviewed-by: NeilBrown <neil@brown.name>
> 
> once the vfs_symlink() omission is fixed.
> 
> NeilBrown


Chuck found a couple of potential leaks in there so those will also
need to be fixed. As I was writing some xfstests for the VFS pieces, I
found another problem too:

Currently the F_SETLEASE API sets FL_LEASE leases, but the new
delegation breaks that this set adds don't break FL_LEASE leases, since
these are FL_DELEG leases.

This distinction is mostly due to historical reasons. Leases were added
first (for Samba oplocks), but didn't break on metadata changes. When
Bruce added delegations, he wanted to ensure that the lease API didn't
suddenly change behavior.

I see several potential options to fix this:

1/ The simplest is to just make the F_SETLEASE command set FL_DELEG
leases when the inode is a directory. That makes for a messy userland
interface where files get FL_LEASE objects, but directories get
FL_DELEG. I think that will be less useful for userland.

2/ Don't expose this to userland at all (yet?), and just keep returning
EINVAL on attempts to set a lease on a directory. The downside there is
that this would require us to use nfsd for testing this functionality.
Less people will do that than would if it were an xfstest that ran on
most local filesystems. I do have some pynfs tests though which could
help cover the gap.

3/ Add new F_SETDELEG/F_GETDELEG fcntl() commands. The nice thing about
this is that it would also allow us to add a flags field to these
commands. The later patches that add CB_NOTIFY support add the ability
to ignore certain types of delegation break events. This option would
allow us to expose that functionality to userland too. NFS Ganesha and
Samba, for example, could make use of this.

#3 wouldn't be too difficult (aside from having to update the
manpages), so I kind of like that idea.

Thoughts?
-- 
Jeff Layton <jlayton@kernel.org>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 00/11] vfs: recall-only directory delegations for knfsd
  2025-10-17 11:31 [PATCH v2 00/11] vfs: recall-only directory delegations for knfsd Jeff Layton
                   ` (11 preceding siblings ...)
  2025-10-17 23:44 ` [PATCH v2 00/11] vfs: recall-only directory delegations for knfsd NeilBrown
@ 2025-10-21 13:09 ` Christian Brauner
  12 siblings, 0 replies; 20+ messages in thread
From: Christian Brauner @ 2025-10-21 13:09 UTC (permalink / raw)
  To: Jeff Layton
  Cc: Miklos Szeredi, Alexander Viro, Jan Kara, Chuck Lever,
	Alexander Aring, Trond Myklebust, Anna Schumaker, Steve French,
	Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N, Tom Talpey,
	Bharath SM, Greg Kroah-Hartman, Rafael J. Wysocki,
	Danilo Krummrich, David Howells, Tyler Hicks, NeilBrown,
	Olga Kornievskaia, Dai Ngo, Amir Goldstein, Namjae Jeon,
	Steve French, Sergey Senozhatsky, Carlos Maiolino,
	Kuniyuki Iwashima, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, linux-fsdevel, linux-kernel, linux-nfs,
	linux-cifs, samba-technical, netfs, ecryptfs, linux-unionfs,
	linux-xfs, netdev

On Fri, Oct 17, 2025 at 07:31:52AM -0400, Jeff Layton wrote:
> A smaller variation of the v1 patchset that I posted earlier this week.
> Neil's review inspired me to get rid of the lm_may_setlease operation
> and to do the conflict resolution internally inside of nfsd. That means
> a smaller VFS-layer change, and an overall reduction in code.
> 
> This patchset adds support for directory delegations to nfsd. This
> version only supports recallable delegations. There is no CB_NOTIFY
> support yet. I have patches for those, but we've decided to add that
> support in a later kernel once we get some experience with this part.
> Anna is working on the client-side pieces.
> 
> It would be great if we could get into linux-next soon so that it can be
> merged for v6.19. Christian, could you pick up the vfs/filelock patches,
> and Chuck pick up the nfsd patches?

Happy to! Sorry, for the late reply. I was out for a few days.

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2025-10-21 13:09 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-17 11:31 [PATCH v2 00/11] vfs: recall-only directory delegations for knfsd Jeff Layton
2025-10-17 11:31 ` [PATCH v2 01/11] filelock: push the S_ISREG check down to ->setlease handlers Jeff Layton
2025-10-17 11:31 ` [PATCH v2 02/11] vfs: add try_break_deleg calls for parents to vfs_{link,rename,unlink} Jeff Layton
2025-10-17 11:31 ` [PATCH v2 03/11] vfs: allow mkdir to wait for delegation break on parent Jeff Layton
2025-10-17 11:31 ` [PATCH v2 04/11] vfs: allow rmdir " Jeff Layton
2025-10-17 11:31 ` [PATCH v2 05/11] vfs: break parent dir delegations in open(..., O_CREAT) codepath Jeff Layton
2025-10-17 11:31 ` [PATCH v2 06/11] vfs: make vfs_create break delegations on parent directory Jeff Layton
2025-10-17 11:31 ` [PATCH v2 07/11] vfs: make vfs_mknod " Jeff Layton
2025-10-17 11:32 ` [PATCH v2 08/11] filelock: lift the ban on directory leases in generic_setlease Jeff Layton
2025-10-17 11:32 ` [PATCH v2 09/11] nfsd: allow filecache to hold S_IFDIR files Jeff Layton
2025-10-18 19:33   ` Chuck Lever
2025-10-19 13:41     ` Jeff Layton
2025-10-17 11:32 ` [PATCH v2 10/11] nfsd: allow DELEGRETURN on directories Jeff Layton
2025-10-18 19:37   ` Chuck Lever
2025-10-17 11:32 ` [PATCH v2 11/11] nfsd: wire up GET_DIR_DELEGATION handling Jeff Layton
2025-10-18 19:46   ` Chuck Lever
2025-10-19 13:46     ` Jeff Layton
2025-10-17 23:44 ` [PATCH v2 00/11] vfs: recall-only directory delegations for knfsd NeilBrown
2025-10-19 14:17   ` Jeff Layton
2025-10-21 13:09 ` Christian Brauner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).