On Thu, Mar 16, 2017 at 10:34 AM, Amir Goldstein <amir73il@xxxxxxxxx> wrote:
> On Thu, Mar 16, 2017 at 9:45 AM, Jan Kara <jack@xxxxxxx> 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 :-)
You may add
Reviewed-by: Amir Goldstein <amir73il@xxxxxxxxx>
based on commit 6102788 on your testing branch
