lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 6 Jun 2022 12:41:06 -0700 From: Yosry Ahmed <yosryahmed@...gle.com> To: Michal Koutný <mkoutny@...e.com> Cc: Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, Andrii Nakryiko <andrii@...nel.org>, Martin KaFai Lau <kafai@...com>, Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>, John Fastabend <john.fastabend@...il.com>, KP Singh <kpsingh@...nel.org>, Hao Luo <haoluo@...gle.com>, Tejun Heo <tj@...nel.org>, Zefan Li <lizefan.x@...edance.com>, Johannes Weiner <hannes@...xchg.org>, Shuah Khan <shuah@...nel.org>, Roman Gushchin <roman.gushchin@...ux.dev>, Michal Hocko <mhocko@...nel.org>, Stanislav Fomichev <sdf@...gle.com>, David Rientjes <rientjes@...gle.com>, Greg Thelen <gthelen@...gle.com>, Shakeel Butt <shakeelb@...gle.com>, Linux Kernel Mailing List <linux-kernel@...r.kernel.org>, Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>, Cgroups <cgroups@...r.kernel.org> Subject: Re: [PATCH bpf-next v1 5/5] bpf: add a selftest for cgroup hierarchical stats collection On Mon, Jun 6, 2022 at 5:32 AM Michal Koutný <mkoutny@...e.com> wrote: > > On Fri, Jun 03, 2022 at 12:52:27PM -0700, Yosry Ahmed <yosryahmed@...gle.com> wrote: > > Good catch. I get confused between cgrp->subsys and > > task->cgroups->subsys sometimes because of different fallback > > behavior. IIUC cgrp->subsys should have NULL if the memory controller > > is not enabled (no nearest ancestor fallback), and hence I can use > > memory_subsys_enabled() that I defined just above task_memcg() to test > > for this (I have no idea why I am not already using it here). Is my > > understanding correct? > > You're correct, css_set (task->cgroups) has a css (memcg) always defined > (be it root only (or even a css from v1 hierarchy but that should not > relevant here)). A particular cgroup can have the css set to NULL. > > When I think about your stats collecting example now, task_memcg() looks > more suitable to achieve proper hierarchical counting in the end (IOW > you'd lose info from tasks who don't reside in memcg-enabled leaf). I guess it depends on how userspace reasons about this, and whether or not you want to collect stats from leaves that don't reside in a memcg-enabled leaf. I will go through all the memcg-enabled checks and make sure they make sense and are consistent, maybe add some comments to make the userspace policy here clear. > > (It's just that task_memcg won't return NULL. Unless the kernel is > compiled without memcg support completely, which makes me think how do > the config-dependent values propagate to BPF programs?) I don't know if there is a standard way to handle this, but I think you should know the configs of your kernel when you are loading a bpf program? In this particular case, if CONFIG_CGROUPS=0 then the bpf programs will not even load due to lack of hook points or kfuncs won't exist. If the CONFIG_CGROUPS=1 but CONFIG_MEMCG=0 I think everything will work normally except that task_memcg() will always return NULL so no stats will be collected, which makes sense. There will be some overhead to running bpf programs that will always do nothing, but I would argue that it's the userspace's fault here for loading bpf programs on a non-compatible kernel. > > Thanks, > Michal
Powered by blists - more mailing lists