On Thu, Nov 7, 2024 at 12:34 PM Amir Goldstein <amir73il@xxxxxxxxx> wrote: > > On Thu, Nov 7, 2024 at 8:53 PM Song Liu <songliubraving@xxxxxxxx> wrote: > > > > > > > > > On Nov 7, 2024, at 3:10 AM, Amir Goldstein <amir73il@xxxxxxxxx> wrote: > > > > > > On Wed, Oct 30, 2024 at 12:13 AM Song Liu <song@xxxxxxxxxx> wrote: > > >> > > >> This test shows a simplified logic that monitors a subtree. This is > > >> simplified as it doesn't handle all the scenarios, such as: > > >> > > >> 1) moving a subsubtree into/outof the being monitoring subtree; > > > > > > There is a solution for that (see below) > > > > > >> 2) mount point inside the being monitored subtree > > > > > > For that we will need to add the MOUNT/UNMOUNT/MOVE_MOUNT events, > > > but those have been requested by userspace anyway. > > > > > >> > > >> Therefore, this is not to show a way to reliably monitor a subtree. > > >> Instead, this is to test the functionalities of bpf based fastpath. > > >> To really monitor a subtree reliably, we will need more complex logic. > > > > > > Actually, this example is the foundation of my vision for efficient and race > > > free subtree filtering: > > > > > > 1. The inode map is to be treated as a cache for the is_subdir() query > > > > Using is_subdir() as the truth and managing the cache in inode map seems > > promising to me. > > > > > 2. Cache entries can also have a "distance from root" (i.e. depth) value > > > 3. Each unknown queried path can call is_subdir() and populate the cache > > > entries for all ancestors > > > 4. The cache/map size should be limited and when limit is reached, > > > evicting entries by depth priority makes sense > > > 5. A rename event for a directory whose inode is in the map and whose > > > new parent is not in the map or has a different value than old parent > > > needs to invalidate the entire map > > > 6. fast_path also needs a hook from inode evict to clear cache entries > > > > The inode map is physically attached to the inode itself. So the evict > > event is automatically handled. IOW, an inode's entry in the inode map > > is automatically removed when the inode is freed. For the same reason, > > we don't need to set a limit in map size and add evicting logic. Of > > course, this works based on the assumption that we don't use too much > > memory for each inode. I think this assumption is true. > > Oh no, it is definitely wrong each inode is around 1K and this was the > main incentive to implement FAN_MARK_EVICTABLE and > FAN_MARK_FILESYSTEK in the first place, because recursive > pinning of all inodes in a large tree is not scalable to large trees. The inode map does not pin the inode in memory. The inode will still be evicted as normal. The entry associated with the being evicted inode in the map will be freed together as the inoide is freed. The overhead I mentioned was the extra few bytes (for the flag, etc.) per inode. When an evicted inode is loaded again, we will use is_subdir() to recreate the entry in the inode map. Does this make sense and address the concern? Thanks, Song
