BPF List
 help / color / mirror / Atom feed
From: Chris Mason <clm@meta.com>
To: Roman Gushchin <roman.gushchin@linux.dev>
Cc: bot+bpf-ci@kernel.org, akpm@linux-foundation.org,
	linux-kernel@vger.kernel.org, ast@kernel.org, surenb@google.com,
	mhocko@kernel.org, shakeel.butt@linux.dev, hannes@cmpxchg.org,
	andrii@kernel.org, inwardvessel@gmail.com, linux-mm@kvack.org,
	cgroups@vger.kernel.org, bpf@vger.kernel.org,
	martin.lau@kernel.org, song@kernel.org, memxor@gmail.com,
	tj@kernel.org, daniel@iogearbox.net, eddyz87@gmail.com,
	yonghong.song@linux.dev, ihor.solodrai@linux.dev
Subject: Re: [PATCH v2 14/23] mm: allow specifying custom oom constraint for BPF triggers
Date: Tue, 28 Oct 2025 12:35:11 -0400	[thread overview]
Message-ID: <52ac4439-781f-4267-a374-ae0643a2c36b@meta.com> (raw)
In-Reply-To: <87v7jz3smj.fsf@linux.dev>



On 10/28/25 12:20 PM, Roman Gushchin wrote:
> Chris Mason <clm@meta.com> writes:
> 
>> On 10/27/25 7:48 PM, bot+bpf-ci@kernel.org wrote:
>>>> @@ -1350,11 +1367,14 @@ __bpf_kfunc int bpf_oom_kill_process(struct oom_control *oc,
>>>>   * Returns a negative value if an error occurred.
>>>>   */
>>>>  __bpf_kfunc int bpf_out_of_memory(struct mem_cgroup *memcg__nullable,
>>>> -				  int order, u64 flags)
>>>> +				  int order, u64 flags,
>>>> +				  const char *constraint_text__nullable)
>>>>  {
>>>>  	struct oom_control oc = {
>>>>  		.memcg = memcg__nullable,
>>>>  		.order = order,
>>>> +		.constraint = CONSTRAINT_BPF,
>>>> +		.bpf_constraint = constraint_text__nullable,
>>>>  	};
>>>>  	int ret;
>>>
>>> When CONSTRAINT_BPF is set in bpf_out_of_memory(), the early return in
>>> constrained_alloc() prevents oc->totalpages from being initialized.  This
>>> leaves totalpages at zero (from the designated initializer).
>>>
>>> Later in the call chain out_of_memory()->select_bad_process()->
>>> oom_evaluate_task()->oom_badness(), the code performs division by
>>> totalpages at line 237:
>>>
>>>     adj *= totalpages / 1000;
>>>
>>> Can this cause a division by zero?  The path is reachable when a BPF
>>> program calls bpf_out_of_memory() and either no BPF OOM handler is
>>> registered or the handler fails to free memory, causing execution to fall
>>> through to select_bad_process().
>>
>> Looks like the AI got a little excited about finding the uninit variable
>> chain and forgot what dividing by zero really means.  I'll add a false
>> positive check for this.
> 
> Yup, it was *almost* correct :)
> 
> But overall I'm really impressed: it found few legit bugs as well.
> The only thing: I wish I could run it privately before posting to
> public mailing lists...

I'm pretty happy with the false positive rate, and definitely appreciate
people engaging with the AI reviews to help improve things.

The BPF CI is directly running my review prompts github
(https://github.com/masoncl/review-prompts), so it's possible to run
locally with claude-code, and I'm assuming any of the other agents.

I've been refining the prompts against claude, but welcome patches to
make it work well with any of the others.

-chris


  reply	other threads:[~2025-10-28 16:35 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-27 23:21 [PATCH v2 11/23] mm: introduce BPF kfunc to access memory events Roman Gushchin
2025-10-27 23:21 ` [PATCH v2 12/23] bpf: selftests: selftests for memcg stat kfuncs Roman Gushchin
2025-10-27 23:21 ` [PATCH v2 13/23] mm: introduce bpf_out_of_memory() BPF kfunc Roman Gushchin
2025-10-27 23:57   ` bot+bpf-ci
2025-10-28 16:43     ` Roman Gushchin
2025-11-10  9:46   ` Michal Hocko
2025-11-11 19:13     ` Roman Gushchin
2025-11-12  7:50       ` Michal Hocko
2025-10-27 23:21 ` [PATCH v2 14/23] mm: allow specifying custom oom constraint for BPF triggers Roman Gushchin
2025-10-27 23:48   ` bot+bpf-ci
2025-10-28 15:58     ` Chris Mason
2025-10-28 16:20       ` Roman Gushchin
2025-10-28 16:35         ` Chris Mason [this message]
2025-11-10  9:31   ` Michal Hocko
2025-11-11 19:17     ` Roman Gushchin
2025-11-12  7:52       ` Michal Hocko
2025-10-27 23:21 ` [PATCH v2 15/23] mm: introduce bpf_task_is_oom_victim() kfunc Roman Gushchin
2025-10-28 17:32   ` Tejun Heo
2025-10-28 18:09     ` Roman Gushchin
2025-10-28 18:31       ` Tejun Heo
2025-10-27 23:21 ` [PATCH v2 16/23] libbpf: introduce bpf_map__attach_struct_ops_opts() Roman Gushchin
2025-10-27 23:48   ` bot+bpf-ci
2025-10-28 17:07     ` Roman Gushchin
2025-10-28 17:24       ` Andrii Nakryiko
2025-10-27 23:22 ` [PATCH v2 17/23] bpf: selftests: introduce read_cgroup_file() helper Roman Gushchin
2025-10-27 23:48   ` bot+bpf-ci
2025-10-28 16:31     ` Roman Gushchin
2025-10-27 23:22 ` [PATCH v2 18/23] bpf: selftests: BPF OOM handler test Roman Gushchin
2025-10-27 23:22 ` [PATCH v2 19/23] sched: psi: refactor psi_trigger_create() Roman Gushchin
2025-10-27 23:22 ` [PATCH v2 20/23] sched: psi: implement bpf_psi struct ops Roman Gushchin
2025-10-27 23:48   ` bot+bpf-ci
2025-10-28 17:40   ` Tejun Heo
2025-10-28 18:29     ` Roman Gushchin
2025-10-28 18:35       ` Tejun Heo
2025-10-28 19:54         ` Roman Gushchin
2025-10-27 23:22 ` [PATCH v2 21/23] sched: psi: implement bpf_psi_create_trigger() kfunc Roman Gushchin
2025-12-08  8:49   ` hui.zhu
2025-12-09  1:49     ` Roman Gushchin
2025-10-27 23:22 ` [PATCH v2 22/23] bpf: selftests: add config for psi Roman Gushchin
2025-10-27 23:22 ` [PATCH v2 23/23] bpf: selftests: PSI struct ops test Roman Gushchin
2025-10-27 23:48   ` bot+bpf-ci
2025-10-28 17:13     ` Roman Gushchin
2025-10-28 17:30       ` Alexei Starovoitov
2025-11-10  9:48   ` Michal Hocko
2025-11-11 19:03     ` Roman Gushchin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=52ac4439-781f-4267-a374-ae0643a2c36b@meta.com \
    --to=clm@meta.com \
    --cc=akpm@linux-foundation.org \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bot+bpf-ci@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=cgroups@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=hannes@cmpxchg.org \
    --cc=ihor.solodrai@linux.dev \
    --cc=inwardvessel@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=martin.lau@kernel.org \
    --cc=memxor@gmail.com \
    --cc=mhocko@kernel.org \
    --cc=roman.gushchin@linux.dev \
    --cc=shakeel.butt@linux.dev \
    --cc=song@kernel.org \
    --cc=surenb@google.com \
    --cc=tj@kernel.org \
    --cc=yonghong.song@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox