linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Amir Goldstein <amir73il@gmail.com>
To: Jan Kara <jack@suse.cz>
Cc: Matthew Bobrowski <mbobrowski@mbobrowski.org>,
	Steve Grubb <sgrubb@redhat.com>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>
Subject: Re: [PATCH v2 1/1] fanotify: introduce new event flag FAN_EXEC
Date: Tue, 2 Oct 2018 13:37:13 +0300	[thread overview]
Message-ID: <CAOQ4uxh2LAJ4CtOXNYm16CjvVCAuBxBDkGHCVfNf9MHOfSYunw@mail.gmail.com> (raw)
In-Reply-To: <20181002092448.GA4135@quack2.suse.cz>

On Tue, Oct 2, 2018 at 12:24 PM Jan Kara <jack@suse.cz> wrote:
>
> On Mon 01-10-18 17:01:23, Amir Goldstein wrote:
[...]
> > IMO we need to report FAN_OPEN for every open like we do now.
> > and additionally report FAN_OPEN_EXEC for open for exec.
>
> Fully agreed.
>
> > Then user can implement FAN_OPEN_NOEXEC by setting ignore mask
> > with FAN_OPEN_EXEC.
>
> Good point.
>
> > You can make the analogy to the compound event FAN_CLOSE.
> > If user sets a mask to the compound event FAN_CLOSE and sets ignore
> > mask with FAN_CLOSE_WRITE, then user effectively implemented
> > FAN_CLOSE_NOWRITE with similar semantics to implementation of
> > FAN_OPEN_NOEXEC.
> >
> > Similarly, if user requests FAN_OPEN|FAN_CLOSE and then checks
> > (event->mask & FAN_OPEN) or (event->mask & FAN_CLOSE) it  has
> > similar meaning. Testing (event->mask & FAN_OPEN_EXEC) and
> > (event->mask & FAN_CLOSE_WRITE) in this case is similarly informative.
> >
> > Honestly, I can't think of an application interested only in
> > FAN_CLOSE_NOWRITE nor only in FAN_OPEN_NOEXEC, but the
> > functionality is available for both. The fact that user *can* implement the
> > former without ignore mask and cannot implement to latter without
> > ignore mask is IMO the neccesary evil we need to carry for historic API
> > decisions.
>
> So it seems we are in full agreement that we don't want "optional event
> flags" as Matthew implemented it currently and rather want new event type
> FAN_OPEN_EXEC?
>

First, let me try to better define the semantic difference between
"optional event flag" vs. "new event type" (which may be clear to you).
The former may be reported to user even if user did not set the flag
in any mark mask.
The latter may be reported to user only if user set the event type in
a mark mask.

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.
Second, fsnotify_change() reports a combination of event types on a single
event mask (i.e. FS_MODIFY|FS_ATTRIB), which then may cause users
to see an event type they didn't ask for reported.
We could change fsnotify_change() to report 3 separate events for
FS_MODIFY,FS_ATTRIB,FS_ACCESS, but really, I don't see the point.

How badly can a program be written that it opts into EXEC/ONDIR events
in fanotify_init() and doesn't request them in fanotify_mark() and it flips
when those "optional" flags are reported?
Assuming we also properly document that behavior.
BTW, as far as I understand the current man page, I did not find any explicit
statement that says that you CANNOT get an event if you did not ask for it.
FWIW, inotify and fanotify man pages are quite similar, so it may infer that
fanotify inherits the same expectations as one had from inotify.

Having said all that, I'd like to clarify that I do not object to "new
event type",
I understand why you find it "cleaner".
I just find it less "efficient", because it adds extra calls to
fsnotify() for what
IMO is not a good enough reason.

Thoughts?
Amir.

  reply	other threads:[~2018-10-02 17:20 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-27 13:05 [PATCH v2 1/1] fanotify: introduce new event flag FAN_EXEC Matthew Bobrowski
2018-09-27 13:57 ` Amir Goldstein
2018-09-28  1:27   ` Matthew Bobrowski
2018-09-28  5:39     ` Amir Goldstein
2018-10-01  8:21       ` Matthew Bobrowski
2018-10-01  9:13         ` Amir Goldstein
2018-10-01 10:58 ` Jan Kara
2018-10-01 14:01   ` Amir Goldstein
2018-10-02  9:24     ` Jan Kara
2018-10-02 10:37       ` Amir Goldstein [this message]
2018-10-03 15:40         ` Jan Kara
2018-10-03 16:18           ` Amir Goldstein
2018-10-03 16:33             ` Jan Kara
2018-10-03 20:45               ` Matthew Bobrowski
2018-10-07 11:13               ` Matthew Bobrowski
2018-10-07 13:40                 ` Amir Goldstein
2018-10-08  9:35                 ` Jan Kara
2018-10-02 11:50       ` Matthew Bobrowski
2018-10-03 15:45         ` Jan Kara
2018-10-01 11:06 ` Jan Kara

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAOQ4uxh2LAJ4CtOXNYm16CjvVCAuBxBDkGHCVfNf9MHOfSYunw@mail.gmail.com \
    --to=amir73il@gmail.com \
    --cc=jack@suse.cz \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=mbobrowski@mbobrowski.org \
    --cc=sgrubb@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).