On Mon 20-06-22 16:45:51, Amir Goldstein wrote:
> This flag is a new way to configure ignore mask which allows adding and
> removing the event flags FAN_ONDIR and FAN_EVENT_ON_CHILD in ignore mask.
>
> The legacy FAN_MARK_IGNORED_MASK flag would always ignore events on
> directories and would ignore events on children depending on whether
> the FAN_EVENT_ON_CHILD flag was set in the (non ignored) mask.
>
> FAN_MARK_IGNORE can be used to ignore events on children without setting
> FAN_EVENT_ON_CHILD in the mark's mask and will not ignore events on
> directories unconditionally, only when FAN_ONDIR is set in ignore mask.
>
> The new behavior is sticky. After calling fanotify_mark() with
> FAN_MARK_IGNORE once, calling fanotify_mark() with FAN_MARK_IGNORED_MASK
> will update the ignore mask, but will not change the event flags in
> ignore mask nor how these flags are treated.
IMHO this stickyness is not very obvious. Wouldn't it be less error-prone
for users to say that once FAN_MARK_IGNORE is used for a mark, all
subsequent modifications of ignore mask have to use FAN_MARK_IGNORE? I mean
if some program bothers with FAN_MARK_IGNORE, I'd expect it to use it for
all its calls as otherwise the mixup is kind of difficult to reason
about...
Also it follows the behavior we have picked for FAN_MARK_EVICTABLE AFAIR
but that's not really important to me.
> @@ -1591,10 +1601,20 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
>
> /*
> * Event flags (FAN_ONDIR, FAN_EVENT_ON_CHILD) have no effect with
> - * FAN_MARK_IGNORED_MASK.
> + * FAN_MARK_IGNORED_MASK. They can be updated in ignore mask with
> + * FAN_MARK_IGNORE and then they do take effect.
> */
> - if (ignore)
> + switch (ignore) {
> + case 0:
> + case FAN_MARK_IGNORE:
> + break;
> + case FAN_MARK_IGNORED_MASK:
> mask &= ~FANOTIFY_EVENT_FLAGS;
> + umask = FANOTIFY_EVENT_FLAGS;
> + break;
> + default:
> + return -EINVAL;
> + }
I think this would be easier to follow as two ifs:
/* We don't allow FAN_MARK_IGNORE & FAN_MARK_IGNORED_MASK together */
if (ignore == FAN_MARK_IGNORE | FAN_MARK_IGNORED_MASK)
return -EINVAL;
/*
* Event flags (FAN_ONDIR, FAN_EVENT_ON_CHILD) have no effect with
* FAN_MARK_IGNORED_MASK.
*/
if (ignore == FAN_MARK_IGNORED_MASK) {
mask &= ~FANOTIFY_EVENT_FLAGS;
umask = FANOTIFY_EVENT_FLAGS;
}
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
