Re: [PATCH] fsnotify: Rearrange fast path to minimise overhead when there is no watcher

[Amir Goldstein <amir73il@gmail.com>]

> > What this work does essentially is two things:
> > 1. Call backend once instead of twice when both inode and parent are
> >     watching.
> > 2. Snapshot name and parent inode to pass to backend not only when
> >     parent is watching, but also when an sb/mnt mark exists which
> >     requests to get file names with events.
> >
> > Compared to the existing implementation of fsnotify_parent(),
> > my code needs to also test bits in inode->i_fsnotify_mask,
> > inode->i_sb->s_fsnotify_mask and mnt->mnt_fsnotify_mask
> > before the fast path can be taken.
> > So its back to square one w.r.t your optimizations.
> >
>
> Seems fair but it may be worth noting that the changes appear to be
> optimising the case where there are watchers. The case where there are
> no watchers at all is also interesting and probably a lot more common. I

My changes are not optimizations. They are for adding functionality.
Surely, that shouldn't come at a cost for the common case.

> didn't look too closely at your series as I'm not familiar with fsnotify
> in general. However, at a glance it looks like fsnotify_parent() executes
> a substantial amount of code even if there are no watchers but I could
> be wrong.
>

I don't about substantial, I would say it is on par with the amount of
code that you tries to optimize out of fsnotify().

Before bailing out with DCACHE_FSNOTIFY_PARENT_WATCHED
test, it also references d_inode->i_sb,  real_mount(path->mnt)
and fetches all their ->x_fsnotify_mask fields.

I changed the call pattern from open/modify/... hooks from:
fsnotify_parent(...);
fsnotify(...);

to:
fsnotify_parent(...); /* which calls fsnotify() */

So the NULL marks optimization could be done in beginning of
fsnotify_parent() and it will be just as effective as it is in fsnotify().

Thanks,
Amir.