Thread (13 messages) 13 messages, 2 authors, 2022-06-24

Re: [PATCH 1/2] fanotify: prepare for setting event flags in ignore mask

From: Jan Kara <jack@suse.cz>
Date: 2022-06-23 09:49:48
Also in: linux-fsdevel

On Wed 22-06-22 21:28:23, Amir Goldstein wrote:
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...

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 [off-list ref]
SUSE Labs, CR
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help