On Thu, Jun 23, 2022 at 12:49 PM Jan Kara [off-list ref] wrote:

On Wed 22-06-22 21:28:23, Amir Goldstein wrote:

quoted

On Wed, Jun 22, 2022 at 7:00 PM Jan Kara [off-list ref] wrote:

quoted

On Mon 20-06-22 16:45:50, Amir Goldstein wrote:

quoted

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@gmail.com>

..

quoted

@@ -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...

I see... I think the FS_EVENT_ON_CHILD needs to be added to send_to_group()
just like it knows about FS_ISDIR. Will need to look into it and see which tests
we are missing.

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.

The idea behind FSNOTIFY_ITER_TYPE_PARENT is that the event handlers
have all the information to make the right decision based on mask/ignored_mask
and flags. I guess the implementation is incorrect though.

Well spotted!

Thanks,
Amir.