Message ID | 20231018100000.2453965-6-amir73il@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | Support more filesystems with FAN_REPORT_FID | expand |
On Wed, 2023-10-18 at 13:00 +0300, Amir Goldstein wrote: > 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. > > Signed-off-by: Amir Goldstein <amir73il@gmail.com> > --- > fs/exportfs/expfs.c | 30 +++++++++++++++++++++++++++--- > include/linux/exportfs.h | 10 +++++++--- > 2 files changed, 34 insertions(+), 6 deletions(-) > > diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c > index 30da4539e257..34e7d835d4ef 100644 > --- a/fs/exportfs/expfs.c > +++ b/fs/exportfs/expfs.c > @@ -383,6 +383,30 @@ int generic_encode_ino32_fh(struct inode *inode, __u32 *fh, int *max_len, > } > EXPORT_SYMBOL_GPL(generic_encode_ino32_fh); > > +/** > + * 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 > + * > + * 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 < 3) { > + *max_len = 3; > + return FILEID_INVALID; > + } > + > + fid->i64.ino = inode->i_ino; > + fid->i64.gen = inode->i_generation; Note that i_ino is unsigned long and so is a 32-bit value on 32-bit arches. If the backend storage uses 64-bit inodes, then we usually end up hashing them down to 32-bits first. e.g. see nfs_fattr_to_ino_t(). ceph has some similar code. The upshot is that if you're relying on i_ino, the value can change between different arches, even when they are dealing with the same backend filesystem. Since this is expected to be used by filesystems that don't set up export operations, then that may just be something they have to deal with. I'm not sure what else you can use in lieu of i_ino in this case. > + *max_len = 3; > + > + return FILEID_INO64_GEN; > +} > + > /** > * exportfs_encode_inode_fh - encode a file handle from inode > * @inode: the object to encode > @@ -401,10 +425,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 21eeb9f6bdbd..6688e457da64 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);
On Wed, Oct 18, 2023 at 5:28 PM Jeff Layton <jlayton@kernel.org> wrote: > > On Wed, 2023-10-18 at 13:00 +0300, Amir Goldstein wrote: > > 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. > > > > Signed-off-by: Amir Goldstein <amir73il@gmail.com> > > --- > > fs/exportfs/expfs.c | 30 +++++++++++++++++++++++++++--- > > include/linux/exportfs.h | 10 +++++++--- > > 2 files changed, 34 insertions(+), 6 deletions(-) > > > > diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c > > index 30da4539e257..34e7d835d4ef 100644 > > --- a/fs/exportfs/expfs.c > > +++ b/fs/exportfs/expfs.c > > @@ -383,6 +383,30 @@ int generic_encode_ino32_fh(struct inode *inode, __u32 *fh, int *max_len, > > } > > EXPORT_SYMBOL_GPL(generic_encode_ino32_fh); > > > > +/** > > + * 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 > > + * > > + * 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 < 3) { > > + *max_len = 3; > > + return FILEID_INVALID; > > + } > > + > > + fid->i64.ino = inode->i_ino; > > + fid->i64.gen = inode->i_generation; > > Note that i_ino is unsigned long and so is a 32-bit value on 32-bit > arches. If the backend storage uses 64-bit inodes, then we usually end > up hashing them down to 32-bits first. e.g. see nfs_fattr_to_ino_t(). > ceph has some similar code. > > The upshot is that if you're relying on i_ino, the value can change > between different arches, even when they are dealing with the same > backend filesystem. > > Since this is expected to be used by filesystems that don't set up > export operations, then that may just be something they have to deal > with. I'm not sure what else you can use in lieu of i_ino in this case. > True. That is one more justification for patch [1/5]. If we only support watching inodes when the reported fid is {i_ino, i_generation}, then the likelihood of collision drops considerably compared to watching sb/mount. Because watcher cannot decode fid, watcher must use name_to_handle_at() when setting up the inode watch and store it in some index, so it knows how to map from event->fid to inode later. This means that even if there is a fid collision between two inodes, then the watcher can detect the collision when setting up the inode watch. Thanks, Amir.
On Wed, Oct 18, 2023 at 01:00:00PM +0300, Amir Goldstein wrote: > 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. > > Signed-off-by: Amir Goldstein <amir73il@gmail.com> > --- > fs/exportfs/expfs.c | 30 +++++++++++++++++++++++++++--- > include/linux/exportfs.h | 10 +++++++--- > 2 files changed, 34 insertions(+), 6 deletions(-) > > diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c > index 30da4539e257..34e7d835d4ef 100644 > --- a/fs/exportfs/expfs.c > +++ b/fs/exportfs/expfs.c > @@ -383,6 +383,30 @@ int generic_encode_ino32_fh(struct inode *inode, __u32 *fh, int *max_len, > } > EXPORT_SYMBOL_GPL(generic_encode_ino32_fh); > > +/** > + * 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 Length in what units? Is the 3 below in units of bytes or sizeof(__be32) ? I'm guessing it's the latter; if so, it should be mentioned here. (We have XDR_UNIT for this purpose, btw). export_encode_fh() isn't exactly clear about that either, sadly. > + * > + * 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 < 3) { > + *max_len = 3; Let's make this a symbolic constant rather than a naked integer. > + return FILEID_INVALID; > + } > + > + fid->i64.ino = inode->i_ino; > + fid->i64.gen = inode->i_generation; > + *max_len = 3; > + > + return FILEID_INO64_GEN; > +} > + > /** > * exportfs_encode_inode_fh - encode a file handle from inode > * @inode: the object to encode > @@ -401,10 +425,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 21eeb9f6bdbd..6688e457da64 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 > >
On Wed, Oct 18, 2023 at 6:28 PM Chuck Lever <chuck.lever@oracle.com> wrote: > > On Wed, Oct 18, 2023 at 01:00:00PM +0300, Amir Goldstein wrote: > > 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. > > > > Signed-off-by: Amir Goldstein <amir73il@gmail.com> > > --- > > fs/exportfs/expfs.c | 30 +++++++++++++++++++++++++++--- > > include/linux/exportfs.h | 10 +++++++--- > > 2 files changed, 34 insertions(+), 6 deletions(-) > > > > diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c > > index 30da4539e257..34e7d835d4ef 100644 > > --- a/fs/exportfs/expfs.c > > +++ b/fs/exportfs/expfs.c > > @@ -383,6 +383,30 @@ int generic_encode_ino32_fh(struct inode *inode, __u32 *fh, int *max_len, > > } > > EXPORT_SYMBOL_GPL(generic_encode_ino32_fh); > > > > +/** > > + * 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 > > Length in what units? Is the 3 below in units of bytes or > sizeof(__be32) ? I'm guessing it's the latter; if so, it should > be mentioned here. (We have XDR_UNIT for this purpose, btw). > > export_encode_fh() isn't exactly clear about that either, sadly. > > Yeh, it's the same all over the place including in filesystem implementations. > > + * > > + * 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 < 3) { > > + *max_len = 3; > > Let's make this a symbolic constant rather than a naked integer. > Sure, no problem. Thanks for the review. Amir.
On Wed, Oct 18, 2023 at 1:00 PM Amir Goldstein <amir73il@gmail.com> wrote: > > 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. > > Signed-off-by: Amir Goldstein <amir73il@gmail.com> > --- Hi Jan, Did you get a chance to look at this patch? I saw your review comments on the rest of the series, so was waiting for feedback on this last one before posting v2. BTW, I am going to post a complementary patch to add fsid support for the simple filesystems. Thanks, Amir. > fs/exportfs/expfs.c | 30 +++++++++++++++++++++++++++--- > include/linux/exportfs.h | 10 +++++++--- > 2 files changed, 34 insertions(+), 6 deletions(-) > > diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c > index 30da4539e257..34e7d835d4ef 100644 > --- a/fs/exportfs/expfs.c > +++ b/fs/exportfs/expfs.c > @@ -383,6 +383,30 @@ int generic_encode_ino32_fh(struct inode *inode, __u32 *fh, int *max_len, > } > EXPORT_SYMBOL_GPL(generic_encode_ino32_fh); > > +/** > + * 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 > + * > + * 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 < 3) { > + *max_len = 3; > + return FILEID_INVALID; > + } > + > + fid->i64.ino = inode->i_ino; > + fid->i64.gen = inode->i_generation; > + *max_len = 3; > + > + return FILEID_INO64_GEN; > +} > + > /** > * exportfs_encode_inode_fh - encode a file handle from inode > * @inode: the object to encode > @@ -401,10 +425,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 21eeb9f6bdbd..6688e457da64 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 >
On Mon 23-10-23 16:55:40, Amir Goldstein wrote: > On Wed, Oct 18, 2023 at 1:00 PM Amir Goldstein <amir73il@gmail.com> wrote: > > > > 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. > > > > Signed-off-by: Amir Goldstein <amir73il@gmail.com> > > --- > > Hi Jan, > > Did you get a chance to look at this patch? > I saw your review comments on the rest of the series, so was waiting > for feedback on this last one before posting v2. Ah, sorry. I don't have any further comments regarding this patch besides what Chuck already wrote. Honza
On Mon, Oct 23, 2023 at 7:33 PM Jan Kara <jack@suse.cz> wrote: > > On Mon 23-10-23 16:55:40, Amir Goldstein wrote: > > On Wed, Oct 18, 2023 at 1:00 PM Amir Goldstein <amir73il@gmail.com> wrote: > > > > > > 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. > > > > > > Signed-off-by: Amir Goldstein <amir73il@gmail.com> > > > --- > > > > Hi Jan, > > > > Did you get a chance to look at this patch? > > I saw your review comments on the rest of the series, so was waiting > > for feedback on this last one before posting v2. > > Ah, sorry. I don't have any further comments regarding this patch besides > what Chuck already wrote. No worries. I will post v2 with minor fixes and add your RVB to all patches. Thanks, Amir.
diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c index 30da4539e257..34e7d835d4ef 100644 --- a/fs/exportfs/expfs.c +++ b/fs/exportfs/expfs.c @@ -383,6 +383,30 @@ int generic_encode_ino32_fh(struct inode *inode, __u32 *fh, int *max_len, } EXPORT_SYMBOL_GPL(generic_encode_ino32_fh); +/** + * 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 + * + * 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 < 3) { + *max_len = 3; + return FILEID_INVALID; + } + + fid->i64.ino = inode->i_ino; + fid->i64.gen = inode->i_generation; + *max_len = 3; + + return FILEID_INO64_GEN; +} + /** * exportfs_encode_inode_fh - encode a file handle from inode * @inode: the object to encode @@ -401,10 +425,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 21eeb9f6bdbd..6688e457da64 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);
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. Signed-off-by: Amir Goldstein <amir73il@gmail.com> --- fs/exportfs/expfs.c | 30 +++++++++++++++++++++++++++--- include/linux/exportfs.h | 10 +++++++--- 2 files changed, 34 insertions(+), 6 deletions(-)