2023-10-18 10:00:19

by Amir Goldstein

[permalink] [raw]
Subject: [PATCH 3/5] exportfs: make ->encode_fh() a mandatory method for NFS export

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.

Signed-off-by: Amir Goldstein <[email protected]>
---
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 | 14 ++++++++------
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 | 9 +++------
fs/squashfs/export.c | 1 +
fs/ufs/super.c | 1 +
include/linux/exportfs.h | 4 +++-
19 files changed, 39 insertions(+), 18 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..197ef78a5014 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.
+Fillesystems 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..30da4539e257 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
+ * @fh: where to store the file handle fragment
* @max_len: maximum length to store there
* @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..834e9c9197b4 100644
--- a/fs/smb/client/export.c
+++ b/fs/smb/client/export.c
@@ -41,13 +41,10 @@ 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..6b6e01321405 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,8 @@ extern struct dentry *exportfs_decode_fh(struct vfsmount *mnt, struct fid *fid,
/*
* Generic helpers for filesystems.
*/
+int generic_encode_ino32_fh(struct inode *inode, __u32 *fh, int *max_len,
+ struct inode *parent);
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


2023-10-18 14:18:21

by Jeff Layton

[permalink] [raw]
Subject: Re: [PATCH 3/5] exportfs: make ->encode_fh() a mandatory method for NFS export

On Wed, 2023-10-18 at 12:59 +0300, 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.
>
> Signed-off-by: Amir Goldstein <[email protected]>
> ---
> 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 | 14 ++++++++------
> 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 | 9 +++------
> fs/squashfs/export.c | 1 +
> fs/ufs/super.c | 1 +
> include/linux/exportfs.h | 4 +++-
> 19 files changed, 39 insertions(+), 18 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..197ef78a5014 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.
> +Fillesystems 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..30da4539e257 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
> + * @fh: where to store the file handle fragment
> * @max_len: maximum length to store there
> * @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..834e9c9197b4 100644
> --- a/fs/smb/client/export.c
> +++ b/fs/smb/client/export.c
> @@ -41,13 +41,10 @@ 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..6b6e01321405 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,8 @@ extern struct dentry *exportfs_decode_fh(struct vfsmount *mnt, struct fid *fid,
> /*
> * Generic helpers for filesystems.
> */
> +int generic_encode_ino32_fh(struct inode *inode, __u32 *fh, int *max_len,
> + struct inode *parent);
> 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));

Looks straightforward.

Reviewed-by: Jeff Layton <[email protected]>

2023-10-18 14:54:37

by Dave Kleikamp

[permalink] [raw]
Subject: Re: [PATCH 3/5] exportfs: make ->encode_fh() a mandatory method for NFS export

On 10/18/23 9:16AM, Jeff Layton wrote:
> On Wed, 2023-10-18 at 12:59 +0300, 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.

Isn't it possible for some of these filesystems to be compiled without
CONFIG_EXPORTFS set? Should exportfs.h define an null
generic_encode_ino32_fh() in that case?

Shaggy

>>
>> This is a step towards allowing filesystems to encode non-decodeable file
>> handles for fanotify without having to implement any export_operations.
>>
>> Signed-off-by: Amir Goldstein <[email protected]>
>> ---
>> 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 | 14 ++++++++------
>> 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 | 9 +++------
>> fs/squashfs/export.c | 1 +
>> fs/ufs/super.c | 1 +
>> include/linux/exportfs.h | 4 +++-
>> 19 files changed, 39 insertions(+), 18 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..197ef78a5014 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.
>> +Fillesystems 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..30da4539e257 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
>> + * @fh: where to store the file handle fragment
>> * @max_len: maximum length to store there
>> * @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..834e9c9197b4 100644
>> --- a/fs/smb/client/export.c
>> +++ b/fs/smb/client/export.c
>> @@ -41,13 +41,10 @@ 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..6b6e01321405 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,8 @@ extern struct dentry *exportfs_decode_fh(struct vfsmount *mnt, struct fid *fid,
>> /*
>> * Generic helpers for filesystems.
>> */
>> +int generic_encode_ino32_fh(struct inode *inode, __u32 *fh, int *max_len,
>> + struct inode *parent);
>> 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));
>
> Looks straightforward.
>
> Reviewed-by: Jeff Layton <[email protected]>

2023-10-18 15:20:01

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH 3/5] exportfs: make ->encode_fh() a mandatory method for NFS export

On Wed, Oct 18, 2023 at 12:59:58PM +0300, 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.
>
> Signed-off-by: Amir Goldstein <[email protected]>
> ---
> 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 | 14 ++++++++------
> 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 | 9 +++------
> fs/squashfs/export.c | 1 +
> fs/ufs/super.c | 1 +
> include/linux/exportfs.h | 4 +++-
> 19 files changed, 39 insertions(+), 18 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..197ef78a5014 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.
> +Fillesystems 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..30da4539e257 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
> + * @fh: where to store the file handle fragment
> * @max_len: maximum length to store there
> * @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..834e9c9197b4 100644
> --- a/fs/smb/client/export.c
> +++ b/fs/smb/client/export.c
> @@ -41,13 +41,10 @@ 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..6b6e01321405 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;

The ->encode_fh() method returns an integer type, not a boolean. It
would be more clear if this were written

return nop && (nop->encode_fh != FILEID_ROOT);

(I'm just guessing at what you might have intended).


> }
>
> static inline bool exportfs_can_decode_fh(const struct export_operations *nop)
> @@ -279,6 +279,8 @@ extern struct dentry *exportfs_decode_fh(struct vfsmount *mnt, struct fid *fid,
> /*
> * Generic helpers for filesystems.
> */
> +int generic_encode_ino32_fh(struct inode *inode, __u32 *fh, int *max_len,
> + struct inode *parent);
> 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
>

--
Chuck Lever

2023-10-18 15:24:35

by Amir Goldstein

[permalink] [raw]
Subject: Re: [PATCH 3/5] exportfs: make ->encode_fh() a mandatory method for NFS export

On Wed, Oct 18, 2023 at 5:53 PM Dave Kleikamp <[email protected]> wrote:
>
> On 10/18/23 9:16AM, Jeff Layton wrote:
> > On Wed, 2023-10-18 at 12:59 +0300, 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.
>
> Isn't it possible for some of these filesystems to be compiled without
> CONFIG_EXPORTFS set? Should exportfs.h define an null
> generic_encode_ino32_fh() in that case?
>

Yes, good idea!

Thanks,
Amir.

2023-10-18 15:26:58

by Amir Goldstein

[permalink] [raw]
Subject: Re: [PATCH 3/5] exportfs: make ->encode_fh() a mandatory method for NFS export

On Wed, Oct 18, 2023 at 6:18 PM Chuck Lever <[email protected]> wrote:
>
> On Wed, Oct 18, 2023 at 12:59:58PM +0300, 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.
> >
> > Signed-off-by: Amir Goldstein <[email protected]>
> > ---
[...]

> > diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
> > index 5b3c9f30b422..6b6e01321405 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;
>
> The ->encode_fh() method returns an integer type, not a boolean. It
> would be more clear if this were written
>
> return nop && (nop->encode_fh != FILEID_ROOT);
>
> (I'm just guessing at what you might have intended).
>

You must be pre-coffee ;)

This checks if the method exists, it doesn't invoke the method.

Thanks,
Amir.

2023-10-18 15:37:26

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH 3/5] exportfs: make ->encode_fh() a mandatory method for NFS export

On Wed, Oct 18, 2023 at 06:26:07PM +0300, Amir Goldstein wrote:
> On Wed, Oct 18, 2023 at 6:18 PM Chuck Lever <[email protected]> wrote:
> >
> > On Wed, Oct 18, 2023 at 12:59:58PM +0300, 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.
> > >
> > > Signed-off-by: Amir Goldstein <[email protected]>
> > > ---
> [...]
>
> > > diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
> > > index 5b3c9f30b422..6b6e01321405 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;
> >
> > The ->encode_fh() method returns an integer type, not a boolean. It
> > would be more clear if this were written
> >
> > return nop && (nop->encode_fh != FILEID_ROOT);
> >
> > (I'm just guessing at what you might have intended).
> >
>
> You must be pre-coffee ;)

More like pre-lunch.


> This checks if the method exists, it doesn't invoke the method.

OK, I was confused because I thought you were filling in all the
spots where the method doesn't already exist. My brain lept to
the conclusion that this is therefore calling the method, but I
failed to notice there are no call arguments.

I might understand it now. :-)

Since you want this series to go through the vfs tree:

Acked-by: Chuck Lever <[email protected]>


--
Chuck Lever

2023-10-19 14:41:44

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH 3/5] exportfs: make ->encode_fh() a mandatory method for NFS export

On Wed 18-10-23 12:59:58, 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.
>
> Signed-off-by: Amir Goldstein <[email protected]>

Just one typo cleanup. Also I agree we need a "nop" variant of
generic_encode_ino32_fh() or move this to fs/libfs.c like e.g.
generic_fh_to_dentry().

> diff --git a/Documentation/filesystems/porting.rst b/Documentation/filesystems/porting.rst
> index 4d05b9862451..197ef78a5014 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.
> +Fillesystems that used the default implementation may use the generic helper
^^^ Filesystems

> +generic_encode_ino32_fh() explicitly.

Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR

2023-10-19 15:23:02

by Amir Goldstein

[permalink] [raw]
Subject: Re: [PATCH 3/5] exportfs: make ->encode_fh() a mandatory method for NFS export

On Thu, Oct 19, 2023 at 5:40 PM Jan Kara <[email protected]> wrote:
>
> On Wed 18-10-23 12:59:58, 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.
> >
> > Signed-off-by: Amir Goldstein <[email protected]>
>
> Just one typo cleanup. Also I agree we need a "nop" variant of
> generic_encode_ino32_fh() or move this to fs/libfs.c like e.g.
> generic_fh_to_dentry().
>

I did this:

/*
* 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

I like it better than moving to fs/libfs.c, because if CONFIG_EXPORTFS
is not defined, no code should be calling generic_encode_ino32_fh().

It might be a good idea to define exportfs_can_*() helpers to false
when CONFIG_EXPORTFS is not defined, but at least for fanotify,
this is not relevant because fanotify selects EXPORTFS.

> > diff --git a/Documentation/filesystems/porting.rst b/Documentation/filesystems/porting.rst
> > index 4d05b9862451..197ef78a5014 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.
> > +Fillesystems that used the default implementation may use the generic helper
> ^^^ Filesystems
>

Thanks!
Amir.