Re: [PATCH RESEND bpf-next v3 4/6] selftests/bpf: Add arena-based bitmap data structure

["Emil Tsalapatis" <emil@etsalapatis.com>]

On Mon Jul 6, 2026 at 3:04 PM EDT, Kumar Kartikeya Dwivedi wrote:
> On Mon Jul 6, 2026 at 8:27 PM CEST, sashiko-bot wrote:
>> Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
>> - [High] Silent failures and falsely reported lock acquisition in bounded atomic bit operations on loop abort.
>> - [High] `bmp_empty` and `bmp_subset` return `true` on an aborted loop, defaulting to an unsafe affirmative state.
>> - [Medium] Bulk operations unconditionally apply a bitmask to the last word even if the processing loop aborts early.
>> - [Medium] Missing `READ_ONCE()` or `volatile` cast in `bmp_test_bit` exposes lockless polling loops to compiler hoisting.
>> --
>>
>> commit 634ac04142cf736158c99f577c0f903470937bfc
>> Author: Emil Tsalapatis <emil@etsalapatis.com>
>> selftests/bpf: Add arena-based bitmap data structure
>>
>> This patch adds an arena-based word-aligned bitmap data structure. The
>> structure is useful as a building block for components like sched-ext
>> to represent cpumask structures.
>>
>>> diff --git a/tools/testing/selftests/bpf/libarena/src/bitmap.bpf.c b/tools/testing/selftests/bpf/libarena/src/bitmap.bpf.c
>>> index 0000000000000..80e814401fb9e
>>> --- /dev/null
>>> +++ b/tools/testing/selftests/bpf/libarena/src/bitmap.bpf.c
>> [ ... ]
>>> +__weak
>>> +bool bmp_test_bit(u32 bit, struct bitmap __arena *bmp)
>>> +{
>>> +	return bmp->bits[BIT_WORD(bit)] & BIT_MASK(bit);
>>> +}
>>
>> [Severity: Medium]
>> Does this memory access need a READ_ONCE() or a volatile pointer cast?
>>
>> If this function is used in a lockless concurrent polling loop, could the
>> compiler hoist the plain memory read out of the loop and cause an infinite
>> spin that misses updates from other CPUs?
>>
>
> This one seems worth addressing; I would ignore the rest about can_loop
> exhaustion. Please send a follow up fix.

The function is __weak so AFAICT the compiler would not optimize the
call away even if it was reasoning based on the function's body. I
likewise don't think READ_ONCE() would affect anything (correct me
if I'm wrong).

That being said, this function should probably be defined straight in the
bitmap.h header as static inline. It's dumb to require a full call for
a bit operation just because we use __weak the convention for libarena
external API functions. 

I'll send a new patch with the call moved to the header and READ/WRITE
once for the non-atomic get/set/clear ops.

>
>> [...]