Re: [PATCH 4/4 v2] cgroup: separate rstat list pointers from base stats

Shakeel Butt
2025-02-27T23:01:34Z

On Thu, Feb 27, 2025 at 01:55:43PM -0800, inwardvessel wrote:
> From: JP Kobryn <inwardvessel@gmail.com>
>
> A majority of the cgroup_rstat_cpu struct size is made up of the base
> stat entities. Since only the "self" subsystem state makes use of these,
> move them into a struct of their own. This allows for a new compact
> cgroup_rstat_cpu struct that the formal subsystems can make use of.
> Where applicable, decide on whether to allocate the compact or full
> struct including the base stats.
>
> Signed-off-by: JP Kobryn <inwardvessel@gmail.com>

One nit below otherwise:

Reviewed-by: Shakeel Butt <shakeel.butt@linux.dev>

[...]
> @@ -438,17 +444,31 @@ int cgroup_rstat_init(struct cgroup_subsys_state *css)
>
> /* the root cgrp's self css has rstat_cpu preallocated */
> if (!css->rstat_cpu) {

Early return here for root to eliminate one indent in this function.

> - css->rstat_cpu = alloc_percpu(struct cgroup_rstat_cpu);
> - if (!css->rstat_cpu)
> - return -ENOMEM;
> - }
> + if (is_base_css(css)) {
> + css->rstat_base_cpu = alloc_percpu(struct cgroup_rstat_base_cpu);
> + if (!css->rstat_base_cpu)
> + return -ENOMEM;
>
> - /* ->updated_children list is self terminated */
> - for_each_possible_cpu(cpu) {
> - struct cgroup_rstat_cpu *rstatc = cgroup_rstat_cpu(css, cpu);
> + for_each_possible_cpu(cpu) {
> + struct cgroup_rstat_base_cpu *rstatc;
> +
> + rstatc = cgroup_rstat_base_cpu(css, cpu);
> + rstatc->self.updated_children = css;
> + u64_stats_init(&rstatc->bsync);
> + }
> + } else {
> + css->rstat_cpu = alloc_percpu(struct cgroup_rstat_cpu);
> + if (!css->rstat_cpu)
> + return -ENOMEM;
> +
> + for_each_possible_cpu(cpu) {
> + struct cgroup_rstat_cpu *rstatc;
> +
> + rstatc = cgroup_rstat_cpu(css, cpu);
> + rstatc->updated_children = css;
> + }
> + }
>
> - rstatc->updated_children = css;
> - u64_stats_init(&rstatc->bsync);
> }