All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yonghong Song <yonghong.song@linux.dev>
To: bot+bpf-ci@kernel.org, puranjay@kernel.org, bpf@vger.kernel.org
Cc: puranjay12@gmail.com, ast@kernel.org, andrii@kernel.org,
	daniel@iogearbox.net, martin.lau@kernel.org, eddyz87@gmail.com,
	memxor@gmail.com, kernel-team@meta.com, catalin.marinas@arm.com,
	will@kernel.org, mark.rutland@arm.com,
	linux-arm-kernel@lists.infradead.org, clm@meta.com,
	ihor.solodrai@linux.dev
Subject: Re: [PATCH bpf-next 2/2] bpf: arm64: Optimize recursion detection by not using atomics
Date: Wed, 17 Dec 2025 10:23:59 -0800	[thread overview]
Message-ID: <51466fd3-c837-46a6-af50-28a8336fd8cd@linux.dev> (raw)
In-Reply-To: <f144fd46b602b74fc4c1c2664082fbe893e7ec9c274fcc5fdf13d65151749e9c@mail.kernel.org>



On 12/17/25 8:56 AM, bot+bpf-ci@kernel.org wrote:
>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
>> index 2da986136d26..654fb94bf60c 100644
>> --- a/include/linux/bpf.h
>> +++ b/include/linux/bpf.h
> [ ... ]
>
>> @@ -2006,12 +2009,32 @@ struct bpf_struct_ops_common_value {
>>
>>   static inline bool bpf_prog_get_recursion_context(struct bpf_prog *prog)
>>   {
>> +#ifdef CONFIG_ARM64
>> +	u8 rctx = interrupt_context_level();
>> +	u8 *active = this_cpu_ptr(prog->active);
>> +
>> +	active[rctx]++;
>> +	barrier();
>> +	if (get_unaligned_le32(active) != BIT(rctx * 8))
>> +		return false;
>> +
>> +	return true;
>> +#else
>>   	return this_cpu_inc_return(*(prog->active)) == 1;
>> +#endif
>>   }
> Can preemption between the increment and check cause a counter leak on
> CONFIG_PREEMPT_RCU systems? The trampoline calls this function under
> rcu_read_lock_dont_migrate() which allows preemption on PREEMPT_RCU
> (documented at include/linux/rcupdate.h:856).
>
> Consider this scenario on an ARM64 system with PREEMPT_RCU:
>
> 1. Thread A increments active[0] to 1
> 2. Preemption occurs before Thread A reaches the check
> 3. Thread B on same CPU increments active[0] to 2
> 4. Thread B checks: sees 2 != BIT(0), returns false
> 5. Thread A resumes, checks: sees 2 != BIT(0), returns false
> 6. Both threads return false, neither runs BPF
> 7. Neither calls bpf_prog_put_recursion_context() (see
>     __bpf_prog_enter_recur() at kernel/bpf/trampoline.c:952)
> 8. Counter permanently stuck at 2, all future BPF on this CPU fails
>
> The old atomic code handled this correctly because this_cpu_inc_return()
> completes atomically, ensuring Thread A reads the value 1 before Thread B
> can interfere. With non-atomic operations, Thread A increments but hasn't
> read the value yet, allowing Thread B to interfere and cause both threads
> to see the modified value.
>
> Is there a guarantee that same-context preemption cannot occur in the BPF
> execution path on ARM64, or does this need additional synchronization?

AI is correct here. See below for another thread:
   https://lore.kernel.org/bpf/20251217093326.1745307-1-chen.dylane@linux.dev/T/#m906fd4502fbbedd4609c586122a393363003312a
where preempt_disable is necessary to prevent the above scenario.

But adding preempt_disable may impact the overall system-level performance.

Does this patch can improve performance for *all* ARM64 cpu versions?
Do you have numbers to show how much performance improvement?

>
>> @@ -2006,12 +2009,32 @@ struct bpf_struct_ops_common_value {
>>
>>   static inline void bpf_prog_put_recursion_context(struct bpf_prog *prog)
>>   {
>> +#ifdef CONFIG_ARM64
>> +	u8 rctx = interrupt_context_level();
>> +	u8 *active = this_cpu_ptr(prog->active);
>> +
>> +	barrier();
>> +	active[rctx]--;
>> +#else
>>   	this_cpu_dec(*(prog->active));
>> +#endif
>>   }
[...]

  parent reply	other threads:[~2025-12-17 18:24 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-17 16:28 [PATCH bpf-next 0/2] bpf: Optimize recursion detection on arm64 Puranjay Mohan
2025-12-17 16:28 ` [PATCH bpf-next 1/2] bpf: move recursion detection logic to helpers Puranjay Mohan
2025-12-17 16:28 ` [PATCH bpf-next 2/2] bpf: arm64: Optimize recursion detection by not using atomics Puranjay Mohan
2025-12-17 16:56   ` bot+bpf-ci
2025-12-17 18:13     ` Puranjay Mohan
2025-12-17 18:46       ` Alexei Starovoitov
2025-12-17 18:56         ` Puranjay Mohan
2025-12-17 21:31           ` Yonghong Song
2025-12-17 22:29             ` Puranjay Mohan
2025-12-17 18:23     ` Yonghong Song [this message]
2025-12-17 18:44       ` Puranjay Mohan
2025-12-17 18:47         ` Alexei Starovoitov
2025-12-17 19:45         ` Yonghong Song

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=51466fd3-c837-46a6-af50-28a8336fd8cd@linux.dev \
    --to=yonghong.song@linux.dev \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bot+bpf-ci@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=clm@meta.com \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=ihor.solodrai@linux.dev \
    --cc=kernel-team@meta.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=mark.rutland@arm.com \
    --cc=martin.lau@kernel.org \
    --cc=memxor@gmail.com \
    --cc=puranjay12@gmail.com \
    --cc=puranjay@kernel.org \
    --cc=will@kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.