On Mon, Mar 03, 2025 at 06:29:53PM +0000, Yosry Ahmed <yosry.ahmed@xxxxxxxxx> 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:
signature.asc
Description: PGP signature