[PATCH] fs/super: skip non-memcg-aware nr_cached_objects in memcg slab shrink

[PATCH] fs/super: skip non-memcg-aware nr_cached_objects in memcg slab shrink

[Usama Arif]

The super_block shrinker is registered with SHRINKER_MEMCG_AWARE because its
dentry and inode LRUs are memcg-aware (via list_lru). But the optional
->nr_cached_objects() hooks that the shrinker also drives are not memcg-aware:
btrfs extent maps and xfs inode reclaim operate on filesystem-global
state, and shmem's unused-huge shrinker walks a per-superblock shrinklist.
None of them filter by sc->memcg.

The mismatch shows up under memcg-heavy slab reclaim. shrink_slab_memcg()
calls do_shrink_slab() once per (memcg, NUMA node) pair for every memcg
whose bit is set in the per-superblock shrinker bitmap, which on a busy
host means hundreds of calls per reclaim pass. Each scan queues the same
global shrinker work item that's already kicked from the root path.

Because btrfs/xfs global count is typically non-zero on any in-use filesystem,
the returned total stays positive even if a memcg's own dentry/inode LRUs
are empty. shrink_slab_memcg() therefore never clears the SB shrinker bit
in the memcg bitmap, so subsequent reclaim passes from the same memcg
re-enter super_cache_count() and pay for the global counter walk again.

Restrict ->nr_cached_objects() to the global shrink path (sc->memcg NULL
or root). The memcg-aware dentry/inode LRUs keep being counted and
scanned per memcg as before; only the global fs-specific hooks are skipped.
The root/global shrink path still drives those hooks; only their
invocation from non-root memcg slab reclaim is removed.

Signed-off-by: Usama Arif <usama.arif@linux.dev>
---
 fs/super.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/fs/super.c b/fs/super.c
index 378e81efe643..5216c5dbd4c4 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -24,6 +24,7 @@
 #include <linux/export.h>
 #include <linux/slab.h>
 #include <linux/blkdev.h>
+#include <linux/memcontrol.h>
 #include <linux/mount.h>
 #include <linux/security.h>
 #include <linux/writeback.h>		/* for the emergency remount stuff */
@@ -169,6 +170,19 @@ static void super_wake(struct super_block *sb, unsigned int flag)
 	wake_up_var(&sb->s_flags);
 }
 
+/*
+ * The s_op->nr_cached_objects hooks (used for example by btrfs and xfs)
+ * operate on filesystem-global state and ignore sc->memcg. Driving them
+ * from per-memcg shrink_slab_memcg() invocations only burns CPU walking
+ * per-cpu counters and queueing duplicate work: the actual reclaim happens on
+ * the global path (kswapd or root direct reclaim) regardless. Restrict them
+ * to that path.
+ */
+static inline bool super_fs_objects_eligible(struct shrink_control *sc)
+{
+	return !sc->memcg || mem_cgroup_is_root(sc->memcg);
+}
+
 /*
  * One thing we have to be careful of with a per-sb shrinker is that we don't
  * drop the last active reference to the superblock from within the shrinker.
@@ -198,7 +212,7 @@ static unsigned long super_cache_scan(struct shrinker *shrink,
 	if (!super_trylock_shared(sb))
 		return SHRINK_STOP;
 
-	if (sb->s_op->nr_cached_objects)
+	if (sb->s_op->nr_cached_objects && super_fs_objects_eligible(sc))
 		fs_objects = sb->s_op->nr_cached_objects(sb, sc);
 
 	inodes = list_lru_shrink_count(&sb->s_inode_lru, sc);
@@ -259,7 +273,8 @@ static unsigned long super_cache_count(struct shrinker *shrink,
 		return 0;
 	smp_rmb();
 
-	if (sb->s_op && sb->s_op->nr_cached_objects)
+	if (sb->s_op && sb->s_op->nr_cached_objects &&
+	    super_fs_objects_eligible(sc))
 		total_objects = sb->s_op->nr_cached_objects(sb, sc);
 
 	total_objects += list_lru_shrink_count(&sb->s_dentry_lru, sc);
-- 
2.52.0

Re: [PATCH] fs/super: skip non-memcg-aware nr_cached_objects in memcg slab shrink

[Jan Kara]

On Tue 09-06-26 05:30:47, Usama Arif wrote:
> The super_block shrinker is registered with SHRINKER_MEMCG_AWARE because its
> dentry and inode LRUs are memcg-aware (via list_lru). But the optional
> ->nr_cached_objects() hooks that the shrinker also drives are not memcg-aware:
> btrfs extent maps and xfs inode reclaim operate on filesystem-global
> state, and shmem's unused-huge shrinker walks a per-superblock shrinklist.
> None of them filter by sc->memcg.
> 
> The mismatch shows up under memcg-heavy slab reclaim. shrink_slab_memcg()
> calls do_shrink_slab() once per (memcg, NUMA node) pair for every memcg
> whose bit is set in the per-superblock shrinker bitmap, which on a busy
> host means hundreds of calls per reclaim pass. Each scan queues the same
> global shrinker work item that's already kicked from the root path.
> 
> Because btrfs/xfs global count is typically non-zero on any in-use filesystem,
> the returned total stays positive even if a memcg's own dentry/inode LRUs
> are empty. shrink_slab_memcg() therefore never clears the SB shrinker bit
> in the memcg bitmap, so subsequent reclaim passes from the same memcg
> re-enter super_cache_count() and pay for the global counter walk again.
> 
> Restrict ->nr_cached_objects() to the global shrink path (sc->memcg NULL
> or root). The memcg-aware dentry/inode LRUs keep being counted and
> scanned per memcg as before; only the global fs-specific hooks are skipped.
> The root/global shrink path still drives those hooks; only their
> invocation from non-root memcg slab reclaim is removed.
> 
> Signed-off-by: Usama Arif <usama.arif@linux.dev>

To me this makes sense. However I'm bit surprised that XFS inode shrinker
(which is what gets counted in nr_cached_objects for XFS) isn't memcg
aware. I guess since these inodes are on their way to a relatively quick
destruction, nobody really bothered. So I'm fine with the change, just
I'd like to make sure XFS folks are aware and don't plan anything in this
area.

								Honza

> ---
>  fs/super.c | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/super.c b/fs/super.c
> index 378e81efe643..5216c5dbd4c4 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -24,6 +24,7 @@
>  #include <linux/export.h>
>  #include <linux/slab.h>
>  #include <linux/blkdev.h>
> +#include <linux/memcontrol.h>
>  #include <linux/mount.h>
>  #include <linux/security.h>
>  #include <linux/writeback.h>		/* for the emergency remount stuff */
> @@ -169,6 +170,19 @@ static void super_wake(struct super_block *sb, unsigned int flag)
>  	wake_up_var(&sb->s_flags);
>  }
>  
> +/*
> + * The s_op->nr_cached_objects hooks (used for example by btrfs and xfs)
> + * operate on filesystem-global state and ignore sc->memcg. Driving them
> + * from per-memcg shrink_slab_memcg() invocations only burns CPU walking
> + * per-cpu counters and queueing duplicate work: the actual reclaim happens on
> + * the global path (kswapd or root direct reclaim) regardless. Restrict them
> + * to that path.
> + */
> +static inline bool super_fs_objects_eligible(struct shrink_control *sc)
> +{
> +	return !sc->memcg || mem_cgroup_is_root(sc->memcg);
> +}
> +
>  /*
>   * One thing we have to be careful of with a per-sb shrinker is that we don't
>   * drop the last active reference to the superblock from within the shrinker.
> @@ -198,7 +212,7 @@ static unsigned long super_cache_scan(struct shrinker *shrink,
>  	if (!super_trylock_shared(sb))
>  		return SHRINK_STOP;
>  
> -	if (sb->s_op->nr_cached_objects)
> +	if (sb->s_op->nr_cached_objects && super_fs_objects_eligible(sc))
>  		fs_objects = sb->s_op->nr_cached_objects(sb, sc);
>  
>  	inodes = list_lru_shrink_count(&sb->s_inode_lru, sc);
> @@ -259,7 +273,8 @@ static unsigned long super_cache_count(struct shrinker *shrink,
>  		return 0;
>  	smp_rmb();
>  
> -	if (sb->s_op && sb->s_op->nr_cached_objects)
> +	if (sb->s_op && sb->s_op->nr_cached_objects &&
> +	    super_fs_objects_eligible(sc))
>  		total_objects = sb->s_op->nr_cached_objects(sb, sc);
>  
>  	total_objects += list_lru_shrink_count(&sb->s_dentry_lru, sc);
> -- 
> 2.52.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH] fs/super: skip non-memcg-aware nr_cached_objects in memcg slab shrink

[Usama Arif]

On 09/06/2026 13:52, Jan Kara wrote:
> On Tue 09-06-26 05:30:47, Usama Arif wrote:
>> The super_block shrinker is registered with SHRINKER_MEMCG_AWARE because its
>> dentry and inode LRUs are memcg-aware (via list_lru). But the optional
>> ->nr_cached_objects() hooks that the shrinker also drives are not memcg-aware:
>> btrfs extent maps and xfs inode reclaim operate on filesystem-global
>> state, and shmem's unused-huge shrinker walks a per-superblock shrinklist.
>> None of them filter by sc->memcg.
>>
>> The mismatch shows up under memcg-heavy slab reclaim. shrink_slab_memcg()
>> calls do_shrink_slab() once per (memcg, NUMA node) pair for every memcg
>> whose bit is set in the per-superblock shrinker bitmap, which on a busy
>> host means hundreds of calls per reclaim pass. Each scan queues the same
>> global shrinker work item that's already kicked from the root path.
>>
>> Because btrfs/xfs global count is typically non-zero on any in-use filesystem,
>> the returned total stays positive even if a memcg's own dentry/inode LRUs
>> are empty. shrink_slab_memcg() therefore never clears the SB shrinker bit
>> in the memcg bitmap, so subsequent reclaim passes from the same memcg
>> re-enter super_cache_count() and pay for the global counter walk again.
>>
>> Restrict ->nr_cached_objects() to the global shrink path (sc->memcg NULL
>> or root). The memcg-aware dentry/inode LRUs keep being counted and
>> scanned per memcg as before; only the global fs-specific hooks are skipped.
>> The root/global shrink path still drives those hooks; only their
>> invocation from non-root memcg slab reclaim is removed.
>>
>> Signed-off-by: Usama Arif <usama.arif@linux.dev>
> 
> To me this makes sense. However I'm bit surprised that XFS inode shrinker
> (which is what gets counted in nr_cached_objects for XFS) isn't memcg
> aware. I guess since these inodes are on their way to a relatively quick
> destruction, nobody really bothered. So I'm fine with the change, just
> I'd like to make sure XFS folks are aware and don't plan anything in this
> area.
> 

Thanks for the review!

Yes I am hoping that if there are any objections from xfs or btrfs, it gets
raised. Have cc'ed the btrfs and xfs maintainers and reviewers.

Thanks!
Usama


> 								Honza
> 
>> ---
>>  fs/super.c | 19 +++++++++++++++++--
>>  1 file changed, 17 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/super.c b/fs/super.c
>> index 378e81efe643..5216c5dbd4c4 100644
>> --- a/fs/super.c
>> +++ b/fs/super.c
>> @@ -24,6 +24,7 @@
>>  #include <linux/export.h>
>>  #include <linux/slab.h>
>>  #include <linux/blkdev.h>
>> +#include <linux/memcontrol.h>
>>  #include <linux/mount.h>
>>  #include <linux/security.h>
>>  #include <linux/writeback.h>		/* for the emergency remount stuff */
>> @@ -169,6 +170,19 @@ static void super_wake(struct super_block *sb, unsigned int flag)
>>  	wake_up_var(&sb->s_flags);
>>  }
>>  
>> +/*
>> + * The s_op->nr_cached_objects hooks (used for example by btrfs and xfs)
>> + * operate on filesystem-global state and ignore sc->memcg. Driving them
>> + * from per-memcg shrink_slab_memcg() invocations only burns CPU walking
>> + * per-cpu counters and queueing duplicate work: the actual reclaim happens on
>> + * the global path (kswapd or root direct reclaim) regardless. Restrict them
>> + * to that path.
>> + */
>> +static inline bool super_fs_objects_eligible(struct shrink_control *sc)
>> +{
>> +	return !sc->memcg || mem_cgroup_is_root(sc->memcg);
>> +}
>> +
>>  /*
>>   * One thing we have to be careful of with a per-sb shrinker is that we don't
>>   * drop the last active reference to the superblock from within the shrinker.
>> @@ -198,7 +212,7 @@ static unsigned long super_cache_scan(struct shrinker *shrink,
>>  	if (!super_trylock_shared(sb))
>>  		return SHRINK_STOP;
>>  
>> -	if (sb->s_op->nr_cached_objects)
>> +	if (sb->s_op->nr_cached_objects && super_fs_objects_eligible(sc))
>>  		fs_objects = sb->s_op->nr_cached_objects(sb, sc);
>>  
>>  	inodes = list_lru_shrink_count(&sb->s_inode_lru, sc);
>> @@ -259,7 +273,8 @@ static unsigned long super_cache_count(struct shrinker *shrink,
>>  		return 0;
>>  	smp_rmb();
>>  
>> -	if (sb->s_op && sb->s_op->nr_cached_objects)
>> +	if (sb->s_op && sb->s_op->nr_cached_objects &&
>> +	    super_fs_objects_eligible(sc))
>>  		total_objects = sb->s_op->nr_cached_objects(sb, sc);
>>  
>>  	total_objects += list_lru_shrink_count(&sb->s_dentry_lru, sc);
>> -- 
>> 2.52.0
>>