From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-fsdevel-owner@vger.kernel.org>
Received: from mail-yb1-f177.google.com ([209.85.219.177]:43621 "EHLO
        mail-yb1-f177.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1729158AbfAIHJc (ORCPT
        <rfc822;linux-fsdevel@vger.kernel.org>);
        Wed, 9 Jan 2019 02:09:32 -0500
Received: by mail-yb1-f177.google.com with SMTP id t16so2602509ybk.10
        for <linux-fsdevel@vger.kernel.org>; Tue, 08 Jan 2019 23:09:31 -0800 (PST)
MIME-Version: 1.0
References: <20190108164611.11440-1-jack@suse.cz> <20190108164611.11440-3-jack@suse.cz>
In-Reply-To: <20190108164611.11440-3-jack@suse.cz>
From: Amir Goldstein <amir73il@gmail.com>
Date: Wed, 9 Jan 2019 09:09:20 +0200
Message-ID: <CAOQ4uxhDPq1t0afdUGfQ1FdX1twOrN37sfZgPQUhqThq4RqM2g@mail.gmail.com>
Subject: Re: [PATCH 2/4] fanotify: Move locking inside get_one_event()
To: Jan Kara <jack@suse.cz>
Cc: linux-fsdevel <linux-fsdevel@vger.kernel.org>,
        Vivek Trivedi <t.vivek@samsung.com>,
        Orion Poplawski <orion@nwra.com>,
        Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-fsdevel-owner@vger.kernel.org
List-ID: <linux-fsdevel.vger.kernel.org>

On Tue, Jan 8, 2019 at 6:46 PM Jan Kara <jack@suse.cz> wrote:
>
> get_one_event() has a single caller and that just locks
> notification_lock around the call. Move locking inside get_one_event()
> as that will make using ->response field for permission event state
> easier.
>
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
>  fs/notify/fanotify/fanotify_user.c | 26 +++++++++++---------------
>  1 file changed, 11 insertions(+), 15 deletions(-)
>
> diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> index 908ebc421d15..2b2c8b8a17bd 100644
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -51,25 +51,24 @@ struct kmem_cache *fanotify_perm_event_cachep __read_mostly;
>   * Get an fsnotify notification event if one exists and is small
>   * enough to fit in "count". Return an error pointer if the count
>   * is not large enough.
> - *
> - * Called with the group->notification_lock held.
>   */
>  static struct fsnotify_event *get_one_event(struct fsnotify_group *group,
>                                             size_t count)
>  {
> -       assert_spin_locked(&group->notification_lock);
> -
> -       pr_debug("%s: group=%p count=%zd\n", __func__, group, count);

I see you are slowly cleaning up pr_debug calls. Any particular reason?
Out of all the spam pr_debug calls in the code, this one looks rather useful.

Otherwise, looks ok.

Thanks,
Amir.