Re: [PATCH v5 10/18] fs: add a flag to indicate the fs supports pre-content events

Amir Goldstein
2024-09-05T08:07:59Z

On Wed, Sep 4, 2024 at 10:29 PM Josef Bacik <josef@toxicpanda.com> wrote:
>
> The pre-content events require some extra thinking, especially around
> page faults. In order to make sure we don't advertise a feature working
> that doesn't actually work, add a flag to allow file systems to opt-in
> to this behavior.
>
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Better squash this patch to FAN_PRE_ACCESS patch, so it will
not be allowed to mark with FAN_PRE_ACCESS on unsupported fs
mid series.

OR - move this patch before the FAN_PRE_ACCESS patch and pre-define
FANOTIFY_PRE_CONTENT_EVENTS to 0.

Apart from that, you may add:

Reviewed-by: Amir Goldstein <amir73il@gmail.com>

Thanks,
Amir.

> ---
> fs/notify/fanotify/fanotify_user.c | 2 ++
> include/linux/fs.h | 1 +
> include/linux/fsnotify.h | 4 ++++
> 3 files changed, 7 insertions(+)
>
> diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> index 53eee8af34a0..936e9f9e0cbc 100644
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -1736,6 +1736,8 @@ static int fanotify_events_supported(struct fsnotify_group *group,
>
> /* Pre-content events are only supported on regular files and dirs */
> if (mask & FANOTIFY_PRE_CONTENT_EVENTS) {
> + if (!(path->mnt->mnt_sb->s_type->fs_flags & FS_ALLOW_HSM))
> + return -EINVAL;
> if (!is_dir && !d_is_reg(path->dentry))
> return -EINVAL;
> if (is_dir && mask & FAN_PRE_MODIFY)
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index fd34b5755c0b..5708e91d3625 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2494,6 +2494,7 @@ struct file_system_type {
> #define FS_USERNS_MOUNT 8 /* Can be mounted by userns root */
> #define FS_DISALLOW_NOTIFY_PERM 16 /* Disable fanotify permission events */
> #define FS_ALLOW_IDMAP 32 /* FS has been updated to handle vfs idmappings. */
> +#define FS_ALLOW_HSM 64 /* FS can handle fanotify pre-content events. */
> #define FS_RENAME_DOES_D_MOVE 32768 /* FS will handle d_move() during rename() internally. */
> int (*init_fs_context)(struct fs_context *);
> const struct fs_parameter_spec *parameters;
> diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h
> index 9d001d328619..27992b548f0c 100644
> --- a/include/linux/fsnotify.h
> +++ b/include/linux/fsnotify.h
> @@ -179,6 +179,10 @@ static inline int fsnotify_file_area_perm(struct file *file, int perm_mask,
> if (!S_ISDIR(inode->i_mode) && !S_ISREG(inode->i_mode))
> return 0;
>
> + /* The fs doesn't support pre-content events. */
> + if (!(inode->i_sb->s_type->fs_flags & FS_ALLOW_HSM))
> + return 0;
> +
> if (perm_mask & MAY_WRITE)
> fsnotify_mask = FS_PRE_MODIFY;
> else if (perm_mask & (MAY_READ | MAY_ACCESS))
> --
> 2.43.0
>