Re: [RFC PATCH 0/3] mm, memcg: isolate deprecated v1 state from struct mem_cgroup

[Roman Gushchin <roman.gushchin@linux.dev>]

I believe we want to delete this code sooner than later, why would we rewrite it before that?

> On Oct 29, 2026, at 10:40 PM, Tao Cui <cui.tao@linux.dev> wrote:
> 
> Hi Johannes, Shakeel, Michal,
> 
>> 在 2026/9/19 00:24, Johannes Weiner 写道:
>>> On Fri, Sep 18, 2026 at 11:41:09AM +0200, Michal Hocko wrote:
>>> On Thu 17-09-26 13:26:40, Shakeel Butt wrote:
>>>> On Wed, Sep 16, 2026 at 08:57:34PM +0800, Tao Cui wrote:
>>>>> From: Tao Cui <cuitao@kylinos.cn>
>>>>> 
>>>>> The legacy cgroup v1 memory controller has already been moved out of
>>>>> the shared implementation at the file level (mm/memcontrol-v1.c) and at
>>>>> the Kconfig level (CONFIG_MEMCG_V1, default n since 6.11).  Its
>>>>> per-cgroup state, however, still sits as individual members inside
>>>>> struct mem_cgroup, guarded by #ifdefs.
>>>>> 
>>>>> This series isolates the deprecated implementation from the shared hot
>>>>> structure: all v1-only members are grouped into a dedicated
>>>>> struct mem_cgroup_v1, and every access goes through memcg->v1.X.
>>>>> 
>>>>> With this in place the v1 implementation is self-contained: its
>>>>> interface in mm/memcontrol-v1.c, its state in struct mem_cgroup_v1, and
>>>>> its eventual removal becomes a localized deletion of this struct
>>>>> together with mm/memcontrol-v1.c, instead of unwinding
>>>>> ifdef-scattered members across the shared header.
>>>> 
>>>> Sorry I don't see any benefit of this code churn. The code is already behind
>>>> config. What exactly this code churn is giving us?
>>> 
>>> The only arguable upside is that this would make it ever so slightly
>>> easier to track v1 specific stuff (once that s@v1@memcg1@ or similar).
>> 
>> I had the same thought. It's kiiind of nice to see which members are
>> completely specific to memcg1.
>> 
>>> I am not convinced this is sufficient to justify the churn either.
>> 
>> IMO it helps that it's mostly in memcontrol-v1.c where the chances of
>> interfering with other work are pretty slim.
>> 
>> The patch 1 + patch 2 split is a bit bothersome: indenting everything
>> into a union, only to then move it out in the next patch.
>> 
>> Tao, if you folded these two patches to make it one clean extraction
>> and changed over the sites at the same time, I would probably lean
>> supportive on this.
> 
> Sorry for the late reply.
> 
> Thanks for the feedback.  Looking back, I think Johannes' point about
> the patch split is fair.  I split the first two patches to keep the
> initial step semantically neutral, but it also made the series look
> like churn for the sake of churn.
> 
> I'll fold the first two patches into a single extraction in the next
> revision.  That should present the change more directly and hopefully
> make it easier to judge whether the resulting boundary is worthwhile.
> 
> Thanks,
> Tao
> 
>