On Fri 18-03-22 18:06:40, Amir Goldstein wrote: > > > > So far my thinking is that we either follow the path of possibly generating > > > > more events than necessary (i.e., any merge of two masks that do not both > > > > have FAN_MARK_VOLATILE set will clear FAN_MARK_VOLATILE) > > I agree that would provide predictable behavior which is also similar to > that of _SURV_MODIFY. > But IMO, this is very weird to explain/document in the wider sense. > However, if we only document that > "FAN_MARK_VOLATILE cannot be set on an existing mark and any update > of the mask without FAN_MARK_VOLATILE clears that flag" > (i.e. we make _VOLATILE imply the _CREATE behavior) > then the merge logic is the same as you suggested, but easier to explain. Yes, makes sense. > > > > or we rework the > > > > whole mark API (and implementation!) to completely avoid these strange > > > > effects of flag merging. I don't like FAN_MARK_CREATE much because IMO it > > > > solves only half of the problem - when new mark with a flag wants to merge > > > > with an existing mark, but does not solve the other half when some other > > > > mark wants to merge to a mark with a flag. Thoughts? > > > > > > > > > > Yes. Just one thought. > > > My applications never needed to change the mark mask after it was > > > set and I don't really see a huge use case for changing the mask > > > once it was set (besides removing the entire mark). > > > > > > So instead of FAN_MARK_CREATE, we may try to see if FAN_MARK_CONST > > > results in something that is useful and not too complicated to implement > > > and document. > > > > > > IMO using a "const" initialization for the "volatile" mark is not such a big > > > limitation and should not cripple the feature. > > > > OK, so basically if there's mark already placed at the inode and we try to > > add FAN_MARK_CONST, the addition would fail, and similarly if we later tried > > to add further mark to the inode with FAN_MARK_CONST mark, it would fail? > > > > Thinking out loud: What does FAN_MARK_CONST bring compared to the > > suggestion to go via the path of possibly generating more events by > > clearing FAN_MARK_VOLATILE? I guess some additional safety if you would add > > another mark to the same inode by an accident. Because if you never update > > marks, there's no problem with additional mark flags. > > Is the new flag worth it? Not sure... :) > > I rather not add new flags if we can do without them. > > To summarize my last proposal: > > 1. On fanotify_mark() with FAN_MARK_VOLATILE > 1.a. If the mark is new, the HAS_IREF flag is not set and no ihold() > 1.b. If mark already exists without HAS_IREF flag, mask is updated > 1.c. If mark already exists with HAS_IREF flag, mark add fails with EEXISTS > > 2. On fanotify_mark() without FAN_MARK_VOLATILE > 2.a. If the mark is new or exists without HAS_IREF, the HAS_IREF flag > is set and ihold() > 2.b. If mark already exists with HAS_IREF flag, mask is updated > > Do we have a winner? Sounds good to me. Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
