Re: [PATCH bpf-next v7 4/9] selftests/bpf: Move READ_ONCE/WRITE_ONCE macros to bpf_experimental.h

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

On Fri Apr 17, 2026 at 12:21 PM EDT, Kumar Kartikeya Dwivedi wrote:
> On Fri, 17 Apr 2026 at 17:06, Emil Tsalapatis <emil@etsalapatis.com> wrote:
>>
>> On Sun Apr 12, 2026 at 3:33 PM EDT, Kumar Kartikeya Dwivedi wrote:
>> > On Sun, 12 Apr 2026 at 19:45, Emil Tsalapatis <emil@etsalapatis.com> wrote:
>> >>
>> >> The WRITE_ONCE macro is identically defined both in bpf_atomic.h
>> >> and in bpf_arena_common.h. Deduplicate the definitions by moving
>> >> the macro to the commonly used bpf_experimental.h header. Also
>> >> move the READ_ONCE macro to the same file for uniformity.
>> >>
>> >
>> > The right header for these is bpf_atomic.h, not bpf_experimental.h. We
>> > want volatile loads to be shipped with bpf_atomic.h and all other
>> > atomic primitives.
>> > https://lore.kernel.org/bpf/CAP01T754eOs2DCjMmkK=CppEwfk5EX84tc70ZqS0_OxTfgxE=w@mail.gmail.com
>>
>> bpf_experimental.h is included in bpf_atomic.h, so there's no possible
>> breakage there. A bunch of arena-related code like bpf_arena_list.h
>> also uses WRITE_ONCE but doesn't include bpf_atomic.h, so we'd have to
>> manually add an extra headers for no reason. Even if we do that,
>> bpf_atomic.h includes vmlinux.h and so any header like bpf_arena_list.h
>> I mentioned that requires WRITE_ONCE won't be importable by userspace.
>>
>> Besides, WRITE_ONCE/READ_ONCE are ultimately compiler shorthands even if
>> they are used most heavily in atomics and don't stick out in
>> bpf_experimental.h.
>>
>
> It's a sad situation indeed. It just feels odd not to provide the
> equivalent of relaxed loads in the header encapsulating all atomic
> access primitives.

I agree it's not the best, my first attempt was to move the definitions to
bpf_atomic.h but that ended up being so much extra adjustments that it
felt silly to do just for this macro.

> bpf_experimental.h is a kitchen sink header for everything we need to
> include that doesn't get its own definition somewhere.

Maybe that is the problem? bpf_experimental.h has outgrown its original
purpose, it it worth splitting into multiple headers to avoid carrying
around all the compat scaffolding just to get access to a couple
definitions? For example, cast_kern/cast_user and the hardcoded address
cast bytecode are useless to most programs (and most arena programs
since they require LLVM >= 19), but they are still included everywhere.

> I think trying to be consistent and deliberating on having topical
> headers for a library you want to export is a good exercise to have
> right now, while we're at it.

>
> It might be cleaner in the end to just place ifndef around all of the
> macro definitions in bpf_atomic.h. That's probably good hygiene too,
> since I presume the naming is universal and programs themselves may
> have their own versions, etc.
>

> Maybe others have better opinions. I would have suggested namespacing
> everything with arena_, but then they work on non-arena just as well,
> so that doesn't make too much sense.
>
>
>>
>> >
>> >> Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
>> >> Acked-by: Song Liu <song@kernel.org>
>> >> ---
>> >> [...]
>>