Skip to content

Commit 4ea63db

Browse files
raflybroclaude
andcommitted
fix: avoid XADD-with-fetch atomic that aborts on older LLVM BPF backends
block_rq_issue used __sync_fetch_and_add's return value against a shared BPF_ARRAY counter to assign req_id, which lowers to a BPF XADD with the BPF_FETCH flag. Older/distro clang BPF backends can't legally emit that and hard-abort the whole process with "LLVM ERROR: Invalid usage of the XADD return value" (uncatchable as a Python exception, since BCC embeds clang/LLVM in-process). Replace the shared atomic counter with a BPF_PERCPU_ARRAY bumped via a plain per-CPU increment, folding the CPU id into the high bits for cross-CPU uniqueness, removing the need for any atomic fetch-and-add entirely. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 81033a0 commit 4ea63db

2 files changed

Lines changed: 50 additions & 6 deletions

File tree

docs/COMPATIBILITY_FIXES.md

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,3 +81,38 @@ dump. The captured data includes:
8181
* **System specs**: OS/distribution, CPU, and memory. The IP-geolocation
8282
country lookup is intentionally skipped on this path so a slow/unreachable
8383
network can't delay or block the dump.
84+
85+
## 5. `LLVM ERROR: Invalid usage of the XADD return value` on Older LLVM/BCC Builds
86+
87+
### The Problem
88+
On some machines, loading the BPF program aborted the whole process outright:
89+
```
90+
Loading BPF program... \ LLVM ERROR: line 3018: Invalid usage of the XADD return value
91+
Aborted
92+
```
93+
No Python exception was raised — the `try/except` around BPF initialization
94+
never fired, because this is a hard `SIGABRT` from inside LLVM itself (BCC
95+
embeds clang/LLVM in-process rather than shelling out, so a fatal LLVM error
96+
crashes the whole interpreter).
97+
98+
The cause was `prober.c`'s block-tracing code assigning a per-request id from
99+
`__sync_fetch_and_add(gen, 1)` against a single, shared (non-per-CPU)
100+
`BPF_ARRAY` counter. Because the *return value* of the atomic add was used
101+
(not discarded), clang had to lower it to a BPF `XADD` with the `BPF_FETCH`
102+
flag — a fetch-and-add variant that only newer LLVM BPF backends can legally
103+
emit. This project pins no LLVM/clang floor anywhere (only "Kernel: Linux
104+
5.4+" / "BCC: 0.18.0+" are documented, in `docs/TRACE_FORMAT.md`), and
105+
distros package BCC against whatever LLVM happens to be available, so the
106+
generated bytecode reliably crashed on older/other-distro toolchains while
107+
working fine on the machine used for development.
108+
109+
### The Solution
110+
The shared atomic counter was replaced with a `BPF_PERCPU_ARRAY` counter: each
111+
CPU bumps its own copy with a plain (non-atomic) read-increment, and
112+
`bpf_get_smp_processor_id()` is folded into the high bits of the id so ids
113+
stay unique across CPUs. This removes the fetch-returning atomic entirely, so
114+
the compiler no longer needs `BPF_FETCH`/XADD support at all — sidestepping
115+
the LLVM-version dependency rather than trying to detect and gate around it.
116+
Nothing downstream depended on strict global monotonic ordering of `req_id`,
117+
only on per-request uniqueness/correlation, so the change is behavior-neutral
118+
for consumers.

src/tracer/prober/prober.c

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -676,10 +676,17 @@ struct block_rq_key_t {
676676
BPF_TABLE("lru_hash", struct block_rq_key_t, struct block_issue_ctx, block_start_times, 65536); /**< Issue time + submitter, keyed by dev+sector */
677677
BPF_TABLE("lru_hash", struct block_rq_key_t, u64, block_insert_times, 65536); /**< Tracks block request insert time (queue latency) */
678678

679-
/* Monotonic generator for per-request IDs (see block_event.req_id). A single
680-
* u64 bumped atomically at issue time; lets consumers disambiguate repeated
681-
* I/O to the same (dev, sector) and correlate a request across its lifecycle. */
682-
BPF_ARRAY(block_req_id_gen, u64, 1);
679+
/* Per-CPU generator for per-request IDs (see block_event.req_id): each CPU
680+
* bumps its own counter (no cross-CPU race, so no atomic op is needed) and
681+
* the CPU id is folded into the high bits so ids stay unique across CPUs.
682+
* A shared counter bumped via __sync_fetch_and_add's return value was tried
683+
* first, but that lowers to a BPF XADD-with-fetch, which older/distro clang
684+
* BPF backends can't legally emit and crash on (LLVM ERROR: Invalid usage of
685+
* the XADD return value) -- see docs/COMPATIBILITY_FIXES.md #5. Consumers
686+
* only need per-CPU-monotonic ids to disambiguate repeated I/O to the same
687+
* (dev, sector) and correlate a request across its lifecycle; nothing relies
688+
* on strict global ordering across CPUs. */
689+
BPF_PERCPU_ARRAY(block_req_id_gen, u64, 1);
683690

684691
/* Block-tracing diagnostics counters (per-CPU, summed in userspace at exit):
685692
* [0] requests issued, [1] requests completed (emitted),
@@ -2884,8 +2891,10 @@ TRACEPOINT_PROBE(block, block_rq_issue) {
28842891
// Assign a monotonic per-request id, carried to the completion event.
28852892
u32 gen_key = 0;
28862893
u64 *gen = block_req_id_gen.lookup(&gen_key);
2887-
if (gen)
2888-
ictx.req_id = __sync_fetch_and_add(gen, 1);
2894+
if (gen) {
2895+
u64 local_id = (*gen)++;
2896+
ictx.req_id = ((u64)bpf_get_smp_processor_id() << 48) | (local_id & 0xFFFFFFFFFFFFULL);
2897+
}
28892898

28902899
block_start_times.update(&key, &ictx);
28912900

0 commit comments

Comments
 (0)