Re: [PATCH bpf-next v3 0/4] bpf: Introduce global percpu data

[Leon Hwang <leon.hwang@linux.dev>]

On 29/5/25 01:10, Yonghong Song wrote:
> 
> 
> On 5/27/25 3:31 PM, Andrii Nakryiko wrote:
>> On Mon, May 26, 2025 at 9:22 AM Leon Hwang <leon.hwang@linux.dev> wrote:
>>> This patch set introduces global percpu data, similar to commit
>>> 6316f78306c1 ("Merge branch 'support-global-data'"), to reduce
>>> restrictions
>>> in C for BPF programs.
>>>
>>> With this enhancement, it becomes possible to define and use global
>>> percpu
>>> variables, like the DEFINE_PER_CPU() macro in the kernel[0].
>>>
>>> The section name for global peurcpu data is ".data..percpu". It
>>> cannot be
>>> named ".percpu" or ".percpudata" because defining a one-byte percpu
>>> variable (e.g., char run SEC(".data..percpu") = 0;) can trigger a crash
>>> with Clang 17[1]. The name ".data.percpu" is also avoided because some
>> Does this happen with newer Clangs? If not, I don't think a bug in
>> Clang 17 is reason enough for this weird '.data..percpu' naming
>> convention. I'd still very much prefer .percpu prefix. .data is used
>> for non-per-CPU data, we shouldn't share the prefix, if we can avoid
>> that.
> 
> I checked and clang17 does have a fatal error with '.percpu'. But clang18
> to clang21 all fine.
> 
> For clang17, the error message is
>   fatal error: error in backend: unable to write nop sequence of 3 bytes
> in llvm/lib/MC/MCAssembler.cpp.
> 
> The key reason is in bpf backend llvm/lib/Target/BPF/MCTargetDesc/
> BPFAsmBackend.cpp
> 
> bool BPFAsmBackend::writeNopData(raw_ostream &OS, uint64_t Count,
>                                  const MCSubtargetInfo *STI) const {
>   if ((Count % 8) != 0)
>     return false;
> 
>   for (uint64_t i = 0; i < Count; i += 8)
>     support::endian::write<uint64_t>(OS, 0x15000000, Endian);
> 
>   return true;
> }
> 
> Since Count is 3, writeNopData returns false and it caused the fatal error.
> 
> The bug is likely in MC itself as for the same BPF writeNopData
> implementatation,
> clang18 works fine (with Count is 8). So the bug should be fixed in
> clang18.
> 
From my testing, Clang 18 handles 'char run SEC(".percpu");' correctly
without crashing.

To use the '.percpu' section while avoiding the Clang 17 bug, the test
case can be adjusted as follows:

int data SEC(".percpu") = -1;
int nums[7] SEC(".percpu");
#if defined(__clang__) && __clang_major__ >= 18
char run SEC(".percpu") = 0;
struct {
	char set;
	int i;
	int nums[7];
} struct_data SEC(".percpu") = {
	.set = 0,
	.i = -1,
};
#else
struct {
	int i;
	int nums[7];
} struct_data SEC(".percpu") = {
	.i = -1,
};
#endif

SEC("raw_tp/task_rename")
int update_percpu_data(struct __sk_buff *skb)
{
	struct_data.nums[6] = 0xc0de;
	struct_data.i = 1;
	nums[6] = 0xc0de;
	data = 1;
#if defined(__clang__) && __clang_major__ >= 18
	struct_data.set = 1;
	run = 1;
#endif
	return 0;
}

With this change, the 'char run SEC(".percpu");' declaration will only
be compiled and tested with Clang 18 or newer, effectively avoiding the
crash with Clang 17.

Thanks,
Leon