Re: [PATCH v2 06/23] mm: introduce BPF struct ops for OOM handling

Alexei Starovoitov
2025-10-28T22:08:02Z

On Tue, Oct 28, 2025 at 11:42 AM Roman Gushchin
<roman.gushchin@linux.dev> wrote:
>
> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
>
> > On Mon, Oct 27, 2025 at 4:18 PM Roman Gushchin <roman.gushchin@linux.dev> wrote:
> >>
> >> +bool bpf_handle_oom(struct oom_control *oc)
> >> +{
> >> + struct bpf_oom_ops *bpf_oom_ops = NULL;
> >> + struct mem_cgroup __maybe_unused *memcg;
> >> + int idx, ret = 0;
> >> +
> >> + /* All bpf_oom_ops structures are protected using bpf_oom_srcu */
> >> + idx = srcu_read_lock(&bpf_oom_srcu);
> >> +
> >> +#ifdef CONFIG_MEMCG
> >> + /* Find the nearest bpf_oom_ops traversing the cgroup tree upwards */
> >> + for (memcg = oc->memcg; memcg; memcg = parent_mem_cgroup(memcg)) {
> >> + bpf_oom_ops = READ_ONCE(memcg->bpf_oom);
> >> + if (!bpf_oom_ops)
> >> + continue;
> >> +
> >> + /* Call BPF OOM handler */
> >> + ret = bpf_ops_handle_oom(bpf_oom_ops, memcg, oc);
> >> + if (ret && oc->bpf_memory_freed)
> >> + goto exit;
> >> + }
> >> +#endif /* CONFIG_MEMCG */
> >> +
> >> + /*
> >> + * System-wide OOM or per-memcg BPF OOM handler wasn't successful?
> >> + * Try system_bpf_oom.
> >> + */
> >> + bpf_oom_ops = READ_ONCE(system_bpf_oom);
> >> + if (!bpf_oom_ops)
> >> + goto exit;
> >> +
> >> + /* Call BPF OOM handler */
> >> + ret = bpf_ops_handle_oom(bpf_oom_ops, NULL, oc);
> >> +exit:
> >> + srcu_read_unlock(&bpf_oom_srcu, idx);
> >> + return ret && oc->bpf_memory_freed;
> >> +}
> >
> > ...
> >
> >> +static int bpf_oom_ops_reg(void *kdata, struct bpf_link *link)
> >> +{
> >> + struct bpf_struct_ops_link *ops_link = container_of(link, struct bpf_struct_ops_link, link);
> >> + struct bpf_oom_ops **bpf_oom_ops_ptr = NULL;
> >> + struct bpf_oom_ops *bpf_oom_ops = kdata;
> >> + struct mem_cgroup *memcg = NULL;
> >> + int err = 0;
> >> +
> >> + if (IS_ENABLED(CONFIG_MEMCG) && ops_link->cgroup_id) {
> >> + /* Attach to a memory cgroup? */
> >> + memcg = mem_cgroup_get_from_ino(ops_link->cgroup_id);
> >> + if (IS_ERR_OR_NULL(memcg))
> >> + return PTR_ERR(memcg);
> >> + bpf_oom_ops_ptr = bpf_oom_memcg_ops_ptr(memcg);
> >> + } else {
> >> + /* System-wide OOM handler */
> >> + bpf_oom_ops_ptr = &system_bpf_oom;
> >> + }
> >
> > I don't like the fallback and special case of cgroup_id == 0.
> > imo it would be cleaner to require CONFIG_MEMCG for this feature
> > and only allow attach to a cgroup.
> > There is always a root cgroup that can be attached to and that
> > handler will be acting as "system wide" oom handler.
>
> I thought about it, but then it can't be used on !CONFIG_MEMCG
> configurations and also before cgroupfs is mounted, root cgroup
> is created etc.

before that bpf isn't viable either, and oom is certainly not an issue.

> This is why system-wide things are often handled in a
> special way, e.g. in by PSI (grep system_group_pcpu).
>
> I think supporting !CONFIG_MEMCG configurations might be useful for
> some very stripped down VM's, for example.

I thought I wouldn't need to convince the guy who converted bpf maps
to memcg and it made it pretty much mandatory for the bpf subsystem :)
I think the following is long overdue:
diff --git a/kernel/bpf/Kconfig b/kernel/bpf/Kconfig
index eb3de35734f0..af60be6d3d41 100644
--- a/kernel/bpf/Kconfig
+++ b/kernel/bpf/Kconfig
@@ -34,6 +34,7 @@ config BPF_SYSCALL
select NET_SOCK_MSG if NET
select NET_XGRESS if NET
select PAGE_POOL if NET
+ depends on MEMCG
default n

With this we can cleanup a ton of code.
Let's not add more hacks just because some weird thing
still wants !MEMCG. If they do, they will survive without bpf.