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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri 18-03-22 13:04:51, Amir Goldstein wrote:
> > > 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.
> >
> > Yeah, but with FAN_MARK_VOLATILE the problem also goes the other way
> > around. If I set FAN_MARK_VOLATILE on some inode and later add something to
> > a normal mask, I might be rightfully surprised when the mark gets evicted
> > and thus I will not get events I'm expecting. Granted the application would
> > be stepping on its own toes because marks are "merged" only for the same
> > notification group but still it could be surprising and avoiding such
> > mishaps would probably involve extra tracking on the application side.
> >
> > The problem essentially lies in mixing mark "flags" (ONDIR, ON_CHILD,
> > VOLATILE, SURV_MODIFY) with mark mask. Mark operations with identical set
> > of flags can be merged without troubles but once flags are different
> > results of the merge are always "interesting". So far the consequences were
> > mostly benign (getting more events than the application may have expected)
> > but with FAN_MARK_VOLATILE we can also start loosing events and that is
> > more serious.
> >
> > 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) 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...  :)

								Honza
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux