Re: [PATCH bpf-next v2 2/6] bpf: Add sleepable execution path for raw tracepoint programs

[Yonghong Song <yonghong.song@linux.dev>]

On 3/5/26 8:52 PM, Kumar Kartikeya Dwivedi wrote:
> On Fri, 6 Mar 2026 at 05:23, Kumar Kartikeya Dwivedi <memxor@gmail.com> wrote:
>> On Wed, 25 Feb 2026 at 18:14, <bot+bpf-ci@kernel.org> wrote:
>>>> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
>>>> index 9bc0dfd235af..2dd345e0fdb0 100644
>>>> --- a/kernel/trace/bpf_trace.c
>>>> +++ b/kernel/trace/bpf_trace.c
>>> [ ... ]
>>>
>>>> @@ -2085,12 +2085,26 @@ void __bpf_trace_run(struct bpf_raw_tp_link *link, u64 *args)
>>>>        run_ctx.bpf_cookie = link->cookie;
>>>>        old_run_ctx = bpf_set_run_ctx(&run_ctx.run_ctx);
>>>>
>>>> -     (void) bpf_prog_run(prog, args);
>>>> +     if (prog->sleepable) {
>>>> +             might_fault();
>>>> +             (void)bpf_prog_run(prog, args);
>>> At this commit, __BPF_DECLARE_TRACE_SYSCALL in include/trace/bpf_probe.h
>>> still wraps the BPF callback in preempt_disable_notrace() /
>>> preempt_enable_notrace():
>>>
>>> include/trace/bpf_probe.h:__BPF_DECLARE_TRACE_SYSCALL() {
>>>      might_fault();
>>>      preempt_disable_notrace();
>>>      bpf_trace_run##x(__data, ...);
>>>      preempt_enable_notrace();
>>> }
>>>
>>> Since __bpf_trace_run() is inlined into bpf_trace_run##x(), which is
>>> called between the preempt_disable_notrace() and
>>> preempt_enable_notrace(), the might_fault() here will execute with
>>> preempt_count > 0, producing a warning on CONFIG_DEBUG_ATOMIC_SLEEP
>>> kernels.
>>>
>>> More importantly, if the sleepable program calls a helper like
>>> bpf_copy_from_user(), the page fault handler will see in_atomic()
>>> and return -EFAULT for any non-resident page instead of faulting it
>>> in, defeating the purpose of sleepable programs.
>>>
>>> The v1 RFC series included a separate patch to remove
>>> preempt_disable_notrace() from __BPF_DECLARE_TRACE_SYSCALL.  Should
>>> that change be folded into this commit so the sleepable path works
>>> at each bisect point?
>> But sleepable programs are not yet permitted, so I think this is not
>> needed for bisection-friendliness.
>>
>>>> +     } else {
>>>> +             /*
>>>> +              * Non-sleepable programs may run in the faultable context,
>>>> +              * do cant_sleep() only if program is non-sleepable and context
>>>> +              * is non-faultable.
>>>> +              */
>>>> +             if (!link->link.sleepable)
>>>> +                     cant_sleep();
>>>> +             rcu_read_lock();
>>>> +             (void)bpf_prog_run(prog, args);
>>>> +             rcu_read_unlock();
>>>> +     }
>>> [ ... ]
>>>
>>> The commit message states:
>>>
>>>> For non-sleepable programs, behavior is unchanged: cant_sleep() check,
>>>> rcu_read_lock() protection.
>>> The parent commit does not have a cant_sleep() call in
>>> __bpf_trace_run() -- it uses rcu_read_lock_dont_migrate().  The
>>> conditional cant_sleep() is new in this commit.  Could this sentence
>>> be updated to reflect the actual change?
>> This seems worth adjusting.
> Actually, with the extra cant_sleep() check, we get this splat when
> running normal raw_tp tests.
> E.g.
>
> ...
> [  295.211949] BUG: assuming atomic context at kernel/trace/bpf_trace.c:2098
> [  295.212166] in_atomic(): 0, irqs_disabled(): 0, pid: 587, name: test_progs
> [  295.212316] 3 locks held by test_progs/587:
> ...
> [  295.213044] Call Trace:
> [  295.213046]  <TASK>
> [  295.213048]  dump_stack_lvl+0x54/0x70
> [  295.213055]  __cant_sleep+0xb7/0xd0
> [  295.213060]  bpf_trace_run1+0xce/0x340
> [  295.213072]  bpf_testmod_test_read+0x433/0x5e0 [bpf_testmod]
> [  295.213080]  ? srso_return_thunk+0x5/0x5f
> [  295.213093]  kernfs_fop_read_iter+0x166/0x220
> ...
>
> It means if the tracepoint is not marked faultable but is invoked in a
> non-atomic context, we will get a warning.
> It might be better to just drop this, since I don't think this adds much value.

Agree. This is due to the tracepoints defined in test_kmods where
tracepoints have default non-sleepable. So we have
   prog: non-sleepable
   test_kmods assumed context: non-sleepable
   actual context: sleepable

This caused the warning.

BTW, can we merge patches 2 and 3 together? I know the sleepable tp_btf
is not enabled yet in verifier. But just looking at patch 2 is a little
bit confusing as its caller still has preempt_{disable,enable}_notrace().

>
>
>>>
>> Other than that LGTM.
>>
>> Acked-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
>>
>>> [...]