Re: [PATCH v6 30/33] mm: memcontrol: prepare for reparenting non-hierarchical stats

[Qi Zheng]

On 3/24/26 12:05 PM, Harry Yoo (Oracle) wrote:
> On Tue, Mar 24, 2026 at 10:54:48AM +0800, Qi Zheng wrote:
> > On 3/23/26 8:25 PM, Harry Yoo (Oracle) wrote:
> > > On Mon, Mar 23, 2026 at 05:47:27PM +0800, Qi Zheng wrote:
> > > > On 3/23/26 3:53 PM, Harry Yoo (Oracle) wrote:
> > > > > On Thu, Mar 05, 2026 at 07:52:48PM +0800, Qi Zheng wrote:
> > > > > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > > > > > index 23b70bd80ddc9..b0519a16f5684 100644
> > > > > > --- a/mm/memcontrol.c
> > > > > > +++ b/mm/memcontrol.c
> > > > > > @@ -473,6 +501,30 @@ unsigned long lruvec_page_state_local(struct lruvec *lruvec,
> > > > > >     	return x;
> > > > > >     }
> > > > > > +#ifdef CONFIG_MEMCG_V1
> > > > > > +static void __mod_memcg_lruvec_state(struct mem_cgroup_per_node *pn,
> > > > > > +				     enum node_stat_item idx, int val);
> > > > > > +
> > > > > > +void reparent_memcg_lruvec_state_local(struct mem_cgroup *memcg,
> > > > > > +				       struct mem_cgroup *parent, int idx)
> > > > > > +{
> > > > > > +	int nid;
> > > > > > +
> > > > > > +	for_each_node(nid) {
> > > > > > +		struct lruvec *child_lruvec = mem_cgroup_lruvec(memcg, NODE_DATA(nid));
> > > > > > +		struct lruvec *parent_lruvec = mem_cgroup_lruvec(parent, NODE_DATA(nid));
> > > > > > +		unsigned long value = lruvec_page_state_local(child_lruvec, idx);
> > > > > > +		struct mem_cgroup_per_node *child_pn, *parent_pn;
> > > > > > +
> > > > > > +		child_pn = container_of(child_lruvec, struct mem_cgroup_per_node, lruvec);
> > > > > > +		parent_pn = container_of(parent_lruvec, struct mem_cgroup_per_node, lruvec);
> > > > > > +
> > > > > > +		__mod_memcg_lruvec_state(child_pn, idx, -value);
> > > > > > +		__mod_memcg_lruvec_state(parent_pn, idx, value);
> > > > >
> > > > > We should probably change the type of `@val` from int to val to avoid
> > > > > losing non hierarchical stats during reparenting?
> > > >
> > > > The parameter and return value of memcg_state_val_in_pages() are both
> > > > of type int, so perhaps we need a cleanup patch to do this?
> > >
> > > Yes!
> > >
> > > and @val in memcg_rstat_updated() too, I think.
> >
> > Right.
> >
> > > > I will send a cleanup patchset to do this, which includes the following:
> > > >
> > > > https://lore.kernel.org/all/5e178b4e-a9e0-44dc-a18d-8c014365ee2f@xxxxxxxxx/
> > >
> > > Thanks!
> > >
> > > Should that ideally be applied before this patchset?
> >
> > This would conflict with the current patchset.
>
> Right.
>
> > The v6 has been in mm-unstable for some time
>
> Right.
>
> > so I prefer to add a cleanup patchset to
> > avoid interfering with the merge of this patchset.
>
> but it's a little bit more than a cleanup, no?
>
> I'd say without the followup, this patchset introduces a very subtle
> (likely nobody would notice) functional regression visible on memcgs
> with billions of pages.

Right.

> > Otherwise, sending v7 might be more appropriate.
>   
> I think it should be sent either as v7 or as part of v7.1-rcX.
> (Whichever way Andrew prefers)

OK, In any case I will first send out the cleanup/fix patchset for
everyone to review.

Thanks,
Qi

>