[PATCH v2 0/4] Support more filesystems with FAN_REPORT_FID

[PATCH v2 0/4] Support more filesystems with FAN_REPORT_FID

[Amir Goldstein]

Christian,

The grand plan is to be able to use fanotify with FAN_REPORT_FID as a
drop-in replacement for inotify, but with current upstream, inotify is
supported on all the filesystems and FAN_REPORT_FID only on a few.

Making all filesystem support FAN_REPORT_FID requires that all
filesystems will:
1. Support for AT_HANDLE_FID file handles
2. Report non-zero f_fsid

This patch set takes care of the first requirement.
Patches were reviewed by Jan and the nfsd maintainers.

I have another patch in review [2] for adding non-zero f_fsid to many
simple filesystems, but it is independent of this patch set, so no
reason to couple them together.

Note that patch #2 touches many filesystems due to vfs API change,
requiring an explicit ->encode_fh() method. I did not gets ACKs from
all filesystem maintainers, but the change is trivial and does not
change any logic.

Thanks,
Amir.

Changes since v1 [1]:
- Patch #1 already merged into v6.6-rc7
- Fix build without CONFIG_EXPORTFS
- Fix checkpatch warnings
- Define symbolic constant for FILEID_INO64_GEN_LEN
- Clarify documentation (units of) max_len argument

[1] https://lore.kernel.org/r/20231018100000.2453965-1-amir73il@gmail.com/
[2] https://lore.kernel.org/r/20231023143049.2944970-1-amir73il@gmail.com/

Amir Goldstein (4):
  exportfs: add helpers to check if filesystem can encode/decode file
    handles
  exportfs: make ->encode_fh() a mandatory method for NFS export
  exportfs: define FILEID_INO64_GEN* file handle types
  exportfs: support encoding non-decodeable file handles by default

 Documentation/filesystems/nfs/exporting.rst |  7 +--
 Documentation/filesystems/porting.rst       |  9 ++++
 fs/affs/namei.c                             |  1 +
 fs/befs/linuxvfs.c                          |  1 +
 fs/efs/super.c                              |  1 +
 fs/erofs/super.c                            |  1 +
 fs/exportfs/expfs.c                         | 54 +++++++++++++++------
 fs/ext2/super.c                             |  1 +
 fs/ext4/super.c                             |  1 +
 fs/f2fs/super.c                             |  1 +
 fs/fat/nfs.c                                |  1 +
 fs/fhandle.c                                |  6 +--
 fs/fuse/inode.c                             |  7 +--
 fs/jffs2/super.c                            |  1 +
 fs/jfs/super.c                              |  1 +
 fs/nfsd/export.c                            |  3 +-
 fs/notify/fanotify/fanotify_user.c          |  4 +-
 fs/ntfs/namei.c                             |  1 +
 fs/ntfs3/super.c                            |  1 +
 fs/overlayfs/util.c                         |  2 +-
 fs/smb/client/export.c                      | 11 ++---
 fs/squashfs/export.c                        |  1 +
 fs/ufs/super.c                              |  1 +
 include/linux/exportfs.h                    | 51 ++++++++++++++++++-
 24 files changed, 128 insertions(+), 40 deletions(-)

-- 
2.34.1

[PATCH v2 1/4] exportfs: add helpers to check if filesystem can encode/decode file handles

[Amir Goldstein]

The logic of whether filesystem can encode/decode file handles is open
coded in many places.

In preparation to changing the logic, move the open coded logic into
inline helpers.

Reviewed-by: Jan Kara <jack@suse.cz>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/exportfs/expfs.c                |  8 ++------
 fs/fhandle.c                       |  6 +-----
 fs/nfsd/export.c                   |  3 +--
 fs/notify/fanotify/fanotify_user.c |  4 ++--
 fs/overlayfs/util.c                |  2 +-
 include/linux/exportfs.h           | 27 +++++++++++++++++++++++++++
 6 files changed, 34 insertions(+), 16 deletions(-)

diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
index c20704aa21b3..9ee205df8fa7 100644
--- a/fs/exportfs/expfs.c
+++ b/fs/exportfs/expfs.c
@@ -396,11 +396,7 @@ int exportfs_encode_inode_fh(struct inode *inode, struct fid *fid,
 {
 	const struct export_operations *nop = inode->i_sb->s_export_op;
 
-	/*
-	 * If a decodeable file handle was requested, we need to make sure that
-	 * filesystem can decode file handles.
-	 */
-	if (nop && !(flags & EXPORT_FH_FID) && !nop->fh_to_dentry)
+	if (!exportfs_can_encode_fh(nop, flags))
 		return -EOPNOTSUPP;
 
 	if (nop && nop->encode_fh)
@@ -456,7 +452,7 @@ exportfs_decode_fh_raw(struct vfsmount *mnt, struct fid *fid, int fh_len,
 	/*
 	 * Try to get any dentry for the given file handle from the filesystem.
 	 */
-	if (!nop || !nop->fh_to_dentry)
+	if (!exportfs_can_decode_fh(nop))
 		return ERR_PTR(-ESTALE);
 	result = nop->fh_to_dentry(mnt->mnt_sb, fid, fh_len, fileid_type);
 	if (IS_ERR_OR_NULL(result))
diff --git a/fs/fhandle.c b/fs/fhandle.c
index 6ea8d35a9382..18b3ba8dc8ea 100644
--- a/fs/fhandle.c
+++ b/fs/fhandle.c
@@ -26,12 +26,8 @@ static long do_sys_name_to_handle(const struct path *path,
 	/*
 	 * We need to make sure whether the file system support decoding of
 	 * the file handle if decodeable file handle was requested.
-	 * Otherwise, even empty export_operations are sufficient to opt-in
-	 * to encoding FIDs.
 	 */
-	if (!path->dentry->d_sb->s_export_op ||
-	    (!(fh_flags & EXPORT_FH_FID) &&
-	     !path->dentry->d_sb->s_export_op->fh_to_dentry))
+	if (!exportfs_can_encode_fh(path->dentry->d_sb->s_export_op, fh_flags))
 		return -EOPNOTSUPP;
 
 	if (copy_from_user(&f_handle, ufh, sizeof(struct file_handle)))
diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
index 11a0eaa2f914..dc99dfc1d411 100644
--- a/fs/nfsd/export.c
+++ b/fs/nfsd/export.c
@@ -421,8 +421,7 @@ static int check_export(struct path *path, int *flags, unsigned char *uuid)
 		return -EINVAL;
 	}
 
-	if (!inode->i_sb->s_export_op ||
-	    !inode->i_sb->s_export_op->fh_to_dentry) {
+	if (!exportfs_can_decode_fh(inode->i_sb->s_export_op)) {
 		dprintk("exp_export: export of invalid fs type.\n");
 		return -EINVAL;
 	}
diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index 62fe0b679e58..0eb9622e8a9f 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -1595,7 +1595,7 @@ static int fanotify_test_fid(struct dentry *dentry, unsigned int flags)
 	 * file handles so user can use name_to_handle_at() to compare fids
 	 * reported with events to the file handle of watched objects.
 	 */
-	if (!nop)
+	if (!exportfs_can_encode_fid(nop))
 		return -EOPNOTSUPP;
 
 	/*
@@ -1603,7 +1603,7 @@ static int fanotify_test_fid(struct dentry *dentry, unsigned int flags)
 	 * supports decoding file handles, so user has a way to map back the
 	 * reported fids to filesystem objects.
 	 */
-	if (mark_type != FAN_MARK_INODE && !nop->fh_to_dentry)
+	if (mark_type != FAN_MARK_INODE && !exportfs_can_decode_fh(nop))
 		return -EOPNOTSUPP;
 
 	return 0;
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index 89e0d60d35b6..f0a712214ec2 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -55,7 +55,7 @@ int ovl_can_decode_fh(struct super_block *sb)
 	if (!capable(CAP_DAC_READ_SEARCH))
 		return 0;
 
-	if (!sb->s_export_op || !sb->s_export_op->fh_to_dentry)
+	if (!exportfs_can_decode_fh(sb->s_export_op))
 		return 0;
 
 	return sb->s_export_op->encode_fh ? -1 : FILEID_INO32_GEN;
diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
index 11fbd0ee1370..5b3c9f30b422 100644
--- a/include/linux/exportfs.h
+++ b/include/linux/exportfs.h
@@ -233,6 +233,33 @@ extern int exportfs_encode_inode_fh(struct inode *inode, struct fid *fid,
 extern int exportfs_encode_fh(struct dentry *dentry, struct fid *fid,
 			      int *max_len, int flags);
 
+static inline bool exportfs_can_encode_fid(const struct export_operations *nop)
+{
+	return nop;
+}
+
+static inline bool exportfs_can_decode_fh(const struct export_operations *nop)
+{
+	return nop && nop->fh_to_dentry;
+}
+
+static inline bool exportfs_can_encode_fh(const struct export_operations *nop,
+					  int fh_flags)
+{
+	/*
+	 * If a non-decodeable file handle was requested, we only need to make
+	 * sure that filesystem can encode file handles.
+	 */
+	if (fh_flags & EXPORT_FH_FID)
+		return exportfs_can_encode_fid(nop);
+
+	/*
+	 * If a decodeable file handle was requested, we need to make sure that
+	 * filesystem can also decode file handles.
+	 */
+	return exportfs_can_decode_fh(nop);
+}
+
 static inline int exportfs_encode_fid(struct inode *inode, struct fid *fid,
 				      int *max_len)
 {
-- 
2.34.1

Re: [PATCH v2 1/4] exportfs: add helpers to check if filesystem can encode/decode file handles

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH v2 2/4] exportfs: make ->encode_fh() a mandatory method for NFS export

[Amir Goldstein]

export_operations ->encode_fh() no longer has a default implementation to
encode FILEID_INO32_GEN* file handles.

Rename the default helper for encoding FILEID_INO32_GEN* file handles to
generic_encode_ino32_fh() and convert the filesystems that used the
default implementation to use the generic helper explicitly.

This is a step towards allowing filesystems to encode non-decodeable file
handles for fanotify without having to implement any export_operations.

Reviewed-by: Jan Kara <jack@suse.cz>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Acked-by: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 Documentation/filesystems/nfs/exporting.rst |  7 ++-----
 Documentation/filesystems/porting.rst       |  9 +++++++++
 fs/affs/namei.c                             |  1 +
 fs/befs/linuxvfs.c                          |  1 +
 fs/efs/super.c                              |  1 +
 fs/erofs/super.c                            |  1 +
 fs/exportfs/expfs.c                         | 16 +++++++++-------
 fs/ext2/super.c                             |  1 +
 fs/ext4/super.c                             |  1 +
 fs/f2fs/super.c                             |  1 +
 fs/fat/nfs.c                                |  1 +
 fs/jffs2/super.c                            |  1 +
 fs/jfs/super.c                              |  1 +
 fs/ntfs/namei.c                             |  1 +
 fs/ntfs3/super.c                            |  1 +
 fs/smb/client/export.c                      | 11 +++++------
 fs/squashfs/export.c                        |  1 +
 fs/ufs/super.c                              |  1 +
 include/linux/exportfs.h                    |  9 ++++++++-
 19 files changed, 47 insertions(+), 19 deletions(-)

diff --git a/Documentation/filesystems/nfs/exporting.rst b/Documentation/filesystems/nfs/exporting.rst
index 4b30daee399a..de64d2d002a2 100644
--- a/Documentation/filesystems/nfs/exporting.rst
+++ b/Documentation/filesystems/nfs/exporting.rst
@@ -122,12 +122,9 @@ are exportable by setting the s_export_op field in the struct
 super_block.  This field must point to a "struct export_operations"
 struct which has the following members:
 
-  encode_fh (optional)
+  encode_fh (mandatory)
     Takes a dentry and creates a filehandle fragment which may later be used
-    to find or create a dentry for the same object.  The default
-    implementation creates a filehandle fragment that encodes a 32bit inode
-    and generation number for the inode encoded, and if necessary the
-    same information for the parent.
+    to find or create a dentry for the same object.
 
   fh_to_dentry (mandatory)
     Given a filehandle fragment, this should find the implied object and
diff --git a/Documentation/filesystems/porting.rst b/Documentation/filesystems/porting.rst
index 4d05b9862451..9cc6cb27c4d5 100644
--- a/Documentation/filesystems/porting.rst
+++ b/Documentation/filesystems/porting.rst
@@ -1045,3 +1045,12 @@ filesystem type is now moved to a later point when the devices are closed:
 As this is a VFS level change it has no practical consequences for filesystems
 other than that all of them must use one of the provided kill_litter_super(),
 kill_anon_super(), or kill_block_super() helpers.
+
+---
+
+**mandatory**
+
+export_operations ->encode_fh() no longer has a default implementation to
+encode FILEID_INO32_GEN* file handles.
+Filesystems that used the default implementation may use the generic helper
+generic_encode_ino32_fh() explicitly.
diff --git a/fs/affs/namei.c b/fs/affs/namei.c
index 2fe4a5832fcf..d6b9758ee23d 100644
--- a/fs/affs/namei.c
+++ b/fs/affs/namei.c
@@ -568,6 +568,7 @@ static struct dentry *affs_fh_to_parent(struct super_block *sb, struct fid *fid,
 }
 
 const struct export_operations affs_export_ops = {
+	.encode_fh = generic_encode_ino32_fh,
 	.fh_to_dentry = affs_fh_to_dentry,
 	.fh_to_parent = affs_fh_to_parent,
 	.get_parent = affs_get_parent,
diff --git a/fs/befs/linuxvfs.c b/fs/befs/linuxvfs.c
index 9a16a51fbb88..410dcaffd5ab 100644
--- a/fs/befs/linuxvfs.c
+++ b/fs/befs/linuxvfs.c
@@ -96,6 +96,7 @@ static const struct address_space_operations befs_symlink_aops = {
 };
 
 static const struct export_operations befs_export_operations = {
+	.encode_fh	= generic_encode_ino32_fh,
 	.fh_to_dentry	= befs_fh_to_dentry,
 	.fh_to_parent	= befs_fh_to_parent,
 	.get_parent	= befs_get_parent,
diff --git a/fs/efs/super.c b/fs/efs/super.c
index b287f47c165b..f17fdac76b2e 100644
--- a/fs/efs/super.c
+++ b/fs/efs/super.c
@@ -123,6 +123,7 @@ static const struct super_operations efs_superblock_operations = {
 };
 
 static const struct export_operations efs_export_ops = {
+	.encode_fh	= generic_encode_ino32_fh,
 	.fh_to_dentry	= efs_fh_to_dentry,
 	.fh_to_parent	= efs_fh_to_parent,
 	.get_parent	= efs_get_parent,
diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 3700af9ee173..edbe07a24156 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -626,6 +626,7 @@ static struct dentry *erofs_get_parent(struct dentry *child)
 }
 
 static const struct export_operations erofs_export_ops = {
+	.encode_fh = generic_encode_ino32_fh,
 	.fh_to_dentry = erofs_fh_to_dentry,
 	.fh_to_parent = erofs_fh_to_parent,
 	.get_parent = erofs_get_parent,
diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
index 9ee205df8fa7..8f883c4758f5 100644
--- a/fs/exportfs/expfs.c
+++ b/fs/exportfs/expfs.c
@@ -343,20 +343,21 @@ static int get_name(const struct path *path, char *name, struct dentry *child)
 }
 
 /**
- * export_encode_fh - default export_operations->encode_fh function
+ * generic_encode_ino32_fh - generic export_operations->encode_fh function
  * @inode:   the object to encode
- * @fid:     where to store the file handle fragment
- * @max_len: maximum length to store there
+ * @fh:      where to store the file handle fragment
+ * @max_len: maximum length to store there (in 4 byte units)
  * @parent:  parent directory inode, if wanted
  *
- * This default encode_fh function assumes that the 32 inode number
+ * This generic encode_fh function assumes that the 32 inode number
  * is suitable for locating an inode, and that the generation number
  * can be used to check that it is still valid.  It places them in the
  * filehandle fragment where export_decode_fh expects to find them.
  */
-static int export_encode_fh(struct inode *inode, struct fid *fid,
-		int *max_len, struct inode *parent)
+int generic_encode_ino32_fh(struct inode *inode, __u32 *fh, int *max_len,
+			    struct inode *parent)
 {
+	struct fid *fid = (void *)fh;
 	int len = *max_len;
 	int type = FILEID_INO32_GEN;
 
@@ -380,6 +381,7 @@ static int export_encode_fh(struct inode *inode, struct fid *fid,
 	*max_len = len;
 	return type;
 }
+EXPORT_SYMBOL_GPL(generic_encode_ino32_fh);
 
 /**
  * exportfs_encode_inode_fh - encode a file handle from inode
@@ -402,7 +404,7 @@ int exportfs_encode_inode_fh(struct inode *inode, struct fid *fid,
 	if (nop && nop->encode_fh)
 		return nop->encode_fh(inode, fid->raw, max_len, parent);
 
-	return export_encode_fh(inode, fid, max_len, parent);
+	return -EOPNOTSUPP;
 }
 EXPORT_SYMBOL_GPL(exportfs_encode_inode_fh);
 
diff --git a/fs/ext2/super.c b/fs/ext2/super.c
index aaf3e3e88cb2..b9f158a34997 100644
--- a/fs/ext2/super.c
+++ b/fs/ext2/super.c
@@ -397,6 +397,7 @@ static struct dentry *ext2_fh_to_parent(struct super_block *sb, struct fid *fid,
 }
 
 static const struct export_operations ext2_export_ops = {
+	.encode_fh = generic_encode_ino32_fh,
 	.fh_to_dentry = ext2_fh_to_dentry,
 	.fh_to_parent = ext2_fh_to_parent,
 	.get_parent = ext2_get_parent,
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index dbebd8b3127e..c44db1915437 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1646,6 +1646,7 @@ static const struct super_operations ext4_sops = {
 };
 
 static const struct export_operations ext4_export_ops = {
+	.encode_fh = generic_encode_ino32_fh,
 	.fh_to_dentry = ext4_fh_to_dentry,
 	.fh_to_parent = ext4_fh_to_parent,
 	.get_parent = ext4_get_parent,
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index a8c8232852bb..60cfa11f65bf 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -3282,6 +3282,7 @@ static struct dentry *f2fs_fh_to_parent(struct super_block *sb, struct fid *fid,
 }
 
 static const struct export_operations f2fs_export_ops = {
+	.encode_fh = generic_encode_ino32_fh,
 	.fh_to_dentry = f2fs_fh_to_dentry,
 	.fh_to_parent = f2fs_fh_to_parent,
 	.get_parent = f2fs_get_parent,
diff --git a/fs/fat/nfs.c b/fs/fat/nfs.c
index 3626eb585a98..c52e63e10d35 100644
--- a/fs/fat/nfs.c
+++ b/fs/fat/nfs.c
@@ -279,6 +279,7 @@ static struct dentry *fat_get_parent(struct dentry *child_dir)
 }
 
 const struct export_operations fat_export_ops = {
+	.encode_fh	= generic_encode_ino32_fh,
 	.fh_to_dentry   = fat_fh_to_dentry,
 	.fh_to_parent   = fat_fh_to_parent,
 	.get_parent     = fat_get_parent,
diff --git a/fs/jffs2/super.c b/fs/jffs2/super.c
index 7ea37f49f1e1..f99591a634b4 100644
--- a/fs/jffs2/super.c
+++ b/fs/jffs2/super.c
@@ -150,6 +150,7 @@ static struct dentry *jffs2_get_parent(struct dentry *child)
 }
 
 static const struct export_operations jffs2_export_ops = {
+	.encode_fh = generic_encode_ino32_fh,
 	.get_parent = jffs2_get_parent,
 	.fh_to_dentry = jffs2_fh_to_dentry,
 	.fh_to_parent = jffs2_fh_to_parent,
diff --git a/fs/jfs/super.c b/fs/jfs/super.c
index 2e2f7f6d36a0..2cc2632f3c47 100644
--- a/fs/jfs/super.c
+++ b/fs/jfs/super.c
@@ -896,6 +896,7 @@ static const struct super_operations jfs_super_operations = {
 };
 
 static const struct export_operations jfs_export_operations = {
+	.encode_fh	= generic_encode_ino32_fh,
 	.fh_to_dentry	= jfs_fh_to_dentry,
 	.fh_to_parent	= jfs_fh_to_parent,
 	.get_parent	= jfs_get_parent,
diff --git a/fs/ntfs/namei.c b/fs/ntfs/namei.c
index ab44f2db533b..d7498ddc4a72 100644
--- a/fs/ntfs/namei.c
+++ b/fs/ntfs/namei.c
@@ -384,6 +384,7 @@ static struct dentry *ntfs_fh_to_parent(struct super_block *sb, struct fid *fid,
  * and due to using iget() whereas NTFS needs ntfs_iget().
  */
 const struct export_operations ntfs_export_ops = {
+	.encode_fh	= generic_encode_ino32_fh,
 	.get_parent	= ntfs_get_parent,	/* Find the parent of a given
 						   directory. */
 	.fh_to_dentry	= ntfs_fh_to_dentry,
diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index 5661a363005e..661ffb5aa1e0 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -789,6 +789,7 @@ static int ntfs_nfs_commit_metadata(struct inode *inode)
 }
 
 static const struct export_operations ntfs_export_ops = {
+	.encode_fh = generic_encode_ino32_fh,
 	.fh_to_dentry = ntfs_fh_to_dentry,
 	.fh_to_parent = ntfs_fh_to_parent,
 	.get_parent = ntfs3_get_parent,
diff --git a/fs/smb/client/export.c b/fs/smb/client/export.c
index 37c28415df1e..d606e8cbcb7d 100644
--- a/fs/smb/client/export.c
+++ b/fs/smb/client/export.c
@@ -41,13 +41,12 @@ static struct dentry *cifs_get_parent(struct dentry *dentry)
 }
 
 const struct export_operations cifs_export_ops = {
+	.encode_fh = generic_encode_ino32_fh,
 	.get_parent = cifs_get_parent,
-/*	Following five export operations are unneeded so far and can default:
-	.get_dentry =
-	.get_name =
-	.find_exported_dentry =
-	.decode_fh =
-	.encode_fs =  */
+/*
+ * Following export operations are mandatory for NFS export support:
+ *	.fh_to_dentry =
+ */
 };
 
 #endif /* CONFIG_CIFS_NFSD_EXPORT */
diff --git a/fs/squashfs/export.c b/fs/squashfs/export.c
index 723763746238..62972f0ff868 100644
--- a/fs/squashfs/export.c
+++ b/fs/squashfs/export.c
@@ -173,6 +173,7 @@ __le64 *squashfs_read_inode_lookup_table(struct super_block *sb,
 
 
 const struct export_operations squashfs_export_ops = {
+	.encode_fh = generic_encode_ino32_fh,
 	.fh_to_dentry = squashfs_fh_to_dentry,
 	.fh_to_parent = squashfs_fh_to_parent,
 	.get_parent = squashfs_get_parent
diff --git a/fs/ufs/super.c b/fs/ufs/super.c
index 23377c1baed9..a480810cd4e3 100644
--- a/fs/ufs/super.c
+++ b/fs/ufs/super.c
@@ -137,6 +137,7 @@ static struct dentry *ufs_get_parent(struct dentry *child)
 }
 
 static const struct export_operations ufs_export_ops = {
+	.encode_fh = generic_encode_ino32_fh,
 	.fh_to_dentry	= ufs_fh_to_dentry,
 	.fh_to_parent	= ufs_fh_to_parent,
 	.get_parent	= ufs_get_parent,
diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
index 5b3c9f30b422..85bd027494e5 100644
--- a/include/linux/exportfs.h
+++ b/include/linux/exportfs.h
@@ -235,7 +235,7 @@ extern int exportfs_encode_fh(struct dentry *dentry, struct fid *fid,
 
 static inline bool exportfs_can_encode_fid(const struct export_operations *nop)
 {
-	return nop;
+	return nop && nop->encode_fh;
 }
 
 static inline bool exportfs_can_decode_fh(const struct export_operations *nop)
@@ -279,6 +279,13 @@ extern struct dentry *exportfs_decode_fh(struct vfsmount *mnt, struct fid *fid,
 /*
  * Generic helpers for filesystems.
  */
+#ifdef CONFIG_EXPORTFS
+int generic_encode_ino32_fh(struct inode *inode, __u32 *fh, int *max_len,
+			    struct inode *parent);
+#else
+#define generic_encode_ino32_fh NULL
+#endif
+
 extern struct dentry *generic_fh_to_dentry(struct super_block *sb,
 	struct fid *fid, int fh_len, int fh_type,
 	struct inode *(*get_inode) (struct super_block *sb, u64 ino, u32 gen));
-- 
2.34.1

Re: [PATCH v2 2/4] exportfs: make ->encode_fh() a mandatory method for NFS export

[Dave Kleikamp]

On 10/23/23 1:07PM, Amir Goldstein wrote:
> export_operations ->encode_fh() no longer has a default implementation to
> encode FILEID_INO32_GEN* file handles.
> 
> Rename the default helper for encoding FILEID_INO32_GEN* file handles to
> generic_encode_ino32_fh() and convert the filesystems that used the
> default implementation to use the generic helper explicitly.
> 
> This is a step towards allowing filesystems to encode non-decodeable file
> handles for fanotify without having to implement any export_operations.
> 
> Reviewed-by: Jan Kara <jack@suse.cz>
> Reviewed-by: Jeff Layton <jlayton@kernel.org>
> Acked-by: Chuck Lever <chuck.lever@oracle.com>
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>

Acked-by: Dave Kleikamp <dave.kleikamp@oracle.com>
> ---
>   Documentation/filesystems/nfs/exporting.rst |  7 ++-----
>   Documentation/filesystems/porting.rst       |  9 +++++++++
>   fs/affs/namei.c                             |  1 +
>   fs/befs/linuxvfs.c                          |  1 +
>   fs/efs/super.c                              |  1 +
>   fs/erofs/super.c                            |  1 +
>   fs/exportfs/expfs.c                         | 16 +++++++++-------
>   fs/ext2/super.c                             |  1 +
>   fs/ext4/super.c                             |  1 +
>   fs/f2fs/super.c                             |  1 +
>   fs/fat/nfs.c                                |  1 +
>   fs/jffs2/super.c                            |  1 +
>   fs/jfs/super.c                              |  1 +
>   fs/ntfs/namei.c                             |  1 +
>   fs/ntfs3/super.c                            |  1 +
>   fs/smb/client/export.c                      | 11 +++++------
>   fs/squashfs/export.c                        |  1 +
>   fs/ufs/super.c                              |  1 +
>   include/linux/exportfs.h                    |  9 ++++++++-
>   19 files changed, 47 insertions(+), 19 deletions(-)
> 
> diff --git a/Documentation/filesystems/nfs/exporting.rst b/Documentation/filesystems/nfs/exporting.rst
> index 4b30daee399a..de64d2d002a2 100644
> --- a/Documentation/filesystems/nfs/exporting.rst
> +++ b/Documentation/filesystems/nfs/exporting.rst
> @@ -122,12 +122,9 @@ are exportable by setting the s_export_op field in the struct
>   super_block.  This field must point to a "struct export_operations"
>   struct which has the following members:
>   
> -  encode_fh (optional)
> +  encode_fh (mandatory)
>       Takes a dentry and creates a filehandle fragment which may later be used
> -    to find or create a dentry for the same object.  The default
> -    implementation creates a filehandle fragment that encodes a 32bit inode
> -    and generation number for the inode encoded, and if necessary the
> -    same information for the parent.
> +    to find or create a dentry for the same object.
>   
>     fh_to_dentry (mandatory)
>       Given a filehandle fragment, this should find the implied object and
> diff --git a/Documentation/filesystems/porting.rst b/Documentation/filesystems/porting.rst
> index 4d05b9862451..9cc6cb27c4d5 100644
> --- a/Documentation/filesystems/porting.rst
> +++ b/Documentation/filesystems/porting.rst
> @@ -1045,3 +1045,12 @@ filesystem type is now moved to a later point when the devices are closed:
>   As this is a VFS level change it has no practical consequences for filesystems
>   other than that all of them must use one of the provided kill_litter_super(),
>   kill_anon_super(), or kill_block_super() helpers.
> +
> +---
> +
> +**mandatory**
> +
> +export_operations ->encode_fh() no longer has a default implementation to
> +encode FILEID_INO32_GEN* file handles.
> +Filesystems that used the default implementation may use the generic helper
> +generic_encode_ino32_fh() explicitly.
> diff --git a/fs/affs/namei.c b/fs/affs/namei.c
> index 2fe4a5832fcf..d6b9758ee23d 100644
> --- a/fs/affs/namei.c
> +++ b/fs/affs/namei.c
> @@ -568,6 +568,7 @@ static struct dentry *affs_fh_to_parent(struct super_block *sb, struct fid *fid,
>   }
>   
>   const struct export_operations affs_export_ops = {
> +	.encode_fh = generic_encode_ino32_fh,
>   	.fh_to_dentry = affs_fh_to_dentry,
>   	.fh_to_parent = affs_fh_to_parent,
>   	.get_parent = affs_get_parent,
> diff --git a/fs/befs/linuxvfs.c b/fs/befs/linuxvfs.c
> index 9a16a51fbb88..410dcaffd5ab 100644
> --- a/fs/befs/linuxvfs.c
> +++ b/fs/befs/linuxvfs.c
> @@ -96,6 +96,7 @@ static const struct address_space_operations befs_symlink_aops = {
>   };
>   
>   static const struct export_operations befs_export_operations = {
> +	.encode_fh	= generic_encode_ino32_fh,
>   	.fh_to_dentry	= befs_fh_to_dentry,
>   	.fh_to_parent	= befs_fh_to_parent,
>   	.get_parent	= befs_get_parent,
> diff --git a/fs/efs/super.c b/fs/efs/super.c
> index b287f47c165b..f17fdac76b2e 100644
> --- a/fs/efs/super.c
> +++ b/fs/efs/super.c
> @@ -123,6 +123,7 @@ static const struct super_operations efs_superblock_operations = {
>   };
>   
>   static const struct export_operations efs_export_ops = {
> +	.encode_fh	= generic_encode_ino32_fh,
>   	.fh_to_dentry	= efs_fh_to_dentry,
>   	.fh_to_parent	= efs_fh_to_parent,
>   	.get_parent	= efs_get_parent,
> diff --git a/fs/erofs/super.c b/fs/erofs/super.c
> index 3700af9ee173..edbe07a24156 100644
> --- a/fs/erofs/super.c
> +++ b/fs/erofs/super.c
> @@ -626,6 +626,7 @@ static struct dentry *erofs_get_parent(struct dentry *child)
>   }
>   
>   static const struct export_operations erofs_export_ops = {
> +	.encode_fh = generic_encode_ino32_fh,
>   	.fh_to_dentry = erofs_fh_to_dentry,
>   	.fh_to_parent = erofs_fh_to_parent,
>   	.get_parent = erofs_get_parent,
> diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
> index 9ee205df8fa7..8f883c4758f5 100644
> --- a/fs/exportfs/expfs.c
> +++ b/fs/exportfs/expfs.c
> @@ -343,20 +343,21 @@ static int get_name(const struct path *path, char *name, struct dentry *child)
>   }
>   
>   /**
> - * export_encode_fh - default export_operations->encode_fh function
> + * generic_encode_ino32_fh - generic export_operations->encode_fh function
>    * @inode:   the object to encode
> - * @fid:     where to store the file handle fragment
> - * @max_len: maximum length to store there
> + * @fh:      where to store the file handle fragment
> + * @max_len: maximum length to store there (in 4 byte units)
>    * @parent:  parent directory inode, if wanted
>    *
> - * This default encode_fh function assumes that the 32 inode number
> + * This generic encode_fh function assumes that the 32 inode number
>    * is suitable for locating an inode, and that the generation number
>    * can be used to check that it is still valid.  It places them in the
>    * filehandle fragment where export_decode_fh expects to find them.
>    */
> -static int export_encode_fh(struct inode *inode, struct fid *fid,
> -		int *max_len, struct inode *parent)
> +int generic_encode_ino32_fh(struct inode *inode, __u32 *fh, int *max_len,
> +			    struct inode *parent)
>   {
> +	struct fid *fid = (void *)fh;
>   	int len = *max_len;
>   	int type = FILEID_INO32_GEN;
>   
> @@ -380,6 +381,7 @@ static int export_encode_fh(struct inode *inode, struct fid *fid,
>   	*max_len = len;
>   	return type;
>   }
> +EXPORT_SYMBOL_GPL(generic_encode_ino32_fh);
>   
>   /**
>    * exportfs_encode_inode_fh - encode a file handle from inode
> @@ -402,7 +404,7 @@ int exportfs_encode_inode_fh(struct inode *inode, struct fid *fid,
>   	if (nop && nop->encode_fh)
>   		return nop->encode_fh(inode, fid->raw, max_len, parent);
>   
> -	return export_encode_fh(inode, fid, max_len, parent);
> +	return -EOPNOTSUPP;
>   }
>   EXPORT_SYMBOL_GPL(exportfs_encode_inode_fh);
>   
> diff --git a/fs/ext2/super.c b/fs/ext2/super.c
> index aaf3e3e88cb2..b9f158a34997 100644
> --- a/fs/ext2/super.c
> +++ b/fs/ext2/super.c
> @@ -397,6 +397,7 @@ static struct dentry *ext2_fh_to_parent(struct super_block *sb, struct fid *fid,
>   }
>   
>   static const struct export_operations ext2_export_ops = {
> +	.encode_fh = generic_encode_ino32_fh,
>   	.fh_to_dentry = ext2_fh_to_dentry,
>   	.fh_to_parent = ext2_fh_to_parent,
>   	.get_parent = ext2_get_parent,
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index dbebd8b3127e..c44db1915437 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -1646,6 +1646,7 @@ static const struct super_operations ext4_sops = {
>   };
>   
>   static const struct export_operations ext4_export_ops = {
> +	.encode_fh = generic_encode_ino32_fh,
>   	.fh_to_dentry = ext4_fh_to_dentry,
>   	.fh_to_parent = ext4_fh_to_parent,
>   	.get_parent = ext4_get_parent,
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index a8c8232852bb..60cfa11f65bf 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -3282,6 +3282,7 @@ static struct dentry *f2fs_fh_to_parent(struct super_block *sb, struct fid *fid,
>   }
>   
>   static const struct export_operations f2fs_export_ops = {
> +	.encode_fh = generic_encode_ino32_fh,
>   	.fh_to_dentry = f2fs_fh_to_dentry,
>   	.fh_to_parent = f2fs_fh_to_parent,
>   	.get_parent = f2fs_get_parent,
> diff --git a/fs/fat/nfs.c b/fs/fat/nfs.c
> index 3626eb585a98..c52e63e10d35 100644
> --- a/fs/fat/nfs.c
> +++ b/fs/fat/nfs.c
> @@ -279,6 +279,7 @@ static struct dentry *fat_get_parent(struct dentry *child_dir)
>   }
>   
>   const struct export_operations fat_export_ops = {
> +	.encode_fh	= generic_encode_ino32_fh,
>   	.fh_to_dentry   = fat_fh_to_dentry,
>   	.fh_to_parent   = fat_fh_to_parent,
>   	.get_parent     = fat_get_parent,
> diff --git a/fs/jffs2/super.c b/fs/jffs2/super.c
> index 7ea37f49f1e1..f99591a634b4 100644
> --- a/fs/jffs2/super.c
> +++ b/fs/jffs2/super.c
> @@ -150,6 +150,7 @@ static struct dentry *jffs2_get_parent(struct dentry *child)
>   }
>   
>   static const struct export_operations jffs2_export_ops = {
> +	.encode_fh = generic_encode_ino32_fh,
>   	.get_parent = jffs2_get_parent,
>   	.fh_to_dentry = jffs2_fh_to_dentry,
>   	.fh_to_parent = jffs2_fh_to_parent,
> diff --git a/fs/jfs/super.c b/fs/jfs/super.c
> index 2e2f7f6d36a0..2cc2632f3c47 100644
> --- a/fs/jfs/super.c
> +++ b/fs/jfs/super.c
> @@ -896,6 +896,7 @@ static const struct super_operations jfs_super_operations = {
>   };
>   
>   static const struct export_operations jfs_export_operations = {
> +	.encode_fh	= generic_encode_ino32_fh,
>   	.fh_to_dentry	= jfs_fh_to_dentry,
>   	.fh_to_parent	= jfs_fh_to_parent,
>   	.get_parent	= jfs_get_parent,
> diff --git a/fs/ntfs/namei.c b/fs/ntfs/namei.c
> index ab44f2db533b..d7498ddc4a72 100644
> --- a/fs/ntfs/namei.c
> +++ b/fs/ntfs/namei.c
> @@ -384,6 +384,7 @@ static struct dentry *ntfs_fh_to_parent(struct super_block *sb, struct fid *fid,
>    * and due to using iget() whereas NTFS needs ntfs_iget().
>    */
>   const struct export_operations ntfs_export_ops = {
> +	.encode_fh	= generic_encode_ino32_fh,
>   	.get_parent	= ntfs_get_parent,	/* Find the parent of a given
>   						   directory. */
>   	.fh_to_dentry	= ntfs_fh_to_dentry,
> diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
> index 5661a363005e..661ffb5aa1e0 100644
> --- a/fs/ntfs3/super.c
> +++ b/fs/ntfs3/super.c
> @@ -789,6 +789,7 @@ static int ntfs_nfs_commit_metadata(struct inode *inode)
>   }
>   
>   static const struct export_operations ntfs_export_ops = {
> +	.encode_fh = generic_encode_ino32_fh,
>   	.fh_to_dentry = ntfs_fh_to_dentry,
>   	.fh_to_parent = ntfs_fh_to_parent,
>   	.get_parent = ntfs3_get_parent,
> diff --git a/fs/smb/client/export.c b/fs/smb/client/export.c
> index 37c28415df1e..d606e8cbcb7d 100644
> --- a/fs/smb/client/export.c
> +++ b/fs/smb/client/export.c
> @@ -41,13 +41,12 @@ static struct dentry *cifs_get_parent(struct dentry *dentry)
>   }
>   
>   const struct export_operations cifs_export_ops = {
> +	.encode_fh = generic_encode_ino32_fh,
>   	.get_parent = cifs_get_parent,
> -/*	Following five export operations are unneeded so far and can default:
> -	.get_dentry =
> -	.get_name =
> -	.find_exported_dentry =
> -	.decode_fh =
> -	.encode_fs =  */
> +/*
> + * Following export operations are mandatory for NFS export support:
> + *	.fh_to_dentry =
> + */
>   };
>   
>   #endif /* CONFIG_CIFS_NFSD_EXPORT */
> diff --git a/fs/squashfs/export.c b/fs/squashfs/export.c
> index 723763746238..62972f0ff868 100644
> --- a/fs/squashfs/export.c
> +++ b/fs/squashfs/export.c
> @@ -173,6 +173,7 @@ __le64 *squashfs_read_inode_lookup_table(struct super_block *sb,
>   
>   
>   const struct export_operations squashfs_export_ops = {
> +	.encode_fh = generic_encode_ino32_fh,
>   	.fh_to_dentry = squashfs_fh_to_dentry,
>   	.fh_to_parent = squashfs_fh_to_parent,
>   	.get_parent = squashfs_get_parent
> diff --git a/fs/ufs/super.c b/fs/ufs/super.c
> index 23377c1baed9..a480810cd4e3 100644
> --- a/fs/ufs/super.c
> +++ b/fs/ufs/super.c
> @@ -137,6 +137,7 @@ static struct dentry *ufs_get_parent(struct dentry *child)
>   }
>   
>   static const struct export_operations ufs_export_ops = {
> +	.encode_fh = generic_encode_ino32_fh,
>   	.fh_to_dentry	= ufs_fh_to_dentry,
>   	.fh_to_parent	= ufs_fh_to_parent,
>   	.get_parent	= ufs_get_parent,
> diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
> index 5b3c9f30b422..85bd027494e5 100644
> --- a/include/linux/exportfs.h
> +++ b/include/linux/exportfs.h
> @@ -235,7 +235,7 @@ extern int exportfs_encode_fh(struct dentry *dentry, struct fid *fid,
>   
>   static inline bool exportfs_can_encode_fid(const struct export_operations *nop)
>   {
> -	return nop;
> +	return nop && nop->encode_fh;
>   }
>   
>   static inline bool exportfs_can_decode_fh(const struct export_operations *nop)
> @@ -279,6 +279,13 @@ extern struct dentry *exportfs_decode_fh(struct vfsmount *mnt, struct fid *fid,
>   /*
>    * Generic helpers for filesystems.
>    */
> +#ifdef CONFIG_EXPORTFS
> +int generic_encode_ino32_fh(struct inode *inode, __u32 *fh, int *max_len,
> +			    struct inode *parent);
> +#else
> +#define generic_encode_ino32_fh NULL
> +#endif
> +
>   extern struct dentry *generic_fh_to_dentry(struct super_block *sb,
>   	struct fid *fid, int fh_len, int fh_type,
>   	struct inode *(*get_inode) (struct super_block *sb, u64 ino, u32 gen));

Re: [PATCH v2 2/4] exportfs: make ->encode_fh() a mandatory method for NFS export

[Christoph Hellwig]

On Mon, Oct 23, 2023 at 09:07:59PM +0300, Amir Goldstein wrote:
> export_operations ->encode_fh() no longer has a default implementation to
> encode FILEID_INO32_GEN* file handles.

This statement reads like a factual statement about the current tree.
I'd suggest rewording it to make clear that you are changing the
behavior so that the defaul goes away, and I'd also suggest to move
it after the next paragraph.

Re: [PATCH v2 2/4] exportfs: make ->encode_fh() a mandatory method for NFS export

[Amir Goldstein]

On Fri, Oct 27, 2023 at 9:03 AM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Mon, Oct 23, 2023 at 09:07:59PM +0300, Amir Goldstein wrote:
> > export_operations ->encode_fh() no longer has a default implementation to
> > encode FILEID_INO32_GEN* file handles.
>
> This statement reads like a factual statement about the current tree.
> I'd suggest rewording it to make clear that you are changing the
> behavior so that the defaul goes away, and I'd also suggest to move
> it after the next paragraph.

Ok. will send v3 with those changes.

Thanks,
Amir.

Re: [PATCH v2 2/4] exportfs: make ->encode_fh() a mandatory method for NFS export

[Amir Goldstein]

On Fri, Oct 27, 2023 at 10:09 AM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Fri, Oct 27, 2023 at 9:03 AM Christoph Hellwig <hch@infradead.org> wrote:
> >
> > On Mon, Oct 23, 2023 at 09:07:59PM +0300, Amir Goldstein wrote:
> > > export_operations ->encode_fh() no longer has a default implementation to
> > > encode FILEID_INO32_GEN* file handles.
> >
> > This statement reads like a factual statement about the current tree.
> > I'd suggest rewording it to make clear that you are changing the
> > behavior so that the defaul goes away, and I'd also suggest to move
> > it after the next paragraph.
>
> Ok. will send v3 with those changes.
>

Actually, Christian, since you already picked up the build fix and
MAINTAINERS patch, cloud I bother you to fixup the commit
message of this patch according to Christoph's request:

    exportfs: make ->encode_fh() a mandatory method for NFS export

    Rename the default helper for encoding FILEID_INO32_GEN* file handles
    to generic_encode_ino32_fh() and convert the filesystems that used the
    default implementation to use the generic helper explicitly.

    After this change, exportfs_encode_inode_fh() no longer has a default
    implementation to encode FILEID_INO32_GEN* file handles.

    This is a step towards allowing filesystems to encode non-decodeable file
    handles for fanotify without having to implement any export_operations.


Might as well add hch RVB on patch #1 while at it.

Thanks,
Amir.

Re: [PATCH v2 2/4] exportfs: make ->encode_fh() a mandatory method for NFS export

[Christian Brauner]

> Actually, Christian, since you already picked up the build fix and
> MAINTAINERS patch, cloud I bother you to fixup the commit
> message of this patch according to Christoph's request:
> 
>     exportfs: make ->encode_fh() a mandatory method for NFS export
> 
>     Rename the default helper for encoding FILEID_INO32_GEN* file handles
>     to generic_encode_ino32_fh() and convert the filesystems that used the
>     default implementation to use the generic helper explicitly.
> 
>     After this change, exportfs_encode_inode_fh() no longer has a default
>     implementation to encode FILEID_INO32_GEN* file handles.
> 
>     This is a step towards allowing filesystems to encode non-decodeable file
>     handles for fanotify without having to implement any export_operations.
> 
> 
> Might as well add hch RVB on patch #1 while at it.

Done, please check in vfs.f_fsid and yell if something is wrong.

Re: [PATCH v2 2/4] exportfs: make ->encode_fh() a mandatory method for NFS export

[Amir Goldstein]

On Sat, Oct 28, 2023 at 5:16 PM Christian Brauner <brauner@kernel.org> wrote:
>
> > Actually, Christian, since you already picked up the build fix and
> > MAINTAINERS patch, cloud I bother you to fixup the commit
> > message of this patch according to Christoph's request:
> >
> >     exportfs: make ->encode_fh() a mandatory method for NFS export
> >
> >     Rename the default helper for encoding FILEID_INO32_GEN* file handles
> >     to generic_encode_ino32_fh() and convert the filesystems that used the
> >     default implementation to use the generic helper explicitly.
> >
> >     After this change, exportfs_encode_inode_fh() no longer has a default
> >     implementation to encode FILEID_INO32_GEN* file handles.
> >
> >     This is a step towards allowing filesystems to encode non-decodeable file
> >     handles for fanotify without having to implement any export_operations.
> >
> >
> > Might as well add hch RVB on patch #1 while at it.
>
> Done, please check in vfs.f_fsid and yell if something is wrong.

I see no changes.
Maybe you have forgotten to push the branch??

Thanks,
Amir.

Re: [PATCH v2 2/4] exportfs: make ->encode_fh() a mandatory method for NFS export

[Christian Brauner]

> > Done, please check in vfs.f_fsid and yell if something is wrong.
> 
> I see no changes.
> Maybe you have forgotten to push the branch??

I fixed it all up on Saturday but then didn't push until this morning.

Re: [PATCH v2 2/4] exportfs: make ->encode_fh() a mandatory method for NFS export

[Amir Goldstein]

On Mon, Oct 30, 2023 at 12:26 PM Christian Brauner <brauner@kernel.org> wrote:
>
> > > Done, please check in vfs.f_fsid and yell if something is wrong.
> >
> > I see no changes.
> > Maybe you have forgotten to push the branch??
>
> I fixed it all up on Saturday but then didn't push until this morning.

Looks good.

Although kernel testbot just complained about the CONFIG_EXORTFS=m build error
in the bisection of the current linux-next, so maybe we should squash
the fix patch.
I have no strong feelings about it, so whatever you decide.

Thanks,
Amir.

[PATCH v2 3/4] exportfs: define FILEID_INO64_GEN* file handle types

[Amir Goldstein]

Similar to the common FILEID_INO32* file handle types, define common
FILEID_INO64* file handle types.

The type values of FILEID_INO64_GEN and FILEID_INO64_GEN_PARENT are the
values returned by fuse and xfs for 64bit ino encoded file handle types.

Note that these type value are filesystem specific and they do not define
a universal file handle format, for example:
fuse encodes FILEID_INO64_GEN as [ino-hi32,ino-lo32,gen] and xfs encodes
FILEID_INO64_GEN as [hostr-order-ino64,gen] (a.k.a xfs_fid64).

The FILEID_INO64_GEN fhandle type is going to be used for file ids for
fanotify from filesystems that do not support NFS export.

Reviewed-by: Jan Kara <jack@suse.cz>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/fuse/inode.c          |  7 ++++---
 include/linux/exportfs.h | 11 +++++++++++
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 2e4eb7cf26fb..e63f966698a5 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -1002,7 +1002,7 @@ static int fuse_encode_fh(struct inode *inode, u32 *fh, int *max_len,
 	}
 
 	*max_len = len;
-	return parent ? 0x82 : 0x81;
+	return parent ? FILEID_INO64_GEN_PARENT : FILEID_INO64_GEN;
 }
 
 static struct dentry *fuse_fh_to_dentry(struct super_block *sb,
@@ -1010,7 +1010,8 @@ static struct dentry *fuse_fh_to_dentry(struct super_block *sb,
 {
 	struct fuse_inode_handle handle;
 
-	if ((fh_type != 0x81 && fh_type != 0x82) || fh_len < 3)
+	if ((fh_type != FILEID_INO64_GEN &&
+	     fh_type != FILEID_INO64_GEN_PARENT) || fh_len < 3)
 		return NULL;
 
 	handle.nodeid = (u64) fid->raw[0] << 32;
@@ -1024,7 +1025,7 @@ static struct dentry *fuse_fh_to_parent(struct super_block *sb,
 {
 	struct fuse_inode_handle parent;
 
-	if (fh_type != 0x82 || fh_len < 6)
+	if (fh_type != FILEID_INO64_GEN_PARENT || fh_len < 6)
 		return NULL;
 
 	parent.nodeid = (u64) fid->raw[3] << 32;
diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
index 85bd027494e5..4119d3ee72eb 100644
--- a/include/linux/exportfs.h
+++ b/include/linux/exportfs.h
@@ -98,6 +98,17 @@ enum fid_type {
 	 */
 	FILEID_FAT_WITH_PARENT = 0x72,
 
+	/*
+	 * 64 bit inode number, 32 bit generation number.
+	 */
+	FILEID_INO64_GEN = 0x81,
+
+	/*
+	 * 64 bit inode number, 32 bit generation number,
+	 * 64 bit parent inode number, 32 bit parent generation.
+	 */
+	FILEID_INO64_GEN_PARENT = 0x82,
+
 	/*
 	 * 128 bit child FID (struct lu_fid)
 	 * 128 bit parent FID (struct lu_fid)
-- 
2.34.1

Re: [PATCH v2 3/4] exportfs: define FILEID_INO64_GEN* file handle types

[Christoph Hellwig]

On Mon, Oct 23, 2023 at 09:08:00PM +0300, Amir Goldstein wrote:
> Similar to the common FILEID_INO32* file handle types, define common
> FILEID_INO64* file handle types.
> 
> The type values of FILEID_INO64_GEN and FILEID_INO64_GEN_PARENT are the
> values returned by fuse and xfs for 64bit ino encoded file handle types.

Please actually switch xfs to fully use the helpers instead of
duplicating the logic.  Presumable the same for fuse, but for that
I'd need to look at how it works for fuse right now and if there's not
some subtle differences.

Re: [PATCH v2 3/4] exportfs: define FILEID_INO64_GEN* file handle types

[Amir Goldstein]

On Fri, Oct 27, 2023 at 9:05 AM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Mon, Oct 23, 2023 at 09:08:00PM +0300, Amir Goldstein wrote:
> > Similar to the common FILEID_INO32* file handle types, define common
> > FILEID_INO64* file handle types.
> >
> > The type values of FILEID_INO64_GEN and FILEID_INO64_GEN_PARENT are the
> > values returned by fuse and xfs for 64bit ino encoded file handle types.
>
> Please actually switch xfs to fully use the helpers instead of
> duplicating the logic.

I will follow up with another patch.

> Presumable the same for fuse, but for that
> I'd need to look at how it works for fuse right now and if there's not
> some subtle differences.
>

There are subtle differences:
1. fuse encodes an internal nodeid - not i_ino
2. fuse encodes the inode number as [low32,high32]

It cannot use the generic helper.

Thanks,
Amir.

Re: [PATCH v2 3/4] exportfs: define FILEID_INO64_GEN* file handle types

[Christoph Hellwig]

On Fri, Oct 27, 2023 at 09:43:01AM +0300, Amir Goldstein wrote:
> > Presumable the same for fuse, but for that
> > I'd need to look at how it works for fuse right now and if there's not
> > some subtle differences.
> >
> 
> There are subtle differences:
> 1. fuse encodes an internal nodeid - not i_ino
> 2. fuse encodes the inode number as [low32,high32]
> 
> It cannot use the generic helper.

That's what I almost feared.  It still should use the common symbolic
name for the format just to make everyones life simpler.

Re: [PATCH v2 3/4] exportfs: define FILEID_INO64_GEN* file handle types

[Amir Goldstein]

On Fri, Oct 27, 2023 at 10:32 AM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Fri, Oct 27, 2023 at 09:43:01AM +0300, Amir Goldstein wrote:
> > > Presumable the same for fuse, but for that
> > > I'd need to look at how it works for fuse right now and if there's not
> > > some subtle differences.
> > >
> >
> > There are subtle differences:
> > 1. fuse encodes an internal nodeid - not i_ino
> > 2. fuse encodes the inode number as [low32,high32]

Sorry, that's [hi32,lo32] as written in commit message.

> >
> > It cannot use the generic helper.
>
> That's what I almost feared.  It still should use the common symbolic
> name for the format just to make everyones life simpler.

That's what I thought.
This patch converts fuse to use the new defined FILEID_INO64*
constants.

I plan to send a followup patch to xfs to use the symbolic name
after this constant has landed in vfs.
It is going to be easier than collaborating the merges of xfs and vfs
and there is no reason to rush it.

Thanks,
Amir.

[PATCH v2 4/4] exportfs: support encoding non-decodeable file handles by default

[Amir Goldstein]

AT_HANDLE_FID was added as an API for name_to_handle_at() that request
the encoding of a file id, which is not intended to be decoded.

This file id is used by fanotify to describe objects in events.

So far, overlayfs is the only filesystem that supports encoding
non-decodeable file ids, by providing export_operations with an
->encode_fh() method and without a ->decode_fh() method.

Add support for encoding non-decodeable file ids to all the filesystems
that do not provide export_operations, by encoding a file id of type
FILEID_INO64_GEN from { i_ino, i_generation }.

A filesystem may that does not support NFS export, can opt-out of
encoding non-decodeable file ids for fanotify by defining an empty
export_operations struct (i.e. with a NULL ->encode_fh() method).

This allows the use of fanotify events with file ids on filesystems
like 9p which do not support NFS export to bring fanotify in feature
parity with inotify on those filesystems.

Note that fanotify also requires that the filesystems report a non-null
fsid.  Currently, many simple filesystems that have support for inotify
(e.g. debugfs, tracefs, sysfs) report a null fsid, so can still not be
used with fanotify in file id reporting mode.

Reviewed-by: Jan Kara <jack@suse.cz>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/exportfs/expfs.c      | 32 +++++++++++++++++++++++++++++---
 include/linux/exportfs.h | 10 +++++++---
 2 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
index 8f883c4758f5..7d9fdcc187b7 100644
--- a/fs/exportfs/expfs.c
+++ b/fs/exportfs/expfs.c
@@ -383,6 +383,32 @@ int generic_encode_ino32_fh(struct inode *inode, __u32 *fh, int *max_len,
 }
 EXPORT_SYMBOL_GPL(generic_encode_ino32_fh);
 
+#define FILEID_INO64_GEN_LEN 3
+
+/**
+ * exportfs_encode_ino64_fid - encode non-decodeable 64bit ino file id
+ * @inode:   the object to encode
+ * @fid:     where to store the file handle fragment
+ * @max_len: maximum length to store there (in 4 byte units)
+ *
+ * This generic function is used to encode a non-decodeable file id for
+ * fanotify for filesystems that do not support NFS export.
+ */
+static int exportfs_encode_ino64_fid(struct inode *inode, struct fid *fid,
+				     int *max_len)
+{
+	if (*max_len < FILEID_INO64_GEN_LEN) {
+		*max_len = FILEID_INO64_GEN_LEN;
+		return FILEID_INVALID;
+	}
+
+	fid->i64.ino = inode->i_ino;
+	fid->i64.gen = inode->i_generation;
+	*max_len = FILEID_INO64_GEN_LEN;
+
+	return FILEID_INO64_GEN;
+}
+
 /**
  * exportfs_encode_inode_fh - encode a file handle from inode
  * @inode:   the object to encode
@@ -401,10 +427,10 @@ int exportfs_encode_inode_fh(struct inode *inode, struct fid *fid,
 	if (!exportfs_can_encode_fh(nop, flags))
 		return -EOPNOTSUPP;
 
-	if (nop && nop->encode_fh)
-		return nop->encode_fh(inode, fid->raw, max_len, parent);
+	if (!nop && (flags & EXPORT_FH_FID))
+		return exportfs_encode_ino64_fid(inode, fid, max_len);
 
-	return -EOPNOTSUPP;
+	return nop->encode_fh(inode, fid->raw, max_len, parent);
 }
 EXPORT_SYMBOL_GPL(exportfs_encode_inode_fh);
 
diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
index 4119d3ee72eb..21bae8bfeef1 100644
--- a/include/linux/exportfs.h
+++ b/include/linux/exportfs.h
@@ -134,7 +134,11 @@ struct fid {
 			u32 parent_ino;
 			u32 parent_gen;
 		} i32;
- 		struct {
+		struct {
+			u64 ino;
+			u32 gen;
+		} __packed i64;
+		struct {
  			u32 block;
  			u16 partref;
  			u16 parent_partref;
@@ -246,7 +250,7 @@ extern int exportfs_encode_fh(struct dentry *dentry, struct fid *fid,
 
 static inline bool exportfs_can_encode_fid(const struct export_operations *nop)
 {
-	return nop && nop->encode_fh;
+	return !nop || nop->encode_fh;
 }
 
 static inline bool exportfs_can_decode_fh(const struct export_operations *nop)
@@ -259,7 +263,7 @@ static inline bool exportfs_can_encode_fh(const struct export_operations *nop,
 {
 	/*
 	 * If a non-decodeable file handle was requested, we only need to make
-	 * sure that filesystem can encode file handles.
+	 * sure that filesystem did not opt-out of encoding fid.
 	 */
 	if (fh_flags & EXPORT_FH_FID)
 		return exportfs_can_encode_fid(nop);
-- 
2.34.1

Re: [PATCH v2 0/4] Support more filesystems with FAN_REPORT_FID

[Amir Goldstein]

On Mon, Oct 23, 2023 at 9:08 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> Christian,
>
> The grand plan is to be able to use fanotify with FAN_REPORT_FID as a
> drop-in replacement for inotify, but with current upstream, inotify is
> supported on all the filesystems and FAN_REPORT_FID only on a few.
>
> Making all filesystem support FAN_REPORT_FID requires that all
> filesystems will:
> 1. Support for AT_HANDLE_FID file handles
> 2. Report non-zero f_fsid
>
> This patch set takes care of the first requirement.
> Patches were reviewed by Jan and the nfsd maintainers.
>
> I have another patch in review [2] for adding non-zero f_fsid to many
> simple filesystems, but it is independent of this patch set, so no
> reason to couple them together.

Christian,

Jan has reviewed the independent f_fsid vfs patch [2], so if you
pick up this patch set, please also apply the f_fsid vfs patch.

This would allow changing "more" in the subject of this cover letter
(and possible PR subject) to "most" (i.e. all the simple filesystems
and all the filesystems that already report a non-zero f_fsid).

For the few remaining filesystems that still report zero f_fsid,
I will be sending independent patches to individual maintainers.
I had already posted f_fsid patches for gfs2 [3] and nfs [4].

Thanks,
Amir.

>
> Note that patch #2 touches many filesystems due to vfs API change,
> requiring an explicit ->encode_fh() method. I did not gets ACKs from
> all filesystem maintainers, but the change is trivial and does not
> change any logic.
>
> Thanks,
> Amir.
>
> Changes since v1 [1]:
> - Patch #1 already merged into v6.6-rc7
> - Fix build without CONFIG_EXPORTFS
> - Fix checkpatch warnings
> - Define symbolic constant for FILEID_INO64_GEN_LEN
> - Clarify documentation (units of) max_len argument
>
> [1] https://lore.kernel.org/r/20231018100000.2453965-1-amir73il@gmail.com/
> [2] https://lore.kernel.org/r/20231023143049.2944970-1-amir73il@gmail.com/

[3] https://lore.kernel.org/linux-fsdevel/20231024075535.2994553-1-amir73il@gmail.com/
[4] https://lore.kernel.org/linux-fsdevel/20231024110109.3007794-1-amir73il@gmail.com/

>
> Amir Goldstein (4):
>   exportfs: add helpers to check if filesystem can encode/decode file
>     handles
>   exportfs: make ->encode_fh() a mandatory method for NFS export
>   exportfs: define FILEID_INO64_GEN* file handle types
>   exportfs: support encoding non-decodeable file handles by default
>
>  Documentation/filesystems/nfs/exporting.rst |  7 +--
>  Documentation/filesystems/porting.rst       |  9 ++++
>  fs/affs/namei.c                             |  1 +
>  fs/befs/linuxvfs.c                          |  1 +
>  fs/efs/super.c                              |  1 +
>  fs/erofs/super.c                            |  1 +
>  fs/exportfs/expfs.c                         | 54 +++++++++++++++------
>  fs/ext2/super.c                             |  1 +
>  fs/ext4/super.c                             |  1 +
>  fs/f2fs/super.c                             |  1 +
>  fs/fat/nfs.c                                |  1 +
>  fs/fhandle.c                                |  6 +--
>  fs/fuse/inode.c                             |  7 +--
>  fs/jffs2/super.c                            |  1 +
>  fs/jfs/super.c                              |  1 +
>  fs/nfsd/export.c                            |  3 +-
>  fs/notify/fanotify/fanotify_user.c          |  4 +-
>  fs/ntfs/namei.c                             |  1 +
>  fs/ntfs3/super.c                            |  1 +
>  fs/overlayfs/util.c                         |  2 +-
>  fs/smb/client/export.c                      | 11 ++---
>  fs/squashfs/export.c                        |  1 +
>  fs/ufs/super.c                              |  1 +
>  include/linux/exportfs.h                    | 51 ++++++++++++++++++-
>  24 files changed, 128 insertions(+), 40 deletions(-)
>
> --
> 2.34.1
>

Re: [PATCH v2 0/4] Support more filesystems with FAN_REPORT_FID

[Christian Brauner]

On Mon, 23 Oct 2023 21:07:57 +0300, Amir Goldstein wrote:
> Christian,
> 
> The grand plan is to be able to use fanotify with FAN_REPORT_FID as a
> drop-in replacement for inotify, but with current upstream, inotify is
> supported on all the filesystems and FAN_REPORT_FID only on a few.
> 
> Making all filesystem support FAN_REPORT_FID requires that all
> filesystems will:
> 1. Support for AT_HANDLE_FID file handles
> 2. Report non-zero f_fsid
> 
> [...]

"Late is the hour in which this patchset chooses to appear."
Let's give it some -next exposure.

---

Applied to the vfs.f_fsid branch of the vfs/vfs.git tree.
Patches in the vfs.f_fsid branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.f_fsid

[1/4] exportfs: add helpers to check if filesystem can encode/decode file handles
      https://git.kernel.org/vfs/vfs/c/66c62769bcf6
[2/4] exportfs: make ->encode_fh() a mandatory method for NFS export
      https://git.kernel.org/vfs/vfs/c/dfaf653dc415
[3/4] exportfs: define FILEID_INO64_GEN* file handle types
      https://git.kernel.org/vfs/vfs/c/2560fa66d2ac
[4/4] exportfs: support encoding non-decodeable file handles by default
      https://git.kernel.org/vfs/vfs/c/950f27681add