From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (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 65BBA1FECCD for ; Wed, 15 Jul 2026 04:32:24 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784089945; cv=none; b=UwzvWx0yTK4F5bwnHcZewH/Bi7ifZBEgWhVsE6XncV+ec01OdsFfX7McempgBHfP9PMhgPH9hzDA0UeXhTYL0KQiIId1xMxfW/ExGBlix82exBGBwHrfCIHeB5+EOT8A+n2PClZYoNqVu4duFNBliZzmYI1CFc+LnQ5T/8p5ar8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784089945; c=relaxed/simple; bh=VImRatbGztLDQY194R/PoY5HKfLiHYjv3dAs+Bvknls=; h=Date:From:To:Cc:Subject:Message-Id:In-Reply-To:References: Mime-Version:Content-Type; b=bGx2LQ62ALxJ/ivxc+ys1h0pKG6nTq2jb2gJ608os84PFGIz2tFXky8goOA/JZbHI0gorG3qqiy0pT2x7b/IYOh/2kCorzaeXCuZVxUTiSCvYyRWW9Et9Ua979/UsaJ59nfl4H0LyozxYa9HE0wkW26HgosGor0LMlGjtXWdRsw= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux-foundation.org header.i=@linux-foundation.org header.b=DazQEg7h; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux-foundation.org header.i=@linux-foundation.org header.b="DazQEg7h" Received: by smtp.kernel.org (Postfix) with ESMTPSA id A55B61F000E9; Wed, 15 Jul 2026 04:32:23 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux-foundation.org; s=korg; t=1784089943; bh=gJb00IhM4AP9SQyu/uM8pi/lNywYn049J8+hhjcsJ20=; h=Date:From:To:Cc:Subject:In-Reply-To:References; b=DazQEg7h5SLtFuAzos91uz6fXTBxBU4y/9fMfNWCFSv1Hy3rHXw2c/pPjuVoWqT5I gnriYzJYQS4Ln0YWiMG5BrtP1O/vjwlH+ZYabPJuSp2pq9poh7onsgM3Tg9vkVyfQ1 xyd6/GYCnLXktkQdxKaxAFhLqtnnSGIcLNNfuuzk= Date: Tue, 14 Jul 2026 21:32:23 -0700 From: Andrew Morton To: Qi Zheng Cc: hughd@google.com, baolin.wang@linux.alibaba.com, usama.arif@linux.dev, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Qi Zheng Subject: Re: [PATCH] mm: shmem: make unused huge shrinker memcg aware Message-Id: <20260714213223.9a03d78f4aba252c512e01b9@linux-foundation.org> In-Reply-To: <20260714031918.308-1-qi.zheng@linux.dev> References: <20260714031918.308-1-qi.zheng@linux.dev> X-Mailer: Sylpheed 3.8.0beta1 (GTK+ 2.24.33; x86_64-pc-linux-gnu) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit On Tue, 14 Jul 2026 11:19:18 +0800 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 Sashiko said a thing: https://sashiko.dev/#/patchset/20260714031918.308-1-qi.zheng@linux.dev > 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. I'm not understanding. Was Usama's above patch ever included in anything? > - 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; > }; > diff --git a/mm/shmem.c b/mm/shmem.c > index b06c1ae2f50c3..a17150e6107c6 100644 > --- a/mm/shmem.c > +++ b/mm/shmem.c > @@ -724,50 +724,252 @@ static const char *shmem_format_huge(int huge) > } > #endif > > +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. > + > +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); > + } > + > + 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. > + 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, > + 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. > + 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); > + 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. > +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; > +} > > ... > Generally speaking, the code seems a bit thin on comments explaining why functions exist, why the code is doing a partucular thing, etc.