Re: [PATCH] mm: shmem: make unused huge shrinker memcg aware

[Baolin Wang]

Hi Qi,

On 7/14/26 11:19 AM, Qi Zheng wrote:
> From: Qi Zheng <zhengqi.arch@xxxxxxxxxxxxx>
>
> 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
>
> Signed-off-by: Qi Zheng <zhengqi.arch@xxxxxxxxxxxxx>
> ---

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.

> This patch is based on next-20260701 because it doesn't include this patch:
>
> https://lore.kernel.org/all/20260609123047.1948242-1-usama.arif@xxxxxxxxx/
>
> 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(-)
>
> diff --git a/include/linux/shmem_fs.h b/include/linux/shmem_fs.h
> index e729b9b0e38d4..0b14de1e890a9 100644
> --- a/include/linux/shmem_fs.h
> +++ b/include/linux/shmem_fs.h
> @@ -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;

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?

> +	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.