linux-kernel - Re: [PATCH v2] cgroup/rstat: change cgroup_base

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <wbff6zths3lvd7df2lblcyzzabovxkjrbxt7x734mdt7qdr3kh@v3urgrdibben>
Date: Fri, 27 Jun 2025 10:17:51 -0700
From: Shakeel Butt <shakeel.butt@...ux.dev>
To: "Wlodarczyk, Bertrand" <bertrand.wlodarczyk@...el.com>
Cc: "tj@...nel.org" <tj@...nel.org>, 
	"hannes@...xchg.org" <hannes@...xchg.org>, "mkoutny@...e.com" <mkoutny@...e.com>, 
	"cgroups@...r.kernel.org" <cgroups@...r.kernel.org>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, 
	"inwardvessel@...il.com" <inwardvessel@...il.com>
Subject: Re: [PATCH v2] cgroup/rstat: change cgroup_base_stat to atomic

On Fri, Jun 27, 2025 at 01:15:31PM +0000, Wlodarczyk, Bertrand wrote:
> > The kernel faces scalability issues when multiple userspace programs 
> > attempt to read cgroup statistics concurrently.
> > 
> > The primary bottleneck is the css_cgroup_lock in cgroup_rstat_flush, 
> > which prevents access and updates to the statistics of the css from 
> > multiple CPUs in parallel.
> > 
> > Given that rstat operates on a per-CPU basis and only aggregates 
> > statistics in the parent cgroup, there is no compelling reason why 
> > these statistics cannot be atomic.
> > By eliminating the lock during CPU statistics access, each CPU can 
> > traverse its rstat hierarchy independently, without blocking.
> > Synchronization is achieved during parent propagation through atomic 
> > operations.
> > 
> > This change significantly enhances performance on commit
> > 8dcb0ed834a3ec03 ("memcg: cgroup: call css_rstat_updated irrespective 
> > of in_nmi()") in scenarios where multiple CPUs accessCPU rstat within 
> > a single cgroup hierarchy, yielding a performance improvement of around 40 times.
> > Notably, performance for memory and I/O rstats remains unchanged, as 
> > the lock remains in place for these usages.
> > 
> > Additionally, this patch addresses a race condition detectable in the 
> > current mainline by KCSAN in __cgroup_account_cputime, which occurs 
> > when attempting to read a single hierarchy from multiple CPUs.
> > 
> > Signed-off-by: Bertrand Wlodarczyk <bertrand.wlodarczyk@...el.com>
> 
> > This patch breaks memory controller as explained in the comments on the previous version.
> 
> Ekhm... no? I addressed the issue and v2 has lock back and surrounding the call to dependent submodules? 
> The behavior is the same as before patching.
> 

Oh you have moved the rstat lock just around pos->ss->css_rstat_flush().

Have you checked if __css_process_update_tree() is safe from concurrent
flushers for a given cpu and css?