On Wed 22-06-22 21:28:23, Amir Goldstein wrote:
> On Wed, Jun 22, 2022 at 7:00 PM Jan Kara <jack@xxxxxxx> wrote:
> >
> > On Mon 20-06-22 16:45:50, Amir Goldstein wrote:
> > > Setting flags FAN_ONDIR FAN_EVENT_ON_CHILD in ignore mask has no effect.
> > > The FAN_EVENT_ON_CHILD flag in mask implicitly applies to ignore mask and
> > > ignore mask is always implicitly applied to events on directories.
> > >
> > > Define a mark flag that replaces this legacy behavior with logic of
> > > applying the ignore mask according to event flags in ignore mask.
> > >
> > > Implement the new logic to prepare for supporting an ignore mask that
> > > ignores events on children and ignore mask that does not ignore events
> > > on directories.
> > >
> > > To emphasize the change in terminology, also rename ignored_mask mark
> > > member to ignore_mask and use accessor to get only ignored events or
> > > events and flags.
> > >
> > > This change in terminology finally aligns with the "ignore mask"
> > > language in man pages and in most of the comments.
> > >
> > > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> >
> > ..
> >
> > > @@ -423,7 +425,8 @@ static bool fsnotify_iter_select_report_types(
> > > * But is *this mark* watching children?
> > > */
> > > if (type == FSNOTIFY_ITER_TYPE_PARENT &&
> > > - !(mark->mask & FS_EVENT_ON_CHILD))
> > > + !(mark->mask & FS_EVENT_ON_CHILD) &&
> > > + !(fsnotify_ignore_mask(mark) & FS_EVENT_ON_CHILD))
> > > continue;
> >
> > So now we have in ->report_mask the FSNOTIFY_ITER_TYPE_PARENT if either
> > ->mask or ->ignore_mask have FS_EVENT_ON_CHILD set. But I see nothing that
> > would stop us from applying say ->mask to the set of events we are
> > interested in if FS_EVENT_ON_CHILD is set only in ->ignore_mask? And
>
> I think I spent some time thinking about this and came to a conclusion that
> 1. It is hard to get all the cases right
> 2. It is a micro optimization
>
> The implication is that the user can set an ignore mask of an object, get no
> events but still cause performance penalty. Right?
> So just don't do that...
So I was more afraid that this actually results in generating events we
should not generate. For example consider dir 'd' with mask FS_OPEN and
ignore_mask FS_MODIFY | FS_EVENT_ON_CHILD. Now open("d/file") happens so
FS_OPEN is generated for d/file. We select FSNOTIFY_ITER_TYPE_PARENT in the
->report_mask because of the ignore_mask on 'd' and pass the iter to
fanotify_handle_event(). There fanotify_group_event_mask() will include
FS_OPEN to marks_mask and conclude event should be reported. But there's no
mark that should result in reporting this...
The problem is that with the introduction of FSNOTIFY_ITER_TYPE_PARENT we
started to rely on that type being set only when the event on child should
be reported to parent and now you break that AFAICT.
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
