On Mon, Apr 11, 2022 at 5:19 PM Jan Kara <jack@xxxxxxx> wrote: > > On Mon 11-04-22 15:57:30, Amir Goldstein wrote: > > On Mon, Apr 11, 2022 at 2:47 PM Jan Kara <jack@xxxxxxx> wrote: > > > > > > On Tue 29-03-22 10:49:01, Amir Goldstein wrote: > > > > When an inode mark is created with flag FAN_MARK_EVICTABLE, it will not > > > > pin the marked inode to inode cache, so when inode is evicted from cache > > > > due to memory pressure, the mark will be lost. > > > > > > > > When an inode mark with flag FAN_MARK_EVICATBLE is updated without using > > > > this flag, the marked inode is pinned to inode cache. > > > > > > > > When an inode mark is updated with flag FAN_MARK_EVICTABLE but an > > > > existing mark already has the inode pinned, the mark update fails with > > > > error EEXIST. > > > > > > I was thinking about this. FAN_MARK_EVICTABLE is effectively a hint to the > > > kernel - you can drop this if you wish. So does it make sense to return > > > error when we cannot follow the hint? Doesn't this just add unnecessary > > > work (determining whether the mark should be evictable or not) to the > > > userspace application using FAN_MARK_EVICTABLE? > > > > I do not fully agree about your definition of "hint to the kernel". > > Yes, for a single inode it may be a hint, but for a million inodes it is pretty > > much a directive that setting a very large number of evictable marks > > CANNOT be used to choke the system. > > > > It's true that the application should be able to avoid shooting its own > > foot and we do not need to be the ones providing this protection, but > > I rather prefer to keep the API more strict and safe than being sorry later. > > After all, I don't think this complicates the implementation nor documentation > > too much. Is it? see: > > > > https://github.com/amir73il/man-pages/commit/b52eb7d1a8478cbd1456f4d9463902bbc4e80f0d > > No, it is not complicating things too much and you're probably right that > having things stricter now may pay off in the future. I was just thinking > that app adding ignore mark now needs to remember whether it has already > added something else for the inode or not to know whether it can use > FAN_MARK_EVICTABLE. Which seemed like unnecessary complication. > Well the way my app does it, it has a hardcoded list of "must exclude" dirs which is sets on startup and then evictable ignore masks are added lazily when events in non-interesting path show up. If app would somehow get an event with path that was in the "must exclude" set, then adding evictable mark would fail and nothing to it because it is an optimization anyway. There is no tracking involved. Anyway, I see there is a bug in EEXIST case, the error is returned *after* updating the mask - I will fix it and re-post v3 with the rest of the fixes. Thanks, Amir.
