Re: [PATCH 4/5] fanotify: add support for exclusive create of mark

[Amir Goldstein <amir73il@gmail.com>]

On Thu, Mar 17, 2022 at 5:45 PM Jan Kara <jack@suse.cz> wrote:
>
> On Thu 17-03-22 16:34:43, Jan Kara wrote:
> > On Mon 07-03-22 17:57:40, Amir Goldstein wrote:
> > > Similar to inotify's IN_MARK_CREATE, adding an fanotify mark with flag
> > > FAN_MARK_CREATE will fail with error EEXIST if an fanotify mark already
> > > exists on the object.
> > >
> > > Unlike inotify's IN_MARK_CREATE, FAN_MARK_CREATE has to supplied in
> > > combination with FAN_MARK_ADD (FAN_MARK_ADD is like inotify_add_watch()
> > > and the behavior of IN_MARK_ADD is the default for fanotify_mark()).
> > >
> > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> >
> > What I'm missing in this changelog is "why". Is it just about feature
> > parity with inotify? I don't find this feature particularly useful...
>
> OK, now I understand after reading patch 5/5. Hum, but I'm not quite happy
> about the limitation to non-existing mark as much as I understand why you
> need it. Let me think...
>

Sorry for not articulating the problem better.
Let me write up the problem and maybe someone can come up with a better
solution than I did.

The problem that I was trying to avoid with FAN_MARK_VOLATILE is similar
to an existing UAPI problem with FAN_MARK_IGNORED_SURV_MODIFY -
This flag can only be set and not cleared and when set it affects all the events
set in the mask prior to that time, leading to unpredictable results.

Let's say a user sets FAN_CLOSE in ignored mask without _SURV_MODIFY
and later sets FAN_OPEN  in ignored mask with _SURV_MODIFY.
Does the ignored mask now include FAN_CLOSE? That depends
whether or not FAN_MODIFY event took place between the two calls.

That is one of the reasons I was trying to get rid of _SURV_MODIFY with
new FAN_MARK_IGNORE flag. The trickery in FAN_MARK_CREATE is
that the problem is averted - if a mark property can only be set and never
cleared and if it affects all past and future changes to mask, allow to set
this property during mark creation time and only during mark creation time.

I don't think there is a real use case for changing the _SURV_MODIFY
nor _VOLATILE property of a mark and indeed with new FAN_MARK_IGNORE
semantics, we may only allow to set _SURV_MODIFY along with
FAN_MARK_CREATE, so there are two problems solved using this method.

The fact that FAN_MARK_CREATE has feature parity with inotify is not
the reason to add it, but it does help to swallow this somewhat awkward
solution. And it is certainly easy to document.

As the commit message implies, I was contemplating whether
FAN_MARK_CREATE should be an alternative to FAN_MARK_ADD
or an ORed flag.
Semantics-wise we could decide either way.
I chose the option that seemed easier to implement and document
the behavior of FAN_MARK_VOLATILE.
Using FAN_MARK_CREATE as an alternative to FAN_MARK_ADD may
be a bit more elegant for UAPI though.
We could use a macro to get UAPI elegance without compromising simplicity:

#define FAN_MARK_NEW (FAN_MARK_ADD | FAN_MARK_CREATE)

Thanks,
Amir.