On Tue, Sep 26, 2023 at 2:43 AM Tejun Heo <tj@xxxxxxxxxx> wrote: > > Hello, > > On Sun, Sep 24, 2023 at 02:32:14PM +0800, Yafang Shao wrote: > > On Sat, Sep 23, 2023 at 12:52 AM Tejun Heo <tj@xxxxxxxxxx> wrote: > > > > > > Hello, > > > > > > On Fri, Sep 22, 2023 at 11:28:38AM +0000, Yafang Shao wrote: > > > > - bpf_cgroup_id_from_task_within_controller > > > > Retrieves the cgroup ID from a task within a specific cgroup controller. > > > > - bpf_cgroup_acquire_from_id_within_controller > > > > Acquires the cgroup from a cgroup ID within a specific cgroup controller. > > > > - bpf_cgroup_ancestor_id_from_task_within_controller > > > > Retrieves the ancestor cgroup ID from a task within a specific cgroup > > > > controller. > > > > > > > > The advantage of these new BPF kfuncs is their ability to abstract away the > > > > complexities of cgroup hierarchies, irrespective of whether they involve > > > > cgroup1 or cgroup2. > > > > > > I'm afraid this is more likely to bring the unnecessary complexities of > > > cgroup1 into cgroup2. > > > > I concur with the idea that we should avoid introducing the > > complexities of cgroup1 into cgroup2. Which specific change do you > > believe might introduce these complexities into cgroup2? Is it the > > modification within task_under_cgroup_hierarchy() or > > cgroup_get_from_id()? > > The helpers you are adding only makes sense for cgroup1. e.g. > bpf_cgroup_ancestor_id_from_task_within_controller() makes no sense in > cgroup2. The ancestor ids don't change according to controllers. The only > thing you would ask in cgroup2 is the level at which a given controller is > enabled at along with the straight-forward "where am I in the hierarchy?" > questions. I really don't want to expose interfaces which assume that the > hierarchies change according to the controller in question. Makes sense. > > Also, as pointed out before, this doesn't cover cgroup1 named hierarchies > which leaves out a good potion of cgroup1 use cases. > > > In fact, we have the option to utilize > > bpf_cgroup_ancestor_id_from_task_within_controller() as a substitute > > for bpf_task_under_cgroup(), which allows us to sidestep the need for > > changes within task_under_cgroup_hierarchy() altogether. > > I don't think this is the direction we should take. If you really want, > please tie the interface directly to the hierarchies. Don't hitch hierarchy > identificdation on the controllers. e.g. Introduce cgroup1 only interface > which takes both hierarchy ID and cgroup ID to operate on them. Thanks for your suggestion. I will think about it. BTW, I can't find the hierarchy ID of systemd (/sys/fs/cgroup/systemd) in /proc/cgroups. Is this intentional as part of the design, or might it be possible that we overlooked it? In the userspace, where can we find the hierarchy ID of a named hierarchy? -- Regards Yafang
