[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <87cy8gty9e.fsf@linux.dev>
Date: Wed, 27 Aug 2025 11:28:29 -0700
From: Roman Gushchin <roman.gushchin@...ux.dev>
To: Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc: Martin KaFai Lau <martin.lau@...ux.dev>, Kumar Kartikeya Dwivedi
<memxor@...il.com>, linux-mm <linux-mm@...ck.org>, bpf
<bpf@...r.kernel.org>, Suren Baghdasaryan <surenb@...gle.com>, Johannes
Weiner <hannes@...xchg.org>, Michal Hocko <mhocko@...e.com>, David
Rientjes <rientjes@...gle.com>, Matt Bobrowski
<mattbobrowski@...gle.com>, Song Liu <song@...nel.org>, Alexei
Starovoitov <ast@...nel.org>, Andrew Morton <akpm@...ux-foundation.org>,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1 01/14] mm: introduce bpf struct ops for OOM handling
Alexei Starovoitov <alexei.starovoitov@...il.com> writes:
> On Tue, Aug 26, 2025 at 11:01 AM Martin KaFai Lau <martin.lau@...ux.dev> wrote:
>>
>> On 8/25/25 10:00 AM, Roman Gushchin wrote:
>> > Martin KaFai Lau <martin.lau@...ux.dev> writes:
>> >
>> >> On 8/20/25 5:24 PM, Roman Gushchin wrote:
>> >>>> How is it decided who gets to run before the other? Is it based on
>> >>>> order of attachment (which can be non-deterministic)?
>> >>> Yeah, now it's the order of attachment.
>> >>>
>> >>>> There was a lot of discussion on something similar for tc progs, and
>> >>>> we went with specific flags that capture partial ordering constraints
>> >>>> (instead of priorities that may collide).
>> >>>> https://lore.kernel.org/all/20230719140858.13224-2-daniel@iogearbox.net
>> >>>> It would be nice if we can find a way of making this consistent.
>> >>
>> >> +1
>> >>
>> >> The cgroup bpf prog has recently added the mprog api support also. If
>> >> the simple order of attachment is not enough and needs to have
>> >> specific ordering, we should make the bpf struct_ops support the same
>> >> mprog api instead of asking each subsystem creating its own.
>> >>
>> >> fyi, another need for struct_ops ordering is to upgrade the
>> >> BPF_PROG_TYPE_SOCK_OPS api to struct_ops for easier extension in the
>> >> future. Slide 13 in
>> >> https://drive.google.com/file/d/1wjKZth6T0llLJ_ONPAL_6Q_jbxbAjByp/view
>> >
>> > Does it mean it's better now to keep it simple in the context of oom
>> > patches with the plan to later reuse the generic struct_ops
>> > infrastructure?
>> >
>> > Honestly, I believe that the simple order of attachment should be
>> > good enough for quite a while, so I'd not over-complicate this,
>> > unless it's not fixable later.
>>
>> I think the simple attachment ordering is fine. Presumably the current link list
>> in patch 1 can be replaced by the mprog in the future. Other experts can chime
>> in if I have missed things.
>
> I don't think the proposed approach of:
> list_for_each_entry_srcu(bpf_oom, &bpf_oom_handlers, node, false) {
> is extensible without breaking things.
> Sooner or later people will want bpf-oom handlers to be per
> container, so we have to think upfront how to do it.
> I would start with one bpf-oom prog per memcg and extend with mprog later.
> Effectively placing 'struct bpf_oom_ops *' into oc->memcg,
> and having one global bpf_oom_ops when oc->memcg == NULL.
> I'm sure other designs are possible, but lets make sure container scope
> is designed from the beginning.
> mprog-like multi prog behavior per container can be added later.
Sounds good to me, will implement something like this in the next version.
Thanks!
Powered by blists - more mailing lists