On Tue, Nov 27, 2018 at 12:24 PM Amir Goldstein amir73il@gmail.com wrote:
On Tue, Nov 27, 2018 at 11:09 AM Miklos Szeredi miklos@szeredi.hu wrote:
Hi Amir,
Here's a backport of this patch to 4.18 and earlier. Tested good with ltp/fanotify09.
AFAICS this backport is identical to my v4.19 backport and yes, it looks fine. I just missed the fact that my v4.19 does apply cleanly on v4.18 or I would have asked Greg to apply it.
Where's your v4.19 backport? I don't see it only any list.
OTOH, your backport is good for v4.18 not earlier, nothing before: 837a39343847 fanotify: generalize fanotify_should_send_event()
Okay.
I didn't quite understand the relevance of masking against ALL_FSNOTIFY_EVENTS in __fsnotify_parent()
Right... that is because stable is missing: 007d1e8395ea fsnotify: generalize handling of extra event flags
The mask in __fsnotify_parent() (on master) the same as (stable) mask in fsnotify(), i.e. test_mask = (mask & ~FS_EVENT_ON_CHILD); so its a very minor but low hanging optimization once ALL_FSNOTIFY_EVENTS is defined. So in the backport the mask in __fsnotify_parent() does nothing but its not critical either.
Okay, I'll remove it to avoid confusion. General comment about not mixing optimization with fix applies here...
and the backport in fsnotify() is also not quite trivial.
It means: if __fsnotify_parent() is reporting this event, then the event is not intended for a mount mark. __fsnotify_parent() is always called paired with fsnotify() (not for parent) and the mount mark will be getting the event in that second call.
So this is a double layered fix for the bug:
The check in fsnotify() optimizes away events reported from __fsnotify_parent() if the parent inode does NOT have event in commutative mask of ANY group, but mount mark DOES have event in mask of SOME groups.
The check in fanotify_should_send_event() does the same for a SPECIFIC group.
Got it. So the *real* fix is in fanotify_should_send_event(), the rest are optimizations.
So, can you please review?
Looks good,
Thanks for looking.
Miklos