On Wed, Mar 23, 2022 at 4:28 PM Jan Kara <jack@xxxxxxx> wrote: > > On Wed 23-03-22 16:00:30, Amir Goldstein wrote: > > > Well, but reclaim from kswapd is always the main and preferred source of > > > memory reclaim. And we will kick kswapd to do work if we are running out of > > > memory. Doing direct filesystem slab reclaim from mark allocation is useful > > > only to throttle possibly aggressive mark allocations to the speed of > > > reclaim (instead of getting ENOMEM). So I'm still not convinced this is a > > > big issue but I certainly won't stop you from implementing more fine > > > grained GFP mode selection and lockdep annotations if you want to go that > > > way :). > > > > Well it was just two lines of code to annotate the fanotify mutex as its own > > class, so I just did that: > > > > https://github.com/amir73il/linux/commit/7b4b6e2c0bd1942cd130e9202c4b187a8fb468c6 > > But this implicitely assumes there isn't any allocation under mark_mutex > anywhere else where it is held. Which is likely true (I didn't check) but > it is kind of fragile. So I was rather imagining we would have per-group > "NOFS" flag and fsnotify_group_lock/unlock() would call > memalloc_nofs_save() based on the flag. And we would use > fsnotify_group_lock/unlock() uniformly across the whole fsnotify codebase. > I see what you mean, but looking at the code it seems quite a bit of churn to go over all the old backends and convert the locks to use wrappers where we "know" those backends are fs reclaim safe (because we did not get reports of deadlocks over the decades they existed). I think I will sleep better with a conversion to three flavors: 1. pflags = fsnotify_group_nofs_lock(fanotify_group); 2. fsnotify_group_lock(dnotify_group) => WARN_ON_ONCE(group->flags & FSNOTIFY_NOFS) mutex_lock(&group->mark_mutex) 3. fsnotify_group_lock_nested(group) => mutex_lock_nested(&group->mark_mutex, SINGLE_DEPTH_NESTING) Thoughts? One more UAPI question. Do you think we should require user to opt-in to NOFS and evictable marks with FAN_SHRINKABLE? If we don't, it will be harder to fix regressions in legacy fanotify workloads if those are reported without breaking the evictable marks UAPI. Thanks, Amir.
