Re: [PATCH bpf-next v13 3/6] bpf: Add sleepable support for classic tracepoint programs

[Mykyta Yatsenko <mykyta.yatsenko5@gmail.com>]

On 4/22/26 9:57 PM, bot+bpf-ci@kernel.org wrote:
>> diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
>> index 8ad72e17d8eb..e98ee7e1e66f 100644
>> --- a/kernel/trace/trace_syscalls.c
>> +++ b/kernel/trace/trace_syscalls.c
> 
> [ ... ]
> 
>> @@ -1410,12 +1410,7 @@ static void perf_syscall_enter(void *ignore, struct pt_regs *regs, long id)
>>   	int size = 0;
>>   	int uargs = 0;
>>
>> -	/*
>> -	 * Syscall probe called with preemption enabled, but the ring
>> -	 * buffer and per-cpu data require preemption to be disabled.
>> -	 */
>>   	might_fault();
>> -	guard(preempt_notrace)();
>>
>>   	syscall_nr = trace_get_syscall_nr(current, regs);
>>   	if (syscall_nr < 0 || syscall_nr >= NR_syscalls)
>> @@ -1429,6 +1424,26 @@ static void perf_syscall_enter(void *ignore, struct pt_regs *regs, long id)
>>
>>   	syscall_get_arguments(current, regs, args);
>>
>> +	/*
>> +	 * Run BPF program in faultable context before per-cpu buffer
>> +	 * allocation, allowing sleepable BPF programs to execute.
>> +	 */
>> +	valid_prog_array = bpf_prog_array_valid(sys_data->enter_event);
>> +	if (valid_prog_array &&
>> +	    !perf_call_bpf_enter(sys_data->enter_event, sys_data,
>> +				 syscall_nr, args))
>> +		return;
>> +
>> +	/*
>> +	 * Per-cpu ring buffer and perf event list operations require
>> +	 * preemption to be disabled.
>> +	 */
>> +	guard(preempt_notrace)();
>> +
>> +	head = this_cpu_ptr(sys_data->enter_event->perf_events);
>> +	if (hlist_empty(head))
>> +		return;
>> +
> 
> Could this lead to unnecessary buffer allocation in a race scenario?
> 
> The patch moves BPF execution before preempt_disable to enable sleepable
> programs. The new code checks valid_prog_array outside preempt_disable
> and runs BPF if true. Then it enters preempt_disable and checks if the
> hlist is empty.
> 
> Between the BPF check (outside preempt_disable) and the hlist check
> (inside preempt_disable), another CPU could unregister the last perf
> event, making the hlist empty. In this window, the code would:
> 
> 1. Run BPF and pass the filter (before preempt_disable)
> 2. Check valid_prog_array and find it true (before preempt_disable)
> 3. Enter preempt_disable via guard(preempt_notrace)()
> 4. Check hlist_empty(head) and find it false (inside preempt_disable)
> 5. Allocate buffer via perf_trace_buf_alloc()
> 6. Submit to an empty or nearly-empty hlist
> 
> The old code checked both conditions together under preempt_disable
> before buffer allocation:
> 
>      head = this_cpu_ptr(sys_data->enter_event->perf_events);
>      valid_prog_array = bpf_prog_array_valid(sys_data->enter_event);
>      if (!valid_prog_array && hlist_empty(head))
>          return;  /* Early exit before buffer allocation */
> 
> If the perf event is unregistered between the new code's BPF execution
> and the hlist check, would the buffer allocation and data copying be
> wasted work?
> 

Given that the downside is wasted work of allocation and data copy,
it's not necessarily a problem, because it's not a hot path.

> [ ... ]
> 
> 
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
> 
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/24799223877