From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de ([195.135.220.15]:49885 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934851AbdADI2r (ORCPT ); Wed, 4 Jan 2017 03:28:47 -0500 Date: Wed, 4 Jan 2017 09:28:44 +0100 From: Jan Kara To: Amir Goldstein Cc: Jan Kara , Eric Paris , linux-fsdevel@vger.kernel.org Subject: Re: [RFC][PATCH 0/4] fsnotify: pass single mark to handle_event() Message-ID: <20170104082844.GH3780@quack2.suse.cz> References: <1482867148-31497-1-git-send-email-amir73il@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1482867148-31497-1-git-send-email-amir73il@gmail.com> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: Hi, On Tue 27-12-16 21:32:24, Amir Goldstein wrote: > I thought this would turn out simpler, so you may be able to use it > for your work, but I'm afraid that's not the case. > > Anyway, since I am leaving for new year's vacation, I am posting > what I have in case you want to use any of it. > > It passed some initial tests I ran, but when I wanted to test the > corner case referred to in patch 1, I found that my test program > hangs open() syscalls with kernel 4.10-rc1 before any of my changes. > > This is the mark setup I was testing [1]: > fanotify_mark(fd, FAN_MARK_ADD, > FAN_OPEN_PERM | FAN_EVENT_ON_CHILD, AT_FDCWD, > path); > fanotify_mark(fd, FAN_MARK_ADD | \ > FAN_MARK_IGNORED_SURV_MODIFY | FAN_MARK_IGNORED_MASK > FAN_OPEN_PERM | FAN_EVENT_ON_CHILD, AT_FDCWD, > FAN_CLOSE_WRITE, AT_FDCWD, > path); > fanotify_mark(fd, FAN_MARK_ADD | FAN_MARK_MOUNT, > FAN_OPEN_PERM | FAN_CLOSE_WRITE, AT_FDCWD, > path); > > Without FAN_EVENT_ON_CHILD it works fine, but with FAN_EVENT_ON_CHILD, > something bad is going on and I did not have time to look into it. I had a look at the patches and the result does not look simpler than what we had before AFAICT. Sure we don't have to pass both marks into ->handle_event but is that really such a big win? And actually my patches for dropping SRCU lock when waiting for userspace response to fanotify permission event need both marks in ->handle_event because they both need to be protected against freeing when SRCU lock is dropped... So I don't think this is really viable path. 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? > 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. Honza -- Jan Kara SUSE Labs, CR