Re: [PATCH v2 1/1] fanotify: introduce new event flag FAN_EXEC

[Jan Kara <jack@suse.cz>]

On Wed 03-10-18 19:18:27, Amir Goldstein wrote:
> On Wed, Oct 3, 2018 at 6:40 PM Jan Kara <jack@suse.cz> wrote:
> >
> > On Tue 02-10-18 13:37:13, Amir Goldstein wrote:
> [...]
> > > I am in fact in leaning to the former (as Mathew implemented it), because
> > > I am looking at inotify and my effort to add the "dentry" events to fanotify.
> > > First, my proposal suggests to report the optional event flag FAN_ONDIR,
> > > just like inotify does.
> >
> > Well, we already do deliver FAN_ONDIR event flag if the event was on
> > directory AFAIK. Just with fanotify you also have to explicitely ask for
> > events on directories to be delivered by setting FAN_ONDIR in the mark's
> > mask.
> >
> 
> We actually mask it in out fanotify, so in inotify, it is out-only and
> in fanotify, it is in-only.

OK, didn't notice that. Thanks for educating me.

> BTW, I could not help cleaning up that horrible FAN_MARK_ONDIR
> and it won us a very nice optimization of directory access events.
> patches to follow soon.

Cool! Less work for me as I also had tingling in my fingers to clean up
that mess, just didn't get to it yet :).

> > If that's your concern, what if we just masked out all
> > "unwanted" events in fanotify_handle_event()? fanotify_should_send_event()
> > does all the masking anyway so it's not like we'd loose any performance
> > with that and with current set of fanotify events it would be completely
> > transparent.
> >
> 
> I though about this first, but got myself confused thinking it would be messy.
> Now I am looking again and don't understand why.
> 
> I will try to sum up the solution for us and for Mathew:
> - No FAN_ENABLE_EXEC (sorry for that detour)
> - Implementation in fsnotify_open() is exactly as Mathew did it, but
> changing the
>   name of the flag to FS_OPEN_EXEC
> - Add FAN_OPEN_EXEC to valid user events mask and valid outgoing events
> - fanotify_should_send_event() returns the mask  to be reported in the event
> -- s/return false/return 0/
> -- return event_mask & FAN_ALL_OUTGOING_EVENTS & marks_mask &
>                                  ~marks_ignored_mask;
> 
> So we won't report events that user did not set a mask for and we won't report
> events that user has set an ignore mask for.

Exactly. Just I'd do the change to fanotify_should_send_event() as a
separate patch and rename that function to something like
fanotify_group_event_mask() or something like that to better express what
it will do.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR