Re: [PATCH v3 next 4/5] memcg: factor out stat(event)/stat_local(event_local) reading functions

[Johannes Weiner <hannes@cmpxchg.org>]

On Fri, Jan 17, 2025 at 09:01:59AM -0800, Yosry Ahmed wrote:
> On Fri, Jan 17, 2025 at 8:56 AM Johannes Weiner <hannes@cmpxchg.org> wrote:
> >
> > On Fri, Jan 17, 2025 at 01:46:44AM +0000, Chen Ridong wrote:
> > > From: Chen Ridong <chenridong@huawei.com>
> > >
> > > The only difference between 'lruvec_page_state' and
> > > 'lruvec_page_state_local' is that they read 'state' and 'state_local',
> > > respectively. Factor out an inner functions to make the code more concise.
> > > Do the same for reading 'memcg_page_stat' and 'memcg_events'.
> > >
> > > Signed-off-by: Chen Ridong <chenridong@huawei.com>
> >
> > bool parameters make for poor readability at the callsites :(
> >
> > With the next patch moving most of the duplication to memcontrol-v1.c,
> > I think it's probably not worth refactoring this.
> 
> Arguably the duplication would now be across two different files,
> making it more difficult to notice and keep the implementations in
> sync.

Dependencies between the files is a bigger pain. E.g. try_charge()
being defined in memcontrol-v1.h makes memcontrol.c more difficult to
work with. That shared state also immediately bitrotted when charge
moving was removed and the last cgroup1 caller disappeared.

The whole point of the cgroup1 split was to simplify cgroup2 code. The
tiny amount of duplication in this case doesn't warrant further
entanglement between the codebases.