Re: [PATCH v4 4/5] cgroup: use separate rstat trees for each subsystem

[JP Kobryn <inwardvessel@gmail.com>]

On 4/17/25 2:26 AM, Michal Koutný wrote:
> On Wed, Apr 16, 2025 at 02:43:57PM -0700, JP Kobryn <inwardvessel@gmail.com> wrote:
>> Hmmm I checked my initial assumptions. I'm still finding that css's from
>> any subsystem regardless of rstat usage can reach this call to exit.
>> Without the guard there will be undefined behavior.
> 
> At which place is the UB? (I saw that all funnels to css_rstat_flush()
> that does the check but I may have overlooked something in the diffs.)

It would occur on access to the per-cpu rstat pointer during the tree
building in the sequence below.

css_rstat_exit(css)
	css_rstat_flush(css)
		css_rstat_updated_list(css, cpu)
			rstatc = css_rstat_cpu(css, cpu)
				per_cpu_ptr(css->rstat_cpu, cpu)

Since I'm doing the early checks in css_rstat_flush() in the next rev
though, I was thinking of this:

void css_rstat_flush(css)
{
	bool is_cgroup = css_is_cgroup(css);
	
	if (!is_cgroup && !css->ss->css_rstat_flush)
		return;

	...

	for (...) {
		if (is_cgroup)
			/* flush base stats and bpf */
		else
			/* flush via css_rstat_flush */
	}
}

Then we could remove the two conditional guards in css_release_work_fn()
and css_free_rwork_fn(). Thoughts?

Note that I was also thinking of doing the same early check in
css_rstat_updated() since it is exposed as a kfunc and someone could
pass in a non-rstat css other than cgroup::self.