Re: [PATCH 1/4 v2] cgroup: move cgroup_rstat from cgroup to cgroup_subsys_state

[Yosry Ahmed <yosry.ahmed@linux.dev>]

On Mon, Mar 03, 2025 at 10:18:33AM -0800, Shakeel Butt wrote:
> On Sat, Mar 01, 2025 at 01:25:03AM +0000, Yosry Ahmed wrote:
> > On Fri, Feb 28, 2025 at 05:06:23PM -0800, JP Kobryn wrote:
> [...]
> > > > 
> > > > We should call bpf_rstat_flush() only if (!pos->ss) as well, right?
> > > > Otherwise we will call BPF rstat flush whenever any subsystem is
> > > > flushed.
> > > > 
> > > > I guess it's because BPF can now pass any subsystem to
> > > > cgroup_rstat_flush(), and we don't keep track. I think it would be
> > > > better if we do not allow BPF programs to select a css and always make
> > > > them flush the self css.
> > > > 
> > > > We can perhaps introduce a bpf_cgroup_rstat_flush() wrapper that takes
> > > > in a cgroup and passes cgroup->self internally to cgroup_rstat_flush().
> > > 
> > > I'm fine with this if others are in agreement. A similar concept was
> > > done in v1.
> > 
> > Let's wait for Shakeel to chime in here since he suggested removing this
> > hook, but I am not sure if he intended to actually do it or not. Better
> > not to waste effort if this will be gone soon anyway.
> > 
> 
> Yes, let's remove this unused hook. JP, can you please followup after
> this series with the removal/deprecation of this?

In this case I think it's fine for the purpose of this series to keep
bpf_rstat_flush() called if any css is being flushed to keep things
simple.

> One thing we might want to careful about is if in future someone to
> add this functionality again, that would not be a clean revert but
> what Yosry is asking for.

Not sure what you mean here, I am not asking for anything :P