All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ihor Solodrai <ihor.solodrai@linux.dev>
To: Emil Tsalapatis <emil@etsalapatis.com>, bpf@vger.kernel.org
Cc: ast@kernel.org, andrii@kernel.org, memxor@gmail.com,
	daniel@iogearbox.net, eddyz87@gmail.com,
	mattbobrowski@google.com, song@kernel.org
Subject: Re: [PATCH bpf-next 2/5] selftests/bpf: libarena: Fix can-loop zero variable definition
Date: Tue, 30 Jun 2026 12:28:55 -0700	[thread overview]
Message-ID: <216b5567-a3f6-4720-8528-9868afe2012a@linux.dev> (raw)
In-Reply-To: <20260618085626.19633-3-emil@etsalapatis.com>

On 6/18/26 1:56 AM, Emil Tsalapatis wrote:
> BPF can_loop based loops require the index variable to stay imprecise.
> This means we must initialize them from a currently imprecise variable
> instead of directly assigning 0 to them, like so:
> 
> static volatile u32 zero = 0;
> 
> for (i = zero; i < NUM_LOOPS; i++) {
> 	/* loop body */
> }
> 
> The libarena implementation of this technique is currently faulty. For
> the technique to work, the variable must not be in a map. This includes
> the .rodata DATASEC map used for const variables. However, libarena
> still defines the zero variable as constant.
> 
> Modify the zero variable definition into a volatile variable. This
> change adds a complication caused by the compiler optimizing array
> derefences from
> 
> for (i = zero; i < NUM_LOOPS; i++) {
> 	val = *(ptr + i);
> }
> 
> into
> 
> for (i = zero; i < NUM_LOOPS; i++) {
> 	val = *ptr++;
> }
> 
> and causing verification failures. Use the barrier_var() clobber macro
> to prevent this optimization from taking place. After that, remove the
> bpf_for() invocations introduced in libarena for parallel spmc testing.
> 
> Reported-by: Eduard Zingerman <eddyz87@gmail.com>
> Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
> ---
>  .../selftests/bpf/libarena/include/libarena/common.h     | 2 +-
>  .../bpf/libarena/selftests/test_asan_buddy.bpf.c         | 8 ++++++--
>  .../selftests/bpf/libarena/selftests/test_buddy.bpf.c    | 8 ++++++--
>  .../bpf/libarena/selftests/test_parallel_spmc.bpf.c      | 9 ++++-----
>  tools/testing/selftests/bpf/libarena/src/common.bpf.c    | 3 +--
>  5 files changed, 18 insertions(+), 12 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/libarena/include/libarena/common.h b/tools/testing/selftests/bpf/libarena/include/libarena/common.h
> index a3eb1641ac36..931ace9a49e2 100644
> --- a/tools/testing/selftests/bpf/libarena/include/libarena/common.h
> +++ b/tools/testing/selftests/bpf/libarena/include/libarena/common.h
> @@ -43,7 +43,7 @@ struct {
>   * imprecise. To force the variable to be imprecise, initialize it with
>   * the opaque volatile variable 0 instead of the constant 0.
>   */
> -extern const volatile u32 zero;
> +volatile u32 zero __weak;
>  extern volatile u64 asan_violated;
>  
>  int arena_fls(__u64 word);
> diff --git a/tools/testing/selftests/bpf/libarena/selftests/test_asan_buddy.bpf.c b/tools/testing/selftests/bpf/libarena/selftests/test_asan_buddy.bpf.c
> index 256d62a03ce7..3266a28f53d7 100644
> --- a/tools/testing/selftests/bpf/libarena/selftests/test_asan_buddy.bpf.c
> +++ b/tools/testing/selftests/bpf/libarena/selftests/test_asan_buddy.bpf.c
> @@ -154,7 +154,8 @@ __weak int asan_test_buddy_oob(void)
>  	size_t sizes[] = {
>  		7, 8, 17, 18, 64, 256, 317, 512, 1024,
>  	};
> -	int ret, i;
> +	int ret;
> +	u32 i;
>  
>  	ret = buddy_init(&buddy);
>  	if (ret) {
> @@ -163,6 +164,7 @@ __weak int asan_test_buddy_oob(void)
>  	}
>  
>  	for (i = zero; i < sizeof(sizes) / sizeof(sizes[0]) && can_loop; i++) {
> +		barrier_var(i);

Looks like we are using two different idioms for keeping indices
imprecise and blocking the compiler optimization: the `barrier_var(i)`
like here, and `volatile u32 i` declaration in patches #4 and #5.

My understanding is the barrier is generally better performance-wise,
since volatile var would be re-loaded every time.

Does it make sense to converge on a single pattern (at least in the
context of libarena)?


> [...]


  parent reply	other threads:[~2026-06-30 19:29 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-18  8:56 [PATCH bpf-next 0/5] selftests/bpf: libarena cleanup and bitmap struct Emil Tsalapatis
2026-06-18  8:56 ` [PATCH bpf-next 1/5] libarena/selftests: Replace leftover st_ prefix with test_ Emil Tsalapatis
2026-06-30 19:21   ` Ihor Solodrai
2026-07-01  4:23     ` Emil Tsalapatis
2026-06-18  8:56 ` [PATCH bpf-next 2/5] selftests/bpf: libarena: Fix can-loop zero variable definition Emil Tsalapatis
2026-06-18  9:09   ` sashiko-bot
2026-06-25 19:15   ` Eduard Zingerman
2026-06-30 19:28   ` Ihor Solodrai [this message]
2026-07-01  4:28     ` Emil Tsalapatis
2026-06-18  8:56 ` [PATCH bpf-next 3/5] selftests/bpf: libarena: Clean up allocation state before buddy tests Emil Tsalapatis
2026-06-18  9:47   ` bot+bpf-ci
2026-06-23 23:42     ` Emil Tsalapatis
2026-06-30 20:23   ` Ihor Solodrai
2026-06-18  8:56 ` [PATCH bpf-next 4/5] selftests/bpf: Add arena-based bitmap data structure Emil Tsalapatis
2026-06-18  9:08   ` sashiko-bot
2026-06-23 23:30     ` Emil Tsalapatis
2026-06-18  9:47   ` bot+bpf-ci
2026-06-30 21:44   ` Ihor Solodrai
2026-07-01  4:33     ` Emil Tsalapatis
2026-06-18  8:56 ` [PATCH bpf-next 5/5] selftests/bpf: libarena: Add bitmap selftets Emil Tsalapatis
2026-06-30 21:46   ` Ihor Solodrai
2026-07-01  4:33     ` Emil Tsalapatis

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=216b5567-a3f6-4720-8528-9868afe2012a@linux.dev \
    --to=ihor.solodrai@linux.dev \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=emil@etsalapatis.com \
    --cc=mattbobrowski@google.com \
    --cc=memxor@gmail.com \
    --cc=song@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.