Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210426184201.4177978-1-krisman@collabora.com> <20210426184201.4177978-4-krisman@collabora.com>
In-Reply-To: <20210426184201.4177978-4-krisman@collabora.com>
From:   Amir Goldstein <amir73il@gmail.com>
Date:   Tue, 27 Apr 2021 08:03:56 +0300
Message-ID: <CAOQ4uxjUp_GecrV5VdBArN07PWQBUguQvwyL6K7NuXK=8yFMAw@mail.gmail.com>
Subject: Re: [PATCH RFC 03/15] fsnotify: Wire flags field on group allocation
To:     Gabriel Krisman Bertazi <krisman@collabora.com>
Cc:     Theodore Tso <tytso@mit.edu>,
        "Darrick J. Wong" <djwong@kernel.org>,
        Dave Chinner <david@fromorbit.com>, Jan Kara <jack@suse.com>,
        David Howells <dhowells@redhat.com>,
        Khazhismel Kumykov <khazhy@google.com>,
        linux-fsdevel <linux-fsdevel@vger.kernel.org>,
        Ext4 <linux-ext4@vger.kernel.org>, kernel@collabora.com
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, Apr 26, 2021 at 9:42 PM Gabriel Krisman Bertazi
<krisman@collabora.com> wrote:
>
> Introduce a flags field in fsnotify_group to track the mode of
> submission this group has.
>
> Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com>
> ---
>  fs/notify/dnotify/dnotify.c        |  2 +-
>  fs/notify/fanotify/fanotify_user.c |  4 ++--
>  fs/notify/group.c                  | 13 ++++++++-----
>  fs/notify/inotify/inotify_user.c   |  2 +-
>  include/linux/fsnotify_backend.h   |  7 +++++--
>  kernel/audit_fsnotify.c            |  2 +-
>  kernel/audit_tree.c                |  2 +-
>  kernel/audit_watch.c               |  2 +-
>  8 files changed, 20 insertions(+), 14 deletions(-)
>
> diff --git a/fs/notify/dnotify/dnotify.c b/fs/notify/dnotify/dnotify.c
> index e85e13c50d6d..37960c8750e4 100644
> --- a/fs/notify/dnotify/dnotify.c
> +++ b/fs/notify/dnotify/dnotify.c
> @@ -383,7 +383,7 @@ static int __init dnotify_init(void)
>                                           SLAB_PANIC|SLAB_ACCOUNT);
>         dnotify_mark_cache = KMEM_CACHE(dnotify_mark, SLAB_PANIC|SLAB_ACCOUNT);
>
> -       dnotify_group = fsnotify_alloc_group(&dnotify_fsnotify_ops);
> +       dnotify_group = fsnotify_alloc_group(&dnotify_fsnotify_ops, 0);
>         if (IS_ERR(dnotify_group))
>                 panic("unable to allocate fsnotify group for dnotify\n");
>         return 0;
> diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> index e0d113e3b65c..f50c4ab721e3 100644
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -929,7 +929,7 @@ static struct fsnotify_event *fanotify_alloc_overflow_event(void)
>  SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
>  {
>         struct fsnotify_group *group;
> -       int f_flags, fd;
> +       int f_flags, fd, fsn_flags = 0;
>         struct user_struct *user;
>         unsigned int fid_mode = flags & FANOTIFY_FID_BITS;
>         unsigned int class = flags & FANOTIFY_CLASS_BITS;
> @@ -982,7 +982,7 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
>                 f_flags |= O_NONBLOCK;
>
>         /* fsnotify_alloc_group takes a ref.  Dropped in fanotify_release */
> -       group = fsnotify_alloc_user_group(&fanotify_fsnotify_ops);
> +       group = fsnotify_alloc_user_group(&fanotify_fsnotify_ops, fsn_flags);
>         if (IS_ERR(group)) {
>                 free_uid(user);
>                 return PTR_ERR(group);
> diff --git a/fs/notify/group.c b/fs/notify/group.c
> index ffd723ffe46d..08acb1afc0c2 100644
> --- a/fs/notify/group.c
> +++ b/fs/notify/group.c
> @@ -112,7 +112,7 @@ void fsnotify_put_group(struct fsnotify_group *group)
>  EXPORT_SYMBOL_GPL(fsnotify_put_group);
>
>  static struct fsnotify_group *__fsnotify_alloc_group(
> -                               const struct fsnotify_ops *ops, gfp_t gfp)
> +       const struct fsnotify_ops *ops, unsigned int flags, gfp_t gfp)
>  {
>         struct fsnotify_group *group;
>
> @@ -134,6 +134,7 @@ static struct fsnotify_group *__fsnotify_alloc_group(
>         INIT_LIST_HEAD(&group->marks_list);
>
>         group->ops = ops;
> +       group->flags = flags;
>
>         return group;
>  }
> @@ -141,18 +142,20 @@ static struct fsnotify_group *__fsnotify_alloc_group(
>  /*
>   * Create a new fsnotify_group and hold a reference for the group returned.
>   */
> -struct fsnotify_group *fsnotify_alloc_group(const struct fsnotify_ops *ops)
> +struct fsnotify_group *fsnotify_alloc_group(const struct fsnotify_ops *ops,
> +                                           unsigned int flags)
>  {
> -       return __fsnotify_alloc_group(ops, GFP_KERNEL);
> +       return __fsnotify_alloc_group(ops, flags, GFP_KERNEL);
>  }
>  EXPORT_SYMBOL_GPL(fsnotify_alloc_group);
>
>  /*
>   * Create a new fsnotify_group and hold a reference for the group returned.
>   */
> -struct fsnotify_group *fsnotify_alloc_user_group(const struct fsnotify_ops *ops)
> +struct fsnotify_group *fsnotify_alloc_user_group(const struct fsnotify_ops *ops,
> +                                                unsigned int flags)
>  {
> -       return __fsnotify_alloc_group(ops, GFP_KERNEL_ACCOUNT);
> +       return __fsnotify_alloc_group(ops, flags, GFP_KERNEL_ACCOUNT);
>  }
>  EXPORT_SYMBOL_GPL(fsnotify_alloc_user_group);
>

*IF* we go this way, note that fsnotify_alloc_group() doesn't need
flags argument.
None of the callers of fsnotify_alloc_group() ever use the
notification list, so it
would be better to pass flag FSN_NOTIFICATION_LIST from the backends that
do use it (fanotify and inotify) for the sake of symmetry with FSN_RING_BUFFER
and no need to change other callers.

Thanks,
Amir.