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 :-)
