From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-fsdevel-owner@vger.kernel.org>
Received: from mail-ot0-f182.google.com ([74.125.82.182]:36226 "EHLO
        mail-ot0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1751104AbdCPIez (ORCPT
        <rfc822;linux-fsdevel@vger.kernel.org>);
        Thu, 16 Mar 2017 04:34:55 -0400
Received: by mail-ot0-f182.google.com with SMTP id i1so47071729ota.3
        for <linux-fsdevel@vger.kernel.org>; Thu, 16 Mar 2017 01:34:54 -0700 (PDT)
MIME-Version: 1.0
In-Reply-To: <20170316074540.GK12989@quack2.suse.cz>
References: <20170315104632.7037-1-jack@suse.cz> <20170315104632.7037-7-jack@suse.cz>
 <CAOQ4uxiOa96r3p9yJXbChCLqWhLdPHT6HJKWq1wNznqRq9Dh+A@mail.gmail.com>
 <20170316072643.GJ12989@quack2.suse.cz> <20170316074540.GK12989@quack2.suse.cz>
From: Amir Goldstein <amir73il@gmail.com>
Date: Thu, 16 Mar 2017 10:34:53 +0200
Message-ID: <CAOQ4uxhgi2UwL8mx-_tWH79MUXC-Cci7DSUAWpx_LK+OmgtMtw@mail.gmail.com>
Subject: Re: [PATCH 06/33] fsnotify: Move mark list head from object into
 dedicated structure
To: Jan Kara <jack@suse.cz>
Cc: linux-fsdevel <linux-fsdevel@vger.kernel.org>,
        Miklos Szeredi <miklos@szeredi.hu>,
        Paul Moore <paul@paul-moore.com>
Content-Type: text/plain; charset=UTF-8
Sender: linux-fsdevel-owner@vger.kernel.org
List-ID: <linux-fsdevel.vger.kernel.org>

On Thu, Mar 16, 2017 at 9:45 AM, Jan Kara <jack@suse.cz> wrote:
> On Thu 16-03-17 08:26:43, Jan Kara wrote:
>> On Wed 15-03-17 16:11:17, Amir Goldstein wrote:
>> > > +/*
>> > > + * Add mark into proper place in given list of marks. These marks may be used
>> > > + * for the fsnotify backend to determine which event types should be delivered
>> > > + * to which group and for which inodes. These marks are ordered according to
>> > > + * priority, highest number first, and then by the group's location in memory.
>> > > + */
>> > > +int fsnotify_add_mark_list(struct fsnotify_mark_connector **connp,
>> > > +                          struct fsnotify_mark *mark, int allow_dups)
>> > >  {
>> > >         struct fsnotify_mark *lmark, *last = NULL;
>> > > +       struct fsnotify_mark_connector *conn;
>> > >         int cmp;
>> > >
>> > > +       if (!*connp) {
>> > > +               conn = kmem_cache_alloc(fsnotify_mark_connector_cachep,
>> > > +                                       GFP_ATOMIC);
>> > > +               if (!conn)
>> > > +                       return -ENOMEM;
>> > > +               INIT_HLIST_HEAD(&conn->list);
>> > > +               /*
>> > > +                * Make sure 'conn' initialization is visible. Matches
>> > > +                * lockless_dereference() in fsnotify().
>> > > +                */
>> > > +               smp_wmb();
>> > > +               *connp = conn;
>> > > +       } else {
>> > > +               conn = *connp;
>> > > +       }
>> > > +
>> >
>> > This chunk would look nicer in fsnotify_connector_get() helper.
>>
>> Good point. I've called the function fsnotify_grab_create_connector() to
>> make it consistent with later fsnotify_grab_connector() and to stress clearly
>> that this may allocate new connector unlike later fsnotify_grab_connector()...
>
> OK, after looking more at your other suggestion and patches, I've moved
> only the allocation into a separate function and called it
> fsnotify_attach_connector_to_object() (which matches
> fsnotify_detach_connector_from_object() added later).
>

That sounds clean and properly named :-)