On Tue, Nov 27, 2018 at 12:24 PM Amir Goldstein <amir73il@xxxxxxxxx> wrote: > > On Tue, Nov 27, 2018 at 11:09 AM Miklos Szeredi <miklos@xxxxxxxxxx> 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
