On Mon, Sep 12, 2016 at 10:32 AM, Jan Kara <jack@xxxxxxx> wrote:
> On Sat 10-09-16 02:33:34, Lino Sanfilippo wrote:
>> > @@ -379,12 +383,21 @@ static int fanotify_release(struct inode *ignored, struct file *file)
>> > spin_unlock(&group->fanotify_data.access_lock);
>> >
>> > /*
>> > - * Since bypass_perm is set, newly queued events will not wait for
>> > - * access response. Wake up the already sleeping ones now.
>> > - * synchronize_srcu() in fsnotify_destroy_group() will wait for all
>> > - * processes sleeping in fanotify_handle_event() waiting for access
>> > - * response and thus also for all permission events to be freed.
>> > + * Destroy all non-permission events. For permission events just
>> > + * dequeue them and set the response. They will be freed once the
>> > + * response is consumed and fanotify_get_response() returns.
>> > */
>> > + mutex_lock(&group->notification_mutex);
>> > + while (!fsnotify_notify_queue_is_empty(group)) {
>> > + fsn_event = fsnotify_remove_first_event(group);
>> > + if (!(fsn_event->mask & FAN_ALL_PERM_EVENTS))
>> > + fsnotify_destroy_event(group, fsn_event);
>> > + else
>> > + FANOTIFY_PE(fsn_event)->response = FAN_ALLOW;
>>
>> Why do we have to handle (i.e destroy) non-permission events here? Is this
>> meant as some kind of optimization?
>
> Where else would they be destroyed? Normal events are destroyed when
> userspace reads them but nobody is going to read these anymore. For
> permission events things are more complex - they get destroyed by the
> process creating these events since that process waits for the response.
I think he meant that normal events get destroyed anyway in
fsnotify_destroy_group(). Which is true, but then we'd need a proper
iterator for picking just permission events and it would be a lot more
complicated in the end.
Thanks,
Miklos
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
