From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de ([195.135.220.15]:34458 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726811AbeJCXWw (ORCPT ); Wed, 3 Oct 2018 19:22:52 -0400 Date: Wed, 3 Oct 2018 18:33:43 +0200 From: Jan Kara To: Amir Goldstein Cc: Jan Kara , Matthew Bobrowski , Steve Grubb , linux-fsdevel Subject: Re: [PATCH v2 1/1] fanotify: introduce new event flag FAN_EXEC Message-ID: <20181003163343.GJ24030@quack2.suse.cz> References: <20181001105803.GG3913@quack2.suse.cz> <20181002092448.GA4135@quack2.suse.cz> <20181003154005.GD24030@quack2.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Wed 03-10-18 19:18:27, Amir Goldstein wrote: > On Wed, Oct 3, 2018 at 6:40 PM Jan Kara wrote: > > > > On Tue 02-10-18 13:37:13, Amir Goldstein wrote: > [...] > > > I am in fact in leaning to the former (as Mathew implemented it), because > > > I am looking at inotify and my effort to add the "dentry" events to fanotify. > > > First, my proposal suggests to report the optional event flag FAN_ONDIR, > > > just like inotify does. > > > > Well, we already do deliver FAN_ONDIR event flag if the event was on > > directory AFAIK. Just with fanotify you also have to explicitely ask for > > events on directories to be delivered by setting FAN_ONDIR in the mark's > > mask. > > > > We actually mask it in out fanotify, so in inotify, it is out-only and > in fanotify, it is in-only. OK, didn't notice that. Thanks for educating me. > BTW, I could not help cleaning up that horrible FAN_MARK_ONDIR > and it won us a very nice optimization of directory access events. > patches to follow soon. Cool! Less work for me as I also had tingling in my fingers to clean up that mess, just didn't get to it yet :). > > If that's your concern, what if we just masked out all > > "unwanted" events in fanotify_handle_event()? fanotify_should_send_event() > > does all the masking anyway so it's not like we'd loose any performance > > with that and with current set of fanotify events it would be completely > > transparent. > > > > I though about this first, but got myself confused thinking it would be messy. > Now I am looking again and don't understand why. > > I will try to sum up the solution for us and for Mathew: > - No FAN_ENABLE_EXEC (sorry for that detour) > - Implementation in fsnotify_open() is exactly as Mathew did it, but > changing the > name of the flag to FS_OPEN_EXEC > - Add FAN_OPEN_EXEC to valid user events mask and valid outgoing events > - fanotify_should_send_event() returns the mask to be reported in the event > -- s/return false/return 0/ > -- return event_mask & FAN_ALL_OUTGOING_EVENTS & marks_mask & > ~marks_ignored_mask; > > So we won't report events that user did not set a mask for and we won't report > events that user has set an ignore mask for. Exactly. Just I'd do the change to fanotify_should_send_event() as a separate patch and rename that function to something like fanotify_group_event_mask() or something like that to better express what it will do. Honza -- Jan Kara SUSE Labs, CR