Re: [PATCH 2/2] fanotify: introduce FAN_MARK_IGNORE
From: Jan Kara <jack@suse.cz>
Date: 2022-06-23 10:14:13
Also in:
linux-fsdevel
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.
quoted hunk ↗ jump to hunk
@@ -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 [off-list ref]
SUSE Labs, CR