Skip to content

Make boottime and hz access atomic - #393

Open
nicholasberlin wants to merge 1 commit into
mainfrom
boottime-atomics
Open

Make boottime and hz access atomic#393
nicholasberlin wants to merge 1 commit into
mainfrom
boottime-atomics

Conversation

@nicholasberlin

@nicholasberlin nicholasberlin commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

quark.boottime and quark.hz live in a process-wide global shared by every queue. quark_update_boottime() may be called from one thread while another converts timestamps through quark_time_to_wallclock() — as auditbeat does with two independent quark consumers in the same process — which no caller-side lock can span. By the letter of C11 the plain u64 loads and stores are a data race.

Route all access through static inline helpers that use __atomic_load_n/__atomic_store_n with relaxed ordering. Relaxed is sufficient since boottime is a single self-contained value with no ordering dependency on other data; readers just need an untorn value and tolerate a stale epoch for one cycle.

To be clear about what this does and doesn't change: the compiled code is identical before and after — on x86-64 and aarch64 an aligned 8-byte access is already a single atomic mov/ldr/str, the helpers inline away, and there is no libatomic dependency (nm clean). Nothing misbehaves today and this fixes no observed bug. What the change buys:

  • It turns "correct by ISA coincidence and current compiler behavior" into "correct by contract". Plain access to a shared object licenses the compiler to cache, hoist or split it; today nothing bites only because every access sits behind an opaque call boundary. LTO, a future compiler, or an innocent refactor could change that silently, and the failure mode would be a single garbage timestamp during an NTP step — unreproducible.
  • It makes the documented contract satisfiable. quark(7) says state must be synchronized by the user, but the epoch spans consumers that don't know about each other, so no user lock can exist. With this change the three calls can be documented as thread-safe from any thread, which Document the boottime calls #388 now does.
  • The C side becomes TSan-clean, and the helpers plus struct comment make the sharing model explicit for the next reader instead of leaving it to be rediscovered.
  • On any 32-bit target a u64 store really is two instructions, so the tear would be real there. Not our platforms today; permanently a non-issue after this.

Also verified to build on centos7 gcc 4.8.5 — the __atomic builtins date to gcc 4.7, whereas C11 _Atomic/stdatomic.h would need 4.9, hence the builtins.

Must merge before #388, which documents the thread-safety this change provides.

@nicholasberlin
nicholasberlin requested a review from a team as a code owner August 14, 2026 17:02
@nicholasberlin
nicholasberlin force-pushed the boottime-atomics branch 2 times, most recently from d753d21 to 74ca4af Compare August 14, 2026 19:52
quark.boottime and quark.hz live in a process-wide global shared by
every queue. quark_update_boottime() may be called from one thread
while another converts timestamps through quark_time_to_wallclock(),
as auditbeat does with two independent quark consumers in the same
process, which no caller-side lock can span. The plain u64 loads and
stores were a data race.

Route all access through static inline helpers that use
__atomic_load_n/__atomic_store_n with relaxed ordering. Relaxed is
sufficient since boottime is a single self-contained value with no
ordering dependency on other data; readers just need an untorn value
and tolerate a stale epoch for one cycle. On x86-64 and aarch64 the
generated code is unchanged (plain mov/ldr/str, no libatomic), and the
builtins compile back to gcc 4.8.5 on centos7.

quark_update_boottime(), quark_get_boottime() and
quark_time_to_wallclock() are now safe to call from any thread.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant