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

[Qi Zheng]

Hi Andrew,

On 8/4/26 5:24 AM, Andrew Morton wrote:
> On Mon,  3 Aug 2026 16:46:32 +0800 Qi Zheng <qi.zheng@xxxxxxxxx> wrote:
>
> >
>
> What David said.
>
> Also, AI review might have found a couple of things:
> 	https://sashiko.dev/#/patchset/cover.1785746588.git.zhengqi.arch@xxxxxxxxxxxxx

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