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

[Qi Zheng <qi.zheng@linux.dev>]

Hi Baolin,

On 7/15/26 5:37 PM, Baolin Wang wrote:
> Hi Qi,
> 
> On 7/14/26 11:19 AM, Qi Zheng wrote:
>> From: Qi Zheng <zhengqi.arch@bytedance.com>
>>
>> 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@bytedance.com>
>> ---
> 
> 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!

> 
>> This patch is based on next-20260701 because it doesn't include this 
>> patch:
>>
>> https://lore.kernel.org/all/20260609123047.1948242-1- 
>> usama.arif@linux.dev/
>>
>> 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?

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.

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