Re: [RFC][PATCH 0/4] fsnotify: pass single mark to handle_event()

[Jan Kara <jack@suse.cz>]

On Wed 04-01-17 11:57:04, Amir Goldstein wrote:
> On Wed, Jan 4, 2017 at 10:28 AM, Jan Kara <jack@suse.cz> wrote:
> > However one thing that may be worth cleaning up is that
> > fanotify_should_send_event() needlessly checks the masks - send to group
> > already did this. So I'd move the check for FS_EVENT_ON_CHILD from
> > fanotify_should_send_event() to send_to_group() - arguably it belongs there
> > - and then just completely drop checking of the masks from
> > fanotify_should_send_event(). What do you think?
> >
> 
> Right, so patch [1/4] plus deduplicating the tests in
> fanotify_should_send_event().
> In principle it makes sense. However, you probably noticed that the logic used
> by fanotify_should_send_event() for FS_EVENT_ON_CHILD is different from
> the generic logic in send_to_group().

Yeah, and my head spins when I try to think how the checks in those two
places actually interact - one more reason to check masks only in one place
:).

> The test in fanotify_should_send_event() is skipped if both inode and vfsmount
> marks are present. My patch [1/4] changes this logic, because I thought it was
> a bug, but my tests indicate that a bug related to FS_EVENT_ON_CHILD exists
> before AND after my change.
> 
> So first, I need to isolate and analyse the bug. When I propose a fix, I will
> make sure the FS_EVENT_ON_CHILD test ends up only in send_to_group().

Thanks!

> >> In general, I would like to start working on an fsnotify testsuite,
> >> so if you have any plans wrt writing extra tests or ideas about specific
> >> missing tests, please let me know about them.
> >
> > That would be certainly worthwhile. Actually when I find some useful
> > testcase I add it to LTP under the
> > testcases/kernel/syscalls/{fanotify|inotify}. So please extend that if you
> > have some more ideas for testcases.
> >
> 
> Yes, I am aware of those testcases.
> I find LTP quite heavy to build, so I though I would spin a dedicated
> testsuite that will contain your testcases, but will also include
> infrastructure for stress testing and profiling.
> Keeping track of performance regressions is clearly a major aspect
> of maintaining fsnotify.

Yeah, I don't build full LTP, just testcases in those two directories. The
advantage of LTP is that quite a few people run it so you get a decent test
coverage on different systems (and I've got reports from people running LTP
about regressions in fsnotify code). So I'd prefer to have the functional
tests in LTP if reasonably feasible. But with respect to performance
testing or some crazy stress tests which take long to execute I can definitely
see space for a dedicated test suite.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR