On Tue, Oct 10, 2023 at 11:41 AM Tejun Heo <tj@xxxxxxxxxx> wrote: > > Hello, > > On Tue, Oct 10, 2023 at 10:14:26AM -0700, T.J. Mercier wrote: > > > BTW is there any fundamental reason the apps cannot use the > > > notifications via cgroup.events as recommended by Tejun? > > > > > This would require that we read both cgroup.procs and cgroup.events, > > since we'd still want to know which processes to signal. I assumed > > this would increase lock contention but there's no synchronization on > > cgroup_is_populated so it looks like not. I had already identified > > this as a workaround, but I'd prefer to depend on just one file to do > > everything. > > I don't think we can guarantee that. There's a reason why we have > [!]populated events. Maybe we can find this particular situation better but > there isn't going to be a guarantee that a cgroup is removable if its > cgroup.procs file is seen empty. > I understand that there are cases where the cgroup can become populated after a read of cgroup.procs shows nothing and before a removal is attempted. But that doesn't seem to be what's happening here. > Note that cgroup.events file is pollable. You can just poll the file and > then respond to them. I don't understand the part of having to read > cgroup.procs, which btw is an a lot more expensive operation. You said > "which processes to signal". If this is to kill all processes in the cgroup, > you can use cgroup.kill which sends signal atomically to all member tasks. > That's coming, but we still need to support kills without cgroup.kill for kernels before it was introduced. There are some non-SIGKILL use cases that code supports, so that's what I meant when I said, "which processes to signal". I guess we could separate these two paths so that one uses cgroup.kill and blocks until populated = 0, and the other just reads cgroup.procs to generate the signals and then returns. But it would be nice to have cgroup.procs just show the pids until after cgroup_exit when the condition for removal is satisfied. > It feels like the use case is just trying to do things in an unsupported way > when there's no actual benefit to doing so. Is there something preventing > you guys from doing how it's supposed to be used? > Isn't this avoiding the problem? The docs say, "A cgroup which doesn't have any children or live processes can be destroyed by removing the directory." If it has live processes they should show up in cgroup.procs. FWIW that's also the language used to describe how the populated notification works, so the two shouldn't report different things. (Assuming they could actually be read simultaneously.)
