Re: [PATCH 3/4 v2] cgroup: separate rstat locks for subsystems

["Michal Koutný" <mkoutny@suse.com>]

[-- Attachment #1: Type: text/plain, Size: 1502 bytes --]

On Mon, Mar 03, 2025 at 06:29:53PM +0000, Yosry Ahmed <yosry.ahmed@linux.dev> wrote:
> I thought about this, but this would have unnecessary memory overhead as
> we only need one lock per-subsystem. So having a lock in every single
> css is wasteful.
> 
> Maybe we can put the lock in struct cgroup_subsys? Then we can still
> initialize them in cgroup_init_subsys().

Ah, yes, muscle memory, of course I had struct cgroup_subsys\> in mind.

> I think it will be confusing to have cgroup_rstat_boot() only initialize
> some of the locks.
> 
> I think if we initialize the subsys locks in cgroup_init_subsys(), then
> we should open code initializing the base locks in cgroup_init(), and
> remove cgroup_rstat_boot().

Can this work?

DEFINE_PER_CPU(raw_spinlock_t, cgroup_rstat_base_cpu_lock) =
	__RAW_SPIN_LOCK_INITIALIZER(cgroup_rstat_base_cpu_lock);

(I see other places in kernel that assign into the per-cpu definition
but I have no idea whether that does expands and links to what's
expected. Neglecting the fact that the lock initializer is apparently
not for external use.)

> Alternatively, we can make cgroup_rstat_boot() take in a subsys and
> initialize its lock. If we pass NULL, then it initialize the base locks.
> In this case we can call cgroup_rstat_boot() for each subsystem that has
> an rstat callback in cgroup_init() (or cgroup_init_subsys()), and then
> once for the base locks.
> 
> WDYT?

Such calls from cgroup_init_subsys() are fine too.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]