Re: [PATCH v3 0/3] make unused huge shrinker memcg aware

Hi Andrew,

On 8/4/26 5:24 AM, Andrew Morton wrote:
> On Mon,  3 Aug 2026 16:46:32 +0800 Qi Zheng <[email protected]> wrote:
> 
>>
> 
> What David said.
> 
> Also, AI review might have found a couple of things:
> 	https://sashiko.dev/#/patchset/[email protected]

I saw it, and it sounds reasonable.

 > > +static struct mem_cgroup *
 > > +shmem_unused_huge_alloc_lru(struct shmem_sb_info *sbinfo, struct 
folio *folio,
 > > +			    gfp_t gfp)
 > > +{
 > > +	struct mem_cgroup *memcg;
 > > +	int ret;
 > > +
 > > +	memcg = get_mem_cgroup_from_folio(folio);
 > > +	if (!memcg)
 > > +		return NULL;
 > Could this leak the mem_cgroup structure if a file is truncated?
 > When shmem_unused_huge_add() calls this function, it stores the memcg
 > reference in info->shrinklist_memcg.
 > If an unprivileged user truncates the file to 0 bytes, 
shmem_truncate_range()
 > frees the folio, but it does not remove the inode from the shrinklist 
or drop
 > this memcg reference.
 > Without memory pressure to trigger the shrinker, the mem_cgroup might 
remain
 > pinned indefinitely until the file is deleted, potentially allowing 
zombie
 > memcgs to accumulate on the system.

Indeed, will also call shmem_unused_huge_del() in shmem_truncate_range()
to take the inode off the shrinklist in time.

 > [ ... ]
 > > +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;
 > Does this early return bypass the NUMA node check for global and root 
memcg
 > reclaim?
 > The comment indicates this is only intended to skip the folio memcg 
check.
 > However, returning true immediately also bypasses the sc->nid check.
 > If a huge folio is migrated to a different NUMA node, kswapd or direct
 > reclaim might incorrectly split folios on the wrong node instead of
 > requeuing the inode to the correct node's list.

Indeed, root memcg reclaim also needs to check if the nid matches.

Will move the nid check earlier in v4:

       if (!sc)
               return true;

       if (folio_nid(folio) != sc->nid)
               return false;

       if (!sc->memcg || mem_cgroup_is_root(sc->memcg))
               return true;

Thanks,
Qi