From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out30-119.freemail.mail.aliyun.com (out30-119.freemail.mail.aliyun.com [115.124.30.119]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5DC752165EA for ; Fri, 17 Jul 2026 12:50:47 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=115.124.30.119 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784292651; cv=none; b=RiYWZiuGvlxv3rqLWkJIKsWp0B95U6Nu2IBV+3ObZw1aGu12Yz9/pX+xD0Ukboe0tgA4HwKRHoGnR6rdHwWHtr7idBvMDJk9khG6kJgvFU+nPN6OQTYVnTYZx5Te45OS2RO7XEt0UoldP3A3vF3VKSYOU1ALXjjKbM0s4mehNcc= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784292651; c=relaxed/simple; bh=XCX3M+4IcjC0wk/L9OeplSNiLHj7tYMpr/Ei/bSd5Mc=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=U6HvF86S96sSdgDiETuhKvBaOKPiweWhXyvzjjyNZZjYsE3ds3IW2XGErMRJGzS1o2hiaAMdyRvQUS+LPzTW4WY1RBj7Hzu1FIzgTQx2LHcd2IC880efQaPyQ+Jr07vS5Bp+S9/HB3mVs2F4LyRufYVPw0cWqM123//Meg9pypY= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.alibaba.com; spf=pass smtp.mailfrom=linux.alibaba.com; dkim=pass (1024-bit key) header.d=linux.alibaba.com header.i=@linux.alibaba.com header.b=dDfOtNWa; arc=none smtp.client-ip=115.124.30.119 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.alibaba.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.alibaba.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.alibaba.com header.i=@linux.alibaba.com header.b="dDfOtNWa" DKIM-Signature:v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.alibaba.com; s=default; t=1784292643; h=Message-ID:Date:MIME-Version:Subject:To:From:Content-Type; bh=T7c+aKoP+w0A9k4vUfdunXUVWhKvAbj5XvTxVOhMlsM=; b=dDfOtNWayC7nXB+PaaIFa0LKXhGUo9+DfVGFYR4g/18oNDXqv3sQISo6fv416R3azByBp9tw+6wYRR+rTXJOOVgiRJ9k/80fl/dP83SoaVbvg7UsHgMouX3othtJDDKoG0xsQzY4kd6K8QhLapY3QvWgmSvAnVYXiLey/FPG2z8= X-Alimail-AntiSpam:AC=PASS;BC=-1|-1;BR=01201311R161e4;CH=green;DM=||false|;DS=||;FP=0|-1|-1|-1|0|-1|-1|-1;HT=maildocker-contentspam033037026112;MF=baolin.wang@linux.alibaba.com;NM=1;PH=DS;RN=7;SR=0;TI=SMTPD_---0X7HJ7HO_1784292642; Received: from 30.120.38.205(mailfrom:baolin.wang@linux.alibaba.com fp:SMTPD_---0X7HJ7HO_1784292642 cluster:ay36) by smtp.aliyun-inc.com; Fri, 17 Jul 2026 20:50:42 +0800 Message-ID: Date: Fri, 17 Jul 2026 20:50:42 +0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] mm: shmem: make unused huge shrinker memcg aware To: Qi Zheng , hughd@google.com, akpm@linux-foundation.org, usama.arif@linux.dev Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, Qi Zheng References: <20260714031918.308-1-qi.zheng@linux.dev> <7624107f-6a2d-4959-b2ee-aa362a8972eb@linux.alibaba.com> From: Baolin Wang In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 7/15/26 5:55 PM, Qi Zheng wrote: > Hi Baolin, Sorry for late reply. > 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 >>> >>> 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 >>> --- >> >> 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 >>>   #include >>>   #include >>> +#include >>>   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. 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. > 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 > >