On Tue, Jul 02, 2024 at 01:07:57PM -0400, Brian Foster wrote: > The nr_dentry_negative counter is intended to only account negative > dentries that are present on the superblock LRU. Therefore, the LRU > add, remove and isolate helpers modify the counter based on whether > the dentry is negative, but the shrinker list related helpers do not > modify the counter, and the paths that change a dentry between > positive and negative only do so if DCACHE_LRU_LIST is set. > > The problem with this is that a dentry on a shrinker list still has > DCACHE_LRU_LIST set to indicate ->d_lru is in use. The additional > DCACHE_SHRINK_LIST flag denotes whether the dentry is on LRU or a > shrink related list. Therefore if a relevant operation (i.e. unlink) > occurs while a dentry is present on a shrinker list, and the > associated codepath only checks for DCACHE_LRU_LIST, then it is > technically possible to modify the negative dentry count for a > dentry that is off the LRU. Since the shrinker list related helpers > do not modify the negative dentry count (because non-LRU dentries > should not be included in the count) when the dentry is ultimately > removed from the shrinker list, this can cause the negative dentry > count to become permanently inaccurate. > > This problem can be reproduced via a heavy file create/unlink vs. > drop_caches workload. On an 80xcpu system, I start 80 tasks each > running a 1k file create/delete loop, and one task spinning on > drop_caches. After 10 minutes or so of runtime, the idle/clean cache > negative dentry count increases from somewhere in the range of 5-10 > entries to several hundred (and increasingly grows beyond > nr_dentry_unused). > > Tweak the logic in the paths that turn a dentry negative or positive > to filter out the case where the dentry is present on a shrink > related list. This allows the above workload to maintain an accurate > negative dentry count. > > Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx> This is sort of a subtle interaction, it took me a bit to piece it together, could you add a comment to the sections indicating the purpose of the extra check? Thanks, Josef
