2023-10-23 18:08:34

by Amir Goldstein

[permalink] [raw]
Subject: [PATCH v2 0/4] Support more filesystems with FAN_REPORT_FID

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/[email protected]/
[2] https://lore.kernel.org/r/[email protected]/

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


2023-10-23 18:08:34

by Amir Goldstein

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

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 <[email protected]>
Reviewed-by: Jeff Layton <[email protected]>
Signed-off-by: Amir Goldstein <[email protected]>
---
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

2023-10-24 11:17:02

by Amir Goldstein

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

On Mon, Oct 23, 2023 at 9:08 PM Amir Goldstein <[email protected]> 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/[email protected]/
> [2] https://lore.kernel.org/r/[email protected]/

[3] https://lore.kernel.org/linux-fsdevel/[email protected]/
[4] https://lore.kernel.org/linux-fsdevel/[email protected]/

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

2023-10-24 16:08:39

by Christian Brauner

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

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

2023-10-27 06:05:23

by Christoph Hellwig

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

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.

2023-10-27 06:43:19

by Amir Goldstein

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

On Fri, Oct 27, 2023 at 9:05 AM Christoph Hellwig <[email protected]> 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.

2023-10-27 07:32:17

by Christoph Hellwig

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

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.

2023-10-27 14:28:01

by Amir Goldstein

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

On Fri, Oct 27, 2023 at 10:32 AM Christoph Hellwig <[email protected]> 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.