Re: [PATCH bpf-next] bpf: use preempt_disable/enable() to protect bpf_bprintf_buffers nesting

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

On 11/10/25 5:25 AM, Sebastian Andrzej Siewior wrote:
> On 2025-11-09 11:44:48 [-0800], Yonghong Song wrote:
>
> Could we do this instead?
> There is  __bpf_stream_push_str() => bpf_stream_page_reserve_elem() =>
> bpf_stream_page_replace() => alloc_pages_nolock().

I would suggest to stick to preempt_disable/enable().
In the bpf-next (newer change), for function
bpf_stream_elem_alloc(), kmalloc_nolock() is used
and no local_lock usage any more.


>
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index b469878de25c8..5a4965724c374 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -1598,6 +1598,7 @@ struct task_struct {
>   	void				*security;
>   #endif
>   #ifdef CONFIG_BPF_SYSCALL
> +	s8				bpf_bprintf_idx;
>   	/* Used by BPF task local storage */
>   	struct bpf_local_storage __rcu	*bpf_storage;
>   	/* Used for BPF run context */
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index eb25e70e0bdc0..62e37c845ec5a 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -770,28 +770,39 @@ static int bpf_trace_copy_string(char *buf, void *unsafe_ptr, char fmt_ptype,
>   /* Support executing three nested bprintf helper calls on a given CPU */
>   #define MAX_BPRINTF_NEST_LEVEL	3
>   
> -static DEFINE_PER_CPU(struct bpf_bprintf_buffers[MAX_BPRINTF_NEST_LEVEL], bpf_bprintf_bufs);
> -static DEFINE_PER_CPU(int, bpf_bprintf_nest_level);
> +struct bpf_cpu_buffer {
> +	struct bpf_bprintf_buffers bufs[MAX_BPRINTF_NEST_LEVEL];
> +	local_lock_t	lock[MAX_BPRINTF_NEST_LEVEL];
> +};
> +
> +static DEFINE_PER_CPU(struct bpf_cpu_buffer, bpf_cpu_bprintf) = {
> +	.lock = { [0 ... MAX_BPRINTF_NEST_LEVEL - 1] = INIT_LOCAL_LOCK(bpf_cpu_bprintf.lock) },
> +};
>   
>   int bpf_try_get_buffers(struct bpf_bprintf_buffers **bufs)
>   {
> -	int nest_level;
> +	s8 nest_level;
>   
> -	nest_level = this_cpu_inc_return(bpf_bprintf_nest_level);
> -	if (WARN_ON_ONCE(nest_level > MAX_BPRINTF_NEST_LEVEL)) {
> -		this_cpu_dec(bpf_bprintf_nest_level);
> +	nest_level = current->bpf_bprintf_idx++;
> +	if (WARN_ON_ONCE(nest_level >= MAX_BPRINTF_NEST_LEVEL)) {
> +		current->bpf_bprintf_idx--;
>   		return -EBUSY;
>   	}
> -	*bufs = this_cpu_ptr(&bpf_bprintf_bufs[nest_level - 1]);
>   
> +	local_lock(&bpf_cpu_bprintf.lock[nest_level]);
> +	*bufs = this_cpu_ptr(&bpf_cpu_bprintf.bufs[nest_level]);
>   	return 0;
>   }
>   
>   void bpf_put_buffers(void)
>   {
> -	if (WARN_ON_ONCE(this_cpu_read(bpf_bprintf_nest_level) == 0))
> +	s8 nest_level;
> +
> +	nest_level = current->bpf_bprintf_idx;
> +	if (WARN_ON_ONCE(nest_level == 0))
>   		return;
> -	this_cpu_dec(bpf_bprintf_nest_level);
> +	local_unlock(&bpf_cpu_bprintf.lock[nest_level - 1]);
> +	current->bpf_bprintf_idx--;
>   }
>   
>   void bpf_bprintf_cleanup(struct bpf_bprintf_data *data)