[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aUQCfdnoLQDLoVyg@cmpxchg.org>
Date: Thu, 18 Dec 2025 08:32:45 -0500
From: Johannes Weiner <hannes@...xchg.org>
To: Qi Zheng <qi.zheng@...ux.dev>
Cc: hughd@...gle.com, mhocko@...e.com, roman.gushchin@...ux.dev,
shakeel.butt@...ux.dev, muchun.song@...ux.dev, david@...nel.org,
lorenzo.stoakes@...cle.com, ziy@...dia.com, harry.yoo@...cle.com,
imran.f.khan@...cle.com, kamalesh.babulal@...cle.com,
axelrasmussen@...gle.com, yuanchu@...gle.com, weixugc@...gle.com,
chenridong@...weicloud.com, mkoutny@...e.com,
akpm@...ux-foundation.org, hamzamahfooz@...ux.microsoft.com,
apais@...ux.microsoft.com, lance.yang@...ux.dev, linux-mm@...ck.org,
linux-kernel@...r.kernel.org, cgroups@...r.kernel.org,
Qi Zheng <zhengqi.arch@...edance.com>
Subject: Re: [PATCH v2 24/28] mm: vmscan: prepare for reparenting traditional
LRU folios
On Wed, Dec 17, 2025 at 03:27:48PM +0800, Qi Zheng wrote:
> From: Qi Zheng <zhengqi.arch@...edance.com>
>
> To reslove the dying memcg issue, we need to reparent LRU folios of child
resolve
> memcg to its parent memcg. For traditional LRU list, each lruvec of every
> memcg comprises four LRU lists. Due to the symmetry of the LRU lists, it
> is feasible to transfer the LRU lists from a memcg to its parent memcg
> during the reparenting process.
>
> This commit implements the specific function, which will be used during
> the reparenting process.
>
> Signed-off-by: Qi Zheng <zhengqi.arch@...edance.com>
> Reviewed-by: Harry Yoo <harry.yoo@...cle.com>
Overall looks sane to me. I have a few nits below, not nothing
major. With those resolved, please feel free to add
Acked-by: Johannes Weiner <hannes@...xchg.org>
> @@ -2648,6 +2648,44 @@ static bool can_age_anon_pages(struct lruvec *lruvec,
> lruvec_memcg(lruvec));
> }
>
> +#ifdef CONFIG_MEMCG
> +static void lruvec_reparent_lru(struct lruvec *src, struct lruvec *dst,
> + enum lru_list lru)
> +{
> + int zid;
> + struct mem_cgroup_per_node *mz_src, *mz_dst;
> +
> + mz_src = container_of(src, struct mem_cgroup_per_node, lruvec);
> + mz_dst = container_of(dst, struct mem_cgroup_per_node, lruvec);
> +
> + if (lru != LRU_UNEVICTABLE)
> + list_splice_tail_init(&src->lists[lru], &dst->lists[lru]);
> +
> + for (zid = 0; zid < MAX_NR_ZONES; zid++) {
> + mz_dst->lru_zone_size[zid][lru] += mz_src->lru_zone_size[zid][lru];
> + mz_src->lru_zone_size[zid][lru] = 0;
> + }
> +}
> +
> +void lru_reparent_memcg(struct mem_cgroup *src, struct mem_cgroup *dst)
I can see why you want to pass both src and dst for convenience, but
it makes the API look a lot more generic than it is. It can only
safely move LRUs from a cgroup to its parent.
As such, I'd slightly prefer only passing one pointer and doing the
parent lookup internally. It's dealer's choice.
However, if you'd like to keep both pointers for a centralized lookup,
can you please rename the parameters @child and @parent, and add
VM_WARN_ON(parent != parent_mem_cgroup(child));
Also please add a comment explaining the expected caller locking.
Lastly, vmscan.c is the reclaim policy. Mechanical LRU shuffling like
this is better placed in mm/swap.c.
Powered by blists - more mailing lists