mm: shmem: make unused huge shrinker memcg aware
Subsystems (2)
PATCH 1/1 mm: shmem: make unused huge shrinker memcg aware
From: Qi Zheng <[email protected]>
The shmem unused huge shrinker keeps a per-superblock list of inodes whose tail huge folio extends beyond i_size. Since that list is not memcg aware, reclaim triggered by one memcg can scan inodes from the whole superblock and split shmem huge folios charged to unrelated memcgs.
Convert the shrink list to a memcg-aware list_lru. Queue each inode on the list_lru sublist matching the memcg and node of the current tail huge folio, so non-root memcg reclaim only walks candidates charged to the reclaiming memcg. Global reclaim, root memcg reclaim and shmem quota reclaim keep global semantics.
The list_lru still tracks inodes while the actual split target is the current tail huge folio, so validate the folio memcg/node during scan. If the folio no longer matches the reclaim context or splitting cannot proceed, requeue the inode according to the current tail folio; if the inode is no longer shrinkable, drop the scan entry.
This can be tested with the shrinker debugfs interface by allocating 32 tmpfs tail THPs in each of two memcgs, then scanning the sb-tmpfs shrinker with memcg A's cgroup id:
before A scan after A scan base A=64M, B=64M A=0, B=0 patched A=64M, B=64M A=0, B=64M
---
This patch is based on next-20260701 because it doesn't include this patch:
https://lore.kernel.org/all/[email protected]/
Later on, Usama will consider moving this restriction down into the fs callback.
include/linux/shmem_fs.h | 14 +- mm/shmem.c | 379 ++++++++++++++++++++++++++++++--------- 2 files changed, 310 insertions(+), 83 deletions(-)
include/linux/shmem_fs.h +11 -3
@@ -11,6 +11,7 @@#include <linux/fs_parser.h> #include <linux/userfaultfd_k.h> #include <linux/bits.h> +#include <linux/list_lru.h> struct swap_iocb ;
@@ -56,6 +57,13 @@ struct shmem_inode_info {struct dquot __rcu *i_dquot[MAXQUOTAS ];#endif struct inode vfs_inode; + +#ifdef CONFIG_TRANSPARENT_HUGEPAGE +struct list_head shrinklist_scan; +struct mem_cgroup *shrinklist_memcg; +int shrinklist_nid; +bool shrinklist_isolated; +#endif };#define SHMEM_FL_USER_VISIBLE (FS_FL_USER_VISIBLE | FS_CASEFOLD_FL)
@@ -85,9 +93,9 @@ struct shmem_sb_info {ino_t next_ino;/* The next per-sb inode number to use */ ino_t __percpu *ino_batch;/* The next per-cpu inode number to use */ struct mempolicy *mpol;/* default memory policy for mappings */ -spinlock_t shrinklist_lock;/* Protects shrinklist */ -struct list_head shrinklist;/* List of shinkable inodes */ -unsigned long shrinklist_len;/* Length of shrinklist */ +#ifdef CONFIG_TRANSPARENT_HUGEPAGE +struct list_lru shrinklist;/* List of shrinkable inodes */ +#endif struct shmem_quota_limits qlimits;/* Default quota limits */ struct simple_xattr_cache xa_cache; };
mm/shmem.c +299 -80
476 lines not on this page · raw message
Signature · 1 line
2.54.0
Andrew Morton
maintainerQuoted 19 lines · On Tue, 14 Jul 2026 11:19:18 +0800 Qi Zheng <[email protected]> wrote:
From: Qi Zheng <[email protected]>
The shmem unused huge shrinker keeps a per-superblock list of inodes whose tail huge folio extends beyond
i_size. Since that list is not memcg aware, reclaim triggered by one memcg can scan inodes from the whole superblock and split shmem huge folios charged to unrelated memcgs.Convert the shrink list to a memcg-aware
list_lru. Queue each inode on thelist_lrusublist matching the memcg and node of the current tail huge folio, so non-root memcg reclaim only walks candidates charged to the reclaiming memcg. Global reclaim, root memcg reclaim and shmem quota reclaim keep global semantics.The
list_lrustill tracks inodes while the actual split target is the current tail huge folio, so validate the folio memcg/node during scan. If the folio no longer matches the reclaim context or splitting cannot proceed, requeue the inode according to the current tail folio; if the inode is no longer shrinkable, drop the scan entry.
Quoted 7 lines
This can be tested with the shrinker debugfs interface by allocating 32 tmpfs tail THPs in each of two memcgs, then scanning the sb-tmpfs shrinker with memcg A's cgroup id:
before A scan after A scan base A=64M, B=64M A=0, B=0 patched A=64M, B=64M A=0, B=64M
Sashiko said a thing:
https://sashiko.dev/#/patchset/[email protected]
Quoted 5 lines
This patch is based on next-20260701 because it doesn't include this patch:
https://lore.kernel.org/all/[email protected]/
Later on, Usama will consider moving this restriction down into the fs callback.
I'm not understanding. Was Usama's above patch ever included in anything?
Quoted 14 lines
include/linux/shmem_fs.h +3
-1-unsigned long shrinklist_len;/* Length of shrinklist */ +#ifdef CONFIG_TRANSPARENT_HUGEPAGE +struct list_lru shrinklist;/* List of shrinkable inodes */ +#endif struct shmem_quota_limits qlimits;/* Default quota limits */ struct simple_xattr_cache xa_cache; };mm/shmem.c +0
-0@@ -724,50 +724,252 @@ static const char *shmem_format_huge(int huge) }#endif
mm/shmem.c static const char *shmem_format_huge(int huge) · line 735
Quoted 10 lines
mm/shmem.c +9
-0+static inline struct mem_cgroup *shmem_get_and_clear_memcg (struct shmem_inode_info *info) +{ +struct mem_cgroup *memcg = info->shrinklist_memcg; + + info->shrinklist_memcg =NULL ; + info->shrinklist_nid =-1 ; + +return memcg; +}
Explicit inlining shouldn't be needed - gcc will figure it out.
Quoted 17 lines
mm/shmem.c +17
-0+ +static void shmem_unused_huge_add (struct inode *inode,struct folio *folio, +gfp_t gfp) +{ +struct shmem_inode_info *info =SHMEM_I (inode); +struct shmem_sb_info *sbinfo =SHMEM_SB (inode->i_sb); +int nid =folio_nid (folio); +struct mem_cgroup *memcg =NULL , *old_memcg =NULL ; + + memcg =shmem_unused_huge_alloc_lru (sbinfo, folio, gfp); +if (IS_ERR (memcg)) +return ; + +spin_lock (&info->lock); +if (!list_empty (&info->shrinklist)) { +if (info->shrinklist_nid == nid && + info->shrinklist_memcg == memcg)
mm/shmem.c static const char *shmem_format_huge(int huge) · line 791
Quoted 10 lines
mm/shmem.c +10
-0+ goto unlock; + +list_lru_del (&sbinfo->shrinklist, &info->shrinklist, + info->shrinklist_nid, info->shrinklist_memcg); + old_memcg =shmem_get_and_clear_memcg (info); + } + +list_lru_add (&sbinfo->shrinklist, &info->shrinklist, nid, memcg); + info->shrinklist_memcg = memcg; + info->shrinklist_nid = nid;
Totally pointless style thing: it's nice to fully initialize the object before adding it to the list.
Quoted 11 lines
mm/shmem.c +8
-0+ memcg =NULL ; +unlock: +spin_unlock (&info->lock); +mem_cgroup_put (old_memcg); +mem_cgroup_put (memcg); +} .., +static enum lru_status shmem_unused_huge_isolate(struct list_head *item, + struct list_lru_one *lru,
mm/shmem.c static const char *shmem_format_huge(int huge) · line 832
Quoted 10 lines
mm/shmem.c +10
-0+ void *arg) +{ +struct shmem_unused_huge_scan *scan = arg; +struct shmem_inode_info *info; +struct inode *inode; +struct mem_cgroup *memcg =NULL ; + + info = list_entry(item, struct shmem_inode_info, shrinklist); +if (!spin_trylock (&info->lock)) +return LRU_SKIP ;
Why the trylock? Please add comment explaining its use.
Quoted 29 lines
mm/shmem.c +29
-0+if (info->shrinklist_isolated) { +spin_unlock (&info->lock); +return LRU_SKIP ; + } + + inode =igrab (&info->vfs_inode); +if (!inode) { +/* +* This inode is being evicted, just drop its stale active LRU +* entry. +*/ +list_lru_isolate (lru, item); + memcg =shmem_get_and_clear_memcg (info); +spin_unlock (&info->lock); +mem_cgroup_put (memcg); +return LRU_REMOVED ; + } + + info->shrinklist_isolated = true; +list_lru_isolate (lru, item); + memcg =shmem_get_and_clear_memcg (info); +list_add_tail (&info->shrinklist_scan, &scan->list); +spin_unlock (&info->lock); +mem_cgroup_put (memcg); + +return LRU_REMOVED ; +} + +static bool is_shmem_unused_huge_readded (struct inode *inode)
mm/shmem.c static const char *shmem_format_huge(int huge) · line 872
Quoted 10 lines
mm/shmem.c +10
-0+{ +struct shmem_inode_info *info =SHMEM_I (inode); +bool readded; + +spin_lock (&info->lock); + readded = info->shrinklist_isolated && !list_empty (&info->shrinklist); +spin_unlock (&info->lock); + +return readded; +}
The return value is out of date as soon as the lock is dropped? A comment explaining why this is OK would be helpful.
Quoted 18 lines
mm/shmem.c +17
-0+static bool is_shmem_unused_huge_match (struct folio *folio, +struct shrink_control *sc) +{ +struct mem_cgroup *memcg =NULL ; +bool match; + +/* +* Only non-root memcg reclaim needs to match the folio charge against +* sc->memcg. Skip the folio memcg check for the following cases: +* 1. shmem quota reclaim (sc == NULL) +* 2. global shrinker reclaim +* 3. root memcg reclaim +*/ +if (!sc || !sc->memcg ||mem_cgroup_is_root (sc->memcg)) +return true; + +if (folio_nid (folio) != sc->nid)
mm/shmem.c static const char *shmem_format_huge(int huge) · line 953
Quoted 10 lines
mm/shmem.c +8
-0+return false; + + memcg =get_mem_cgroup_from_folio (folio); + match = memcg == sc->memcg; +mem_cgroup_put (memcg); + +return match; +}
Generally speaking, the code seems a bit thin on comments explaining why functions exist, why the code is doing a partucular thing, etc.
Hi Andrew,
Quoted 25 lines · Andrew Morton
Quoted 23 lines · On Tue, 14 Jul 2026 11:19:18 +0800 Qi Zheng <[email protected]> wrote:
From: Qi Zheng <[email protected]>
The shmem unused huge shrinker keeps a per-superblock list of inodes whose tail huge folio extends beyond
i_size. Since that list is not memcg aware, reclaim triggered by one memcg can scan inodes from the whole superblock and split shmem huge folios charged to unrelated memcgs.Convert the shrink list to a memcg-aware
list_lru. Queue each inode on thelist_lrusublist matching the memcg and node of the current tail huge folio, so non-root memcg reclaim only walks candidates charged to the reclaiming memcg. Global reclaim, root memcg reclaim and shmem quota reclaim keep global semantics.The
list_lrustill tracks inodes while the actual split target is the current tail huge folio, so validate the folio memcg/node during scan. If the folio no longer matches the reclaim context or splitting cannot proceed, requeue the inode according to the current tail folio; if the inode is no longer shrinkable, drop the scan entry.This can be tested with the shrinker debugfs interface by allocating 32 tmpfs tail THPs in each of two memcgs, then scanning the sb-tmpfs shrinker with memcg A's cgroup id:
Quoted 6 lines
Quoted 3 lines
before A scan after A scan base A=64M, B=64M A=0, B=0 patched A=64M, B=64M A=0, B=64MSashiko said a thing:
https://sashiko.dev/#/patchset/[email protected]
Got it. I noticed that too. shrinklist_memcg and shrinklist_isolated require explicit initalization. I'll fix this in the next version once the following patch is merged into linux-next:
https://lore.kernel.org/all/[email protected]/
Quoted 9 lines
Quoted 6 lines
This patch is based on next-20260701 because it doesn't include this patch:
https://lore.kernel.org/all/[email protected]/
Later on, Usama will consider moving this restriction down into the fs callback.
I'm not understanding. Was Usama's above patch ever included in anything?
It's included in linux-next, e.g. next-20260710.
Quoted 16 lines
Quoted 16 lines
include/linux/shmem_fs.h +3
-1-unsigned long shrinklist_len;/* Length of shrinklist */ +#ifdef CONFIG_TRANSPARENT_HUGEPAGE +struct list_lru shrinklist;/* List of shrinkable inodes */ +#endif struct shmem_quota_limits qlimits;/* Default quota limits */ struct simple_xattr_cache xa_cache; };mm/shmem.c +0
-0@@ -724,50 +724,252 @@ static const char *shmem_format_huge(int huge) }#endif
Quoted 11 lines
mm/shmem.c static const char *shmem_format_huge(int huge) · line 735
Quoted 9 lines
mm/shmem.c +9
-0+static inline struct mem_cgroup *shmem_get_and_clear_memcg (struct shmem_inode_info *info) +{ +struct mem_cgroup *memcg = info->shrinklist_memcg; + + info->shrinklist_memcg =NULL ; + info->shrinklist_nid =-1 ; + +return memcg; +}Explicit inlining shouldn't be needed - gcc will figure it out.
Got it.
Quoted 23 lines
Quoted 23 lines
mm/shmem.c +22
-0+ +static void shmem_unused_huge_add (struct inode *inode,struct folio *folio, +gfp_t gfp) +{ +struct shmem_inode_info *info =SHMEM_I (inode); +struct shmem_sb_info *sbinfo =SHMEM_SB (inode->i_sb); +int nid =folio_nid (folio); +struct mem_cgroup *memcg =NULL , *old_memcg =NULL ; + + memcg =shmem_unused_huge_alloc_lru (sbinfo, folio, gfp); +if (IS_ERR (memcg)) +return ; + +spin_lock (&info->lock); +if (!list_empty (&info->shrinklist)) { +if (info->shrinklist_nid == nid && + info->shrinklist_memcg == memcg) + goto unlock; + +list_lru_del (&sbinfo->shrinklist, &info->shrinklist, + info->shrinklist_nid, info->shrinklist_memcg); + old_memcg =shmem_get_and_clear_memcg (info);
Quoted 8 lines
mm/shmem.c static const char *shmem_format_huge(int huge) · line 791
Quoted 5 lines
mm/shmem.c +5
-0+ } + +list_lru_add (&sbinfo->shrinklist, &info->shrinklist, nid, memcg); + info->shrinklist_memcg = memcg; + info->shrinklist_nid = nid;Totally pointless style thing: it's nice to fully initialize the object before adding it to the list.
Got it.
Quoted 10 lines
Quoted 10 lines
mm/shmem.c +6
-0+ memcg =NULL ; +unlock: +spin_unlock (&info->lock); +mem_cgroup_put (old_memcg); +mem_cgroup_put (memcg); +} ..,
Quoted 14 lines
mm/shmem.c static const char *shmem_format_huge(int huge) · line 832
Quoted 12 lines
mm/shmem.c +12
-0+static enum lru_status shmem_unused_huge_isolate (struct list_head *item, +struct list_lru_one *lru, +void *arg) +{ +struct shmem_unused_huge_scan *scan = arg; +struct shmem_inode_info *info; +struct inode *inode; +struct mem_cgroup *memcg =NULL ; + + info = list_entry(item, struct shmem_inode_info, shrinklist); +if (!spin_trylock (&info->lock)) +return LRU_SKIP ;Why the trylock? Please add comment explaining its use.
This is to prevent an ABBA deadlock.
The shmem add/delete/requeue paths take info->lock first and then call list_lru_add()/list_lru_del(), which acquire the list_lru lock. But the current path takes the list_lru lock first, hence the use of trylock here.
Will add a comment explaining this.
Quoted 36 lines
Quoted 36 lines
mm/shmem.c +34
-0+if (info->shrinklist_isolated) { +spin_unlock (&info->lock); +return LRU_SKIP ; + } + + inode =igrab (&info->vfs_inode); +if (!inode) { +/* +* This inode is being evicted, just drop its stale active LRU +* entry. +*/ +list_lru_isolate (lru, item); + memcg =shmem_get_and_clear_memcg (info); +spin_unlock (&info->lock); +mem_cgroup_put (memcg); +return LRU_REMOVED ; + } + + info->shrinklist_isolated = true; +list_lru_isolate (lru, item); + memcg =shmem_get_and_clear_memcg (info); +list_add_tail (&info->shrinklist_scan, &scan->list); +spin_unlock (&info->lock); +mem_cgroup_put (memcg); + +return LRU_REMOVED ; +} + +static bool is_shmem_unused_huge_readded (struct inode *inode) +{ +struct shmem_inode_info *info =SHMEM_I (inode); +bool readded; + +spin_lock (&info->lock);
Quoted 8 lines
mm/shmem.c static const char *shmem_format_huge(int huge) · line 872
Quoted 5 lines
mm/shmem.c +5
-0+ readded = info->shrinklist_isolated && !list_empty (&info->shrinklist); +spin_unlock (&info->lock); + +return readded; +}The return value is out of date as soon as the lock is dropped? A comment explaining why this is OK would be helpful.
Yes, the value is only a snapshot. The helper is a conservative race detector for the case where the shrinker has an inode on the local scan list and a concurrent fault has already queued the same inode back on the active list_lru. If we observe that, the caller drops the scan entry and lets the active list_lru entry represent the current tail folio.
Will add a comment explaining this.
Quoted 27 lines
Quoted 27 lines
mm/shmem.c +25
-0+static bool is_shmem_unused_huge_match (struct folio *folio, +struct shrink_control *sc) +{ +struct mem_cgroup *memcg =NULL ; +bool match; + +/* +* Only non-root memcg reclaim needs to match the folio charge against +* sc->memcg. Skip the folio memcg check for the following cases: +* 1. shmem quota reclaim (sc == NULL) +* 2. global shrinker reclaim +* 3. root memcg reclaim +*/ +if (!sc || !sc->memcg ||mem_cgroup_is_root (sc->memcg)) +return true; + +if (folio_nid (folio) != sc->nid) +return false; + + memcg =get_mem_cgroup_from_folio (folio); + match = memcg == sc->memcg; +mem_cgroup_put (memcg); + +return match; +}
Quoted 5 lines
Quoted 2 lines
...
Generally speaking, the code seems a bit thin on comments explaining why functions exist, why the code is doing a partucular thing, etc.
Will add more comments in the next version.
Thanks,
Qi
Quoted 1 line
Andrew Morton
maintainerQuoted 11 lines · On Wed, 15 Jul 2026 14:49:28 +0800 Qi Zheng <[email protected]> wrote:
Quoted 9 lines
Quoted 6 lines
This patch is based on next-20260701 because it doesn't include this patch:
https://lore.kernel.org/all/[email protected]/
Later on, Usama will consider moving this restriction down into the fs callback.
I'm not understanding. Was Usama's above patch ever included in anything?
It's included in linux-next, e.g. next-20260710.
Oh, OK, it came in via Christian's vfs.git tree.
0baad6f9b997 ("fs/super: skip non-memcg-aware nr_cached_objects in memcg slab shrink")
Should we be asking Christian to make some alteration here?
Quoted 12 lines · Andrew Morton
Quoted 10 lines · On Wed, 15 Jul 2026 14:49:28 +0800 Qi Zheng <[email protected]> wrote:
Quoted 9 lines
Quoted 6 lines
This patch is based on next-20260701 because it doesn't include this patch:
https://lore.kernel.org/all/[email protected]/
Later on, Usama will consider moving this restriction down into the fs callback.
I'm not understanding. Was Usama's above patch ever included in anything?
Quoted 7 lines
Quoted 1 line
It's included in linux-next, e.g. next-20260710.
Oh, OK, it came in via Christian's vfs.git tree.
0baad6f9b997("fs/super: skip non-memcg-awarenr_cached_objectsin memcg slab shrink")Should we be asking Christian to make some alteration here?
I saw Usama posted an alternative (now at v2) and got Reviewed-bys:
https://lore.kernel.org/all/[email protected]/
Hi Christian, do you have plans to merge this into the vfs.git tree recently?
Thanks,
Qi
Baolin Wang
reviewerHi Qi,
Quoted 23 lines · Qi Zheng
From: Qi Zheng <[email protected]>
The shmem unused huge shrinker keeps a per-superblock list of inodes whose tail huge folio extends beyond
i_size. Since that list is not memcg aware, reclaim triggered by one memcg can scan inodes from the whole superblock and split shmem huge folios charged to unrelated memcgs.Convert the shrink list to a memcg-aware
list_lru. Queue each inode on thelist_lrusublist matching the memcg and node of the current tail huge folio, so non-root memcg reclaim only walks candidates charged to the reclaiming memcg. Global reclaim, root memcg reclaim and shmem quota reclaim keep global semantics.The
list_lrustill tracks inodes while the actual split target is the current tail huge folio, so validate the folio memcg/node during scan. If the folio no longer matches the reclaim context or splitting cannot proceed, requeue the inode according to the current tail folio; if the inode is no longer shrinkable, drop the scan entry.This can be tested with the shrinker debugfs interface by allocating 32 tmpfs tail THPs in each of two memcgs, then scanning the sb-tmpfs shrinker with memcg A's cgroup id:
Quoted 6 lines
before A scan after A scan base A=64M, B=64M A=0, B=0 patched A=64M, B=64M A=0, B=64MSigned-off-by Qi Zheng <[email protected]>---
Thanks for your patch. This work has been on my TODO list for a while :)
Overall it looks good. I'll take a closer look and run some tests.
Quoted 23 lines
This patch is based on next-20260701 because it doesn't include this patch:
https://lore.kernel.org/all/[email protected]/
Later on, Usama will consider moving this restriction down into the fs callback.
include/linux/shmem_fs.h | 14 +- mm/shmem.c | 379 ++++++++++++++++++++++++++++++--------- 2 files changed, 310 insertions(+), 83 deletions(-)include/linux/shmem_fs.h +1
-0@@ -11,6 +11,7 @@#include <linux/fs_parser.h> #include <linux/userfaultfd_k.h> #include <linux/bits.h> +#include <linux/list_lru.h> struct swap_iocb ;
include/linux/shmem_fs.h struct shmem_inode_info { · line 62
Quoted 6 lines
include/linux/shmem_fs.h +3
-0@@ -56,6 +57,13 @@ struct shmem_inode_info {struct dquot __rcu *i_dquot[MAXQUOTAS ];#endif struct inode vfs_inode; + +#ifdef CONFIG_TRANSPARENT_HUGEPAGE +struct list_head shrinklist_scan;
Why introduce another list_head member? Why can't we reuse 'shrinklist' in this structure? In other words, if we isolate this inode from list_lru, then 'inode_info->shrinklist' can be reused to link it into the temporary list, right?
include/linux/shmem_fs.h struct shmem_inode_info { · line 65
Quoted 3 lines
include/linux/shmem_fs.h +3
-0+struct mem_cgroup *shrinklist_memcg; +int shrinklist_nid; +bool shrinklist_isolated;
Do we need an extra bool to indicate whether it's isolated? Can't we just use 'list_empty(inode_info->shrinklist)'? I may have missed some details.
Hi Baolin,
Quoted 30 lines · Baolin Wang
Hi Qi,
Quoted 27 lines · Qi Zheng
From: Qi Zheng <[email protected]>
The shmem unused huge shrinker keeps a per-superblock list of inodes whose tail huge folio extends beyond
i_size. Since that list is not memcg aware, reclaim triggered by one memcg can scan inodes from the whole superblock and split shmem huge folios charged to unrelated memcgs.Convert the shrink list to a memcg-aware
list_lru. Queue each inode on thelist_lrusublist matching the memcg and node of the current tail huge folio, so non-root memcg reclaim only walks candidates charged to the reclaiming memcg. Global reclaim, root memcg reclaim and shmem quota reclaim keep global semantics.The
list_lrustill tracks inodes while the actual split target is the current tail huge folio, so validate the folio memcg/node during scan. If the folio no longer matches the reclaim context or splitting cannot proceed, requeue the inode according to the current tail folio; if the inode is no longer shrinkable, drop the scan entry.This can be tested with the shrinker debugfs interface by allocating 32 tmpfs tail THPs in each of two memcgs, then scanning the sb-tmpfs shrinker with memcg A's cgroup id:
before A scan after A scan
base A=64M, B=64M A=0, B=0
patched A=64M, B=64M A=0, B=64M
Quoted 6 lines
Quoted 2 lines
Signed-off-by Qi Zheng <[email protected]>---
Thanks for your patch. This work has been on my TODO list for a while :)
Overall it looks good. I'll take a closer look and run some tests.
Thanks!
Quoted 22 lines
Quoted 22 lines
This patch is based on next-20260701 because it doesn't include this patch:
https://lore.kernel.org/all/20260609123047.1948242-1- [email protected]/
Later on, Usama will consider moving this restriction down into the fs callback.
include/linux/shmem_fs.h| 14 +-
mm/shmem.c | 379 ++++++++++++++++++++++++++++++---------2 files changed, 310 insertions(+), 83 deletions(-)include/linux/shmem_fs.h +1
-0@@ -11,6 +11,7 @@ #include <linux/fs_parser.h> #include <linux/userfaultfd_k.h> #include <linux/bits.h> +#include <linux/list_lru.h> struct swap_iocb;@@ -56,6 +57,13 @@ struct shmem_inode_info {
Quoted 8 lines
include/linux/shmem_fs.h struct shmem_inode_info { · line 62
Quoted 6 lines
include/linux/shmem_fs.h +3
-0struct dquot __rcu *i_dquot[MAXQUOTAS]; #endif struct inode vfs_inode; + +#ifdef CONFIG_TRANSPARENT_HUGEPAGE + struct list_head shrinklist_scan;Why introduce another
list_headmember? Why can't we reuse 'shrinklist' in this structure? In other words, if we isolate this inode fromlist_lru, then 'inode_info->shrinklist' can be reused to link it into the temporary list, right?
After list_lru isolation, the shrinker drops the list_lru lock and processes the inode on a local scan list. During that time, shmem faults can make the inode's current tail huge folio change, and shmem_unused_huge_add() may need to queue the same inode again on the active list_lru, possibly under a different memcg/node sublist.
If we reused info->shrinklist for the temporary scan list, then while the inode is being scanned:
1. info->shrinklist would be non-empty because it is on the local scan list, so shmem_unused_huge_add() could not tell that the inode is no longer on the active list_lru.
2. Re-adding it to list_lru with the same list_head would corrupt the local scan list.
3. Skipping the add would lose the new current tail huge folio if the scan later drops the old scan entry instead of requeueing it.
So isolation removes the inode from the active list_lru and leaves info->shrinklist empty. A concurrent fault can safely add the inode back to the active list_lru with the current folio's memcg/node, while the shrinker still owns the old scan entry through shrinklist_scan. The scan loop then detects that re-add and drops the stale scan entry, leaving the active entry for a later scan.
Quoted 6 lines
include/linux/shmem_fs.h struct shmem_inode_info { · line 65
Quoted 4 lines
include/linux/shmem_fs.h +3
-0+ struct mem_cgroup *shrinklist_memcg; + int shrinklist_nid; + bool shrinklist_isolated;Do we need an extra bool to indicate whether it's isolated? Can't we just use '
list_empty(inode_info->shrinklist)'? I may have missed some details.
list_empty(info->shrinklist) cannot be used for this. That list node tracks active list_lru membership only. But you are right that the explicit bool is not necessary, we can use list_empty(&info->shrinklist_scan) instead.
Thanks,
Qi
Baolin Wang
reviewerQuoted 1 line · Qi Zheng
Hi Baolin,
Sorry for late reply.
Quoted 77 lines · Baolin Wang
Quoted 30 lines
Hi Qi,
Quoted 27 lines · Qi Zheng
From: Qi Zheng <[email protected]>
The shmem unused huge shrinker keeps a per-superblock list of inodes whose tail huge folio extends beyond
i_size. Since that list is not memcg aware, reclaim triggered by one memcg can scan inodes from the whole superblock and split shmem huge folios charged to unrelated memcgs.Convert the shrink list to a memcg-aware
list_lru. Queue each inode on thelist_lrusublist matching the memcg and node of the current tail huge folio, so non-root memcg reclaim only walks candidates charged to the reclaiming memcg. Global reclaim, root memcg reclaim and shmem quota reclaim keep global semantics.The
list_lrustill tracks inodes while the actual split target is the current tail huge folio, so validate the folio memcg/node during scan. If the folio no longer matches the reclaim context or splitting cannot proceed, requeue the inode according to the current tail folio; if the inode is no longer shrinkable, drop the scan entry.This can be tested with the shrinker debugfs interface by allocating 32 tmpfs tail THPs in each of two memcgs, then scanning the sb-tmpfs shrinker with memcg A's cgroup id:
before A scan after A scan
base A=64M, B=64M A=0, B=0
patched A=64M, B=64M A=0, B=64MQuoted 6 lines
Quoted 2 lines
Signed-off-by Qi Zheng <[email protected]>---
Thanks for your patch. This work has been on my TODO list for a while :)
Overall it looks good. I'll take a closer look and run some tests.
Thanks!
Quoted 22 lines
Quoted 22 lines
This patch is based on next-20260701 because it doesn't include this patch:
https://lore.kernel.org/all/20260609123047.1948242-1- [email protected]/
Later on, Usama will consider moving this restriction down into the fs callback.
include/linux/shmem_fs.h| 14 +-
mm/shmem.c | 379 ++++++++++++++++++++++++++++++---------2 files changed, 310 insertions(+), 83 deletions(-)include/linux/shmem_fs.h +1
-0@@ -11,6 +11,7 @@ #include <linux/fs_parser.h> #include <linux/userfaultfd_k.h> #include <linux/bits.h> +#include <linux/list_lru.h> struct swap_iocb;@@ -56,6 +57,13 @@ struct shmem_inode_info {Quoted 8 lines
include/linux/shmem_fs.h struct shmem_inode_info { · line 62
Quoted 6 lines
include/linux/shmem_fs.h +3
-0struct dquot __rcu *i_dquot[MAXQUOTAS]; #endif struct inode vfs_inode; + +#ifdef CONFIG_TRANSPARENT_HUGEPAGE + struct list_head shrinklist_scan;Why introduce another
list_headmember? Why can't we reuse 'shrinklist' in this structure? In other words, if we isolate this inode fromlist_lru, then 'inode_info->shrinklist' can be reused to link it into the temporary list, right?After
list_lruisolation, the shrinker drops thelist_lrulock and processes the inode on a local scan list. During that time, shmem faults can make the inode's current tail huge folio change, andshmem_unused_huge_add()may need to queue the same inode again on the activelist_lru, possibly under a different memcg/node sublist.
Quoted 7 lines
If we reused
info->shrinklistfor the temporary scan list, then while the inode is being scanned:1.
info->shrinklistwould be non-empty because it is on the local scan list, soshmem_unused_huge_add()could not tell that the inode is no longer on the activelist_lru.
2. Re-adding it tolist_lruwith the samelist_headwould corrupt the local scan list.
Right. Just some preliminary thoughts:
If the current inode needs to be re-added to the list_lru, it means the inode's i_size has changed. If 'info->shrinklist' is on the temporary list at this point, we can follow the same logic as the original 'info->shrinklist' handling and simply skip the re-adding to list_lru. Instead, we can let shmem_unused_huge_shrink() get the end folio via filemap_get_entry(i_size), and then check the memcg and nid — if they differ from the 'sc', we can re-add the inode back to the list_lru in shmem_unused_huge_shrink().
Of course, there may be other races that need to be considered as well.
In any case, we should try to avoid adding too many members to the 'shmem_inode_info' struct, since when there are many tmpfs files this could introduce noticeable memory overhead.
Quoted 25 lines
3. Skipping the add would lose the new current tail huge folio if the scan later drops the old scan entry instead of requeueing it.
So isolation removes the inode from the active
list_lruand leavesinfo->shrinklistempty. A concurrent fault can safely add the inode back to the activelist_lruwith the current folio's memcg/node, while the shrinker still owns the old scan entry throughshrinklist_scan. The scan loop then detects that re-add and drops the stale scan entry, leaving the active entry for a later scan.Quoted 6 lines
include/linux/shmem_fs.h struct shmem_inode_info { · line 65
Quoted 4 lines
include/linux/shmem_fs.h +3
-0+ struct mem_cgroup *shrinklist_memcg; + int shrinklist_nid; + bool shrinklist_isolated;Do we need an extra bool to indicate whether it's isolated? Can't we just use '
list_empty(inode_info->shrinklist)'? I may have missed some details.
list_empty(info->shrinklist)cannot be used for this. That list node tracks activelist_lrumembership only. But you are right that the explicit bool is not necessary, we can use list_empty(&info-Quoted 1 line
shrinklist_scan) instead.Thanks,
Qi
Hi Baolin,
Quoted 93 lines · Baolin Wang
Quoted 1 line · Qi Zheng
Hi Baolin,
Sorry for late reply.
Quoted 78 lines · Baolin Wang
Quoted 30 lines
Hi Qi,
Quoted 27 lines · Qi Zheng
From: Qi Zheng <[email protected]>
The shmem unused huge shrinker keeps a per-superblock list of inodes whose tail huge folio extends beyond
i_size. Since that list is not memcg aware, reclaim triggered by one memcg can scan inodes from the whole superblock and split shmem huge folios charged to unrelated memcgs.Convert the shrink list to a memcg-aware
list_lru. Queue each inode on thelist_lrusublist matching the memcg and node of the current tail huge folio, so non-root memcg reclaim only walks candidates charged to the reclaiming memcg. Global reclaim, root memcg reclaim and shmem quota reclaim keep global semantics.The
list_lrustill tracks inodes while the actual split target is the current tail huge folio, so validate the folio memcg/node during scan. If the folio no longer matches the reclaim context or splitting cannot proceed, requeue the inode according to the current tail folio; if the inode is no longer shrinkable, drop the scan entry.This can be tested with the shrinker debugfs interface by allocating 32 tmpfs tail THPs in each of two memcgs, then scanning the sb-tmpfs shrinker with memcg A's cgroup id:
before A scan after A scan
base A=64M, B=64M A=0, B=0
patched A=64M, B=64M A=0, B=64MQuoted 6 lines
Quoted 2 lines
Signed-off-by Qi Zheng <[email protected]>---
Thanks for your patch. This work has been on my TODO list for a while :)
Overall it looks good. I'll take a closer look and run some tests.
Thanks!
Quoted 23 lines
Quoted 23 lines
This patch is based on next-20260701 because it doesn't include this patch:
https://lore.kernel.org/all/20260609123047.1948242-1- [email protected]/
Later on, Usama will consider moving this restriction down into the fs callback.
include/linux/shmem_fs.h| 14 +-
mm/shmem.c | 379 +++++++++++++++++++++++++++++
+---------2 files changed, 310 insertions(+), 83 deletions(-)include/linux/shmem_fs.h +1
-0@@ -11,6 +11,7 @@ #include <linux/fs_parser.h> #include <linux/userfaultfd_k.h> #include <linux/bits.h> +#include <linux/list_lru.h> struct swap_iocb;@@ -56,6 +57,13 @@ struct shmem_inode_info {Quoted 8 lines
include/linux/shmem_fs.h struct shmem_inode_info { · line 62
Quoted 6 lines
include/linux/shmem_fs.h +3
-0struct dquot __rcu *i_dquot[MAXQUOTAS]; #endif struct inode vfs_inode; + +#ifdef CONFIG_TRANSPARENT_HUGEPAGE + struct list_head shrinklist_scan;Why introduce another
list_headmember? Why can't we reuse 'shrinklist' in this structure? In other words, if we isolate this inode fromlist_lru, then 'inode_info->shrinklist' can be reused to link it into the temporary list, right?After
list_lruisolation, the shrinker drops thelist_lrulock and processes the inode on a local scan list. During that time, shmem faults can make the inode's current tail huge folio change, andshmem_unused_huge_add()may need to queue the same inode again on the activelist_lru, possibly under a different memcg/node sublist.Quoted 7 lines
If we reused
info->shrinklistfor the temporary scan list, then while the inode is being scanned:1.
info->shrinklistwould be non-empty because it is on the local scan list, soshmem_unused_huge_add()could not tell that the inode is no longer on the activelist_lru.
2. Re-adding it tolist_lruwith the samelist_headwould corrupt the local scan list.
Quoted 5 lines
Right. Just some preliminary thoughts:
If the current inode needs to be re-added to the
list_lru, it means the inode'si_sizehas changed. If 'info->shrinklist' is on the temporary list at this point, we can follow the same logic as the original 'info-Quoted 1 line
shrinklist' handling and simply skip the re-adding to
list_lru.Instead, we can let
shmem_unused_huge_shrink()get the end folio viafilemap_get_entry(i_size), and then check the memcg and nid — if they differ from the 'sc', we can re-add the inode back to thelist_lruinshmem_unused_huge_shrink().
That also sounds workable.
Quoted 2 lines
Of course, there may be other races that need to be considered as well.
I will look into it.
Quoted 2 lines
In any case, we should try to avoid adding too many members to the '
shmem_inode_info' struct, since when there are many tmpfs files this could introduce noticeable memory overhead.
Got it.
Thanks,
Qi
Quoted 26 lines
Quoted 25 lines
3. Skipping the add would lose the new current tail huge folio if the scan later drops the old scan entry instead of requeueing it.
So isolation removes the inode from the active
list_lruand leavesinfo->shrinklistempty. A concurrent fault can safely add the inode back to the activelist_lruwith the current folio's memcg/node, while the shrinker still owns the old scan entry throughshrinklist_scan. The scan loop then detects that re-add and drops the stale scan entry, leaving the active entry for a later scan.Quoted 6 lines
include/linux/shmem_fs.h struct shmem_inode_info { · line 65
Quoted 4 lines
include/linux/shmem_fs.h +3
-0+ struct mem_cgroup *shrinklist_memcg; + int shrinklist_nid; + bool shrinklist_isolated;Do we need an extra bool to indicate whether it's isolated? Can't we just use '
list_empty(inode_info->shrinklist)'? I may have missed some details.
list_empty(info->shrinklist)cannot be used for this. That list node tracks activelist_lrumembership only. But you are right that the explicit bool is not necessary, we can uselist_empty(&info- >shrinklist_scan)instead.Thanks,
Qi