On Thu, Sep 8, 2022 at 6:29 AM Roman Gushchin <roman.gushchin@xxxxxxxxx> wrote: > > On Wed, Sep 07, 2022 at 05:43:31AM -1000, Tejun Heo wrote: > > Hello, > > > > On Fri, Sep 02, 2022 at 02:29:50AM +0000, Yafang Shao wrote: > > ... > > > This patchset tries to resolve the above two issues by introducing a > > > selectable memcg to limit the bpf memory. Currently we only allow to > > > select its ancestor to avoid breaking the memcg hierarchy further. > > > Possible use cases of the selectable memcg as follows, > > > > As discussed in the following thread, there are clear downsides to an > > interface which requires the users to specify the cgroups directly. > > > > https://lkml.kernel.org/r/YwNold0GMOappUxc@xxxxxxxxxxxxxxx > > > > So, I don't really think this is an interface we wanna go for. I was hoping > > to hear more from memcg folks in the above thread. Maybe ping them in that > > thread and continue there? > Hi Roman, > As I said previously, I don't like it, because it's an attempt to solve a non > bpf-specific problem in a bpf-specific way. > Why do you still insist that bpf_map->memcg is not a bpf-specific issue after so many discussions? Do you charge the bpf-map's memory the same way as you charge the page caches or slabs ? No, you don't. You charge it in a bpf-specific way. > Yes, memory cgroups are not great for accounting of shared resources, it's well > known. This patchset looks like an attempt to "fix" it specifically for bpf maps > in a particular cgroup setup. Honestly, I don't think it's worth the added > complexity. Especially because a similar behaviour can be achieved simple > by placing the task which creates the map into the desired cgroup. Are you serious ? Have you ever read the cgroup doc? Which clearly describe the "No Internal Process Constraint".[1] Obviously you can't place the task in the desired cgroup, i.e. the parent memcg. [1] https://www.kernel.org/doc/Documentation/cgroup-v2.txt > Beatiful? Not. Neither is the proposed solution. > Is it really hard to admit a fault? -- Regards Yafang