linux-kernel - Re: [PATCH v3 next 4/5] memcg: factor out stat(event)/stat_local(event

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250117180238.GI182896@cmpxchg.org>
Date: Fri, 17 Jan 2025 13:02:38 -0500
From: Johannes Weiner <hannes@...xchg.org>
To: Yosry Ahmed <yosryahmed@...gle.com>
Cc: Chen Ridong <chenridong@...weicloud.com>, akpm@...ux-foundation.org,
	mhocko@...nel.org, roman.gushchin@...ux.dev, shakeel.butt@...ux.dev,
	muchun.song@...ux.dev, davidf@...eo.com, vbabka@...e.cz,
	mkoutny@...e.com, linux-mm@...ck.org, linux-kernel@...r.kernel.org,
	cgroups@...r.kernel.org, chenridong@...wei.com,
	wangweiyang2@...wei.com
Subject: Re: [PATCH v3 next 4/5] memcg: factor out
 stat(event)/stat_local(event_local) reading functions

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@...xchg.org> wrote:
> >
> > On Fri, Jan 17, 2025 at 01:46:44AM +0000, Chen Ridong wrote:
> > > From: Chen Ridong <chenridong@...wei.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@...wei.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.