From 9394197e32eb261fdac20cafb5bf9d05fa765903 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Gr=C3=A9goire?= Date: Sat, 1 Aug 2026 11:03:29 -0400 Subject: [PATCH] all: make memory model saner --- src/kern/log_ring.zig | 2 +- src/kern/smr.zig | 10 +++--- src/kern/spinlock.zig | 3 +- src/platform/pc/smp.zig | 2 +- src/rtl/barrier.zig | 73 +++++++++++++++++------------------------ src/rtl/seqlock.zig | 4 +-- 6 files changed, 42 insertions(+), 52 deletions(-) diff --git a/src/kern/log_ring.zig b/src/kern/log_ring.zig index e4705e2..49e1589 100644 --- a/src/kern/log_ring.zig +++ b/src/kern/log_ring.zig @@ -220,7 +220,7 @@ pub fn RingBuffer(data_size_bits: usize, avg_msg_bits: usize) type { result.seq = info.sequence; // Ensure the copy is finished before the state load. - rtl.barrier.rmb(); + rtl.barrier.fence(.acquire); state_var = desc.state.load(.acquire); result.state = get_desc_state(id, state_var); diff --git a/src/kern/smr.zig b/src/kern/smr.zig index d83303d..80c51a0 100644 --- a/src/kern/smr.zig +++ b/src/kern/smr.zig @@ -79,7 +79,7 @@ pub var full_scans: std.atomic.Value(usize) = .init(0); /// Scan all CPUs and return the minimum observed value. /// If `should_wait` is true, this will spinloop (or block) until all CPUs have reached the given goal. fn scan(dom: *Domain, goal: Sequence, clock: Clock, should_wait: bool) Sequence { - rtl.barrier.mb(); + rtl.barrier.fence(.seq_cst); const clk_write = clock.write_seq.raw; const clk_read = clock.read_seq.raw; @@ -125,7 +125,7 @@ fn scan(dom: *Domain, goal: Sequence, clock: Clock, should_wait: bool) Sequence } if (dom.preempt) { - rtl.barrier.mb(); + rtl.barrier.fence(.seq_cst); // Preemption is enabled, check stalled readers. for (0..ke.ncpus) |i| { @@ -181,7 +181,7 @@ fn scan(dom: *Domain, goal: Sequence, clock: Clock, should_wait: bool) Sequence // (1) is done by the acquire fence below, and (2) is done by the release // success ordering in the CAS. This pairs with the load acquire in poll(). // It would also be valid to relax the CAS and have a full fence here. - rtl.barrier.acq(); + rtl.barrier.fence(.acquire); // Advance the global read sequence. var dom_rd_seq = dom.clock.read_seq.load(.acquire); @@ -259,7 +259,7 @@ pub fn advance(dom: *Domain) Sequence { /// the calling thread are visible. The global clock isn't advanced unlike `advance`, /// it is only when commit() is called. pub fn deferred_advance(dom: *Domain) Sequence { - rtl.barrier.mb(); + rtl.barrier.fence(.seq_cst); return dom.clock.write_seq.load(.monotonic) + seq_incr; } @@ -296,7 +296,7 @@ fn enter_internal(dom: *Domain) Sequence { _ = cpu.current_seq.fetchAdd(wr_seq, .seq_cst); } else { cpu.current_seq.store(wr_seq, .monotonic); - rtl.barrier.mb(); + rtl.barrier.fence(.seq_cst); } return wr_seq; diff --git a/src/kern/spinlock.zig b/src/kern/spinlock.zig index 5ce441c..a91337e 100644 --- a/src/kern/spinlock.zig +++ b/src/kern/spinlock.zig @@ -264,7 +264,8 @@ pub const QSpinLock = LockTemplate(struct { } // Store cpu + 1 so that we can distinguish from an empty tail field. - rtl.barrier.wmb(); + rtl.barrier.fence(.release); + const tail: u16 = @bitCast(Tail{ .cpu = @intCast(ke.cpu.current() + 1), .idx = @intCast(idx) }); const old: Tail = @bitCast(self.data.split.tail.swap(tail, .acq_rel)); diff --git a/src/platform/pc/smp.zig b/src/platform/pc/smp.zig index ab7ca92..e9c968f 100644 --- a/src/platform/pc/smp.zig +++ b/src/platform/pc/smp.zig @@ -113,7 +113,7 @@ pub fn init() linksection(r.init) void { start_thread.remote(@intCast(cpu_id)).* = make_thread(@ptrCast(&ap_entry), stack_top - r.kib(32)); - rtl.barrier.wmb(); + rtl.barrier.fence(.release); // Send the INIT-SIPI-SIPI sequence to start the AP. apic.send_init(apic_id); diff --git a/src/rtl/barrier.zig b/src/rtl/barrier.zig index 19b253c..f0f64a6 100644 --- a/src/rtl/barrier.zig +++ b/src/rtl/barrier.zig @@ -4,51 +4,24 @@ const std = @import("std"); const builtin = @import("builtin"); -/// Read Memory Barrier (Load-Load) -/// Ensures all loads before the barrier complete before any loads after it. -pub inline fn rmb() void { - switch (builtin.cpu.arch) { - .x86, .x86_64 => { - asm volatile ("" ::: .{ .memory = true }); - }, - .aarch64 => { - asm volatile ("dmb ishld" ::: .{ .memory = true }); - }, - .arm => { - asm volatile ("dmb ish" ::: .{ .memory = true }); - }, - .riscv64, .riscv32 => { - asm volatile ("fence r,r" ::: .{ .memory = true }); - }, - else => @compileError("unsupported architecture"), +pub inline fn fence(comptime ordering: std.builtin.AtomicOrder) void { + switch (ordering) { + .seq_cst, .acq_rel => mb(), + .acquire => acq(), + .release => rel(), + else => @compileError("Unsupported ordering"), } } -/// Write Memory Barrier (Store-Store) -/// Ensures all stores before the barrier complete before any stores after it. -pub inline fn wmb() void { +// Full memory barrier +inline fn mb() void { switch (builtin.cpu.arch) { - .x86, .x86_64 => { - asm volatile ("" ::: .{ .memory = true }); + .x86_64 => { + // Faster than mfence + asm volatile ("lock addl $0, (%%rsp)" ::: .{ .memory = true }); }, - .aarch64 => { - asm volatile ("dmb ishst" ::: .{ .memory = true }); - }, - .arm => { - asm volatile ("dmb ish" ::: .{ .memory = true }); - }, - .riscv64, .riscv32 => { - asm volatile ("fence w,w" ::: .{ .memory = true }); - }, - else => @compileError("unsupported architecture"), - } -} - -/// Full Memory Barrier (Load/Store - Load/Store) -pub inline fn mb() void { - switch (builtin.cpu.arch) { - .x86, .x86_64 => { - asm volatile ("mfence" ::: .{ .memory = true }); + .x86 => { + asm volatile ("lock addl $0, (%%esp)" ::: .{ .memory = true }); }, .aarch64, .arm => { asm volatile ("dmb ish" ::: .{ .memory = true }); @@ -60,8 +33,8 @@ pub inline fn mb() void { } } -/// Acquire Barrier (Load/Load - Load/Store) -pub inline fn acq() void { +// Acquire barrier +inline fn acq() void { switch (builtin.cpu.arch) { .x86, .x86_64 => { asm volatile ("" ::: .{ .memory = true }); @@ -76,6 +49,22 @@ pub inline fn acq() void { } } +// Release barrier +inline fn rel() void { + switch (builtin.cpu.arch) { + .x86, .x86_64 => { + asm volatile ("" ::: .{ .memory = true }); + }, + .aarch64, .arm => { + asm volatile ("dmb ish" ::: .{ .memory = true }); + }, + .riscv64, .riscv32 => { + asm volatile ("fence rw,w" ::: .{ .memory = true }); + }, + else => @compileError("unsupported architecture"), + } +} + /// Copy to dest from src with atomic loads. /// Both addresses need to be aligned. pub fn atomic_load_memcpy(dest: anytype, src: anytype, comptime ordering: std.builtin.AtomicOrder) void { diff --git a/src/rtl/seqlock.zig b/src/rtl/seqlock.zig index bcf0478..653ef9f 100644 --- a/src/rtl/seqlock.zig +++ b/src/rtl/seqlock.zig @@ -26,7 +26,7 @@ pub fn SeqLock(comptime T: type) type { rtl.barrier.atomic_load_memcpy(&ret, &self.raw, .monotonic); // Ensure the sequence read happens *after* the data is fully loaded. - rtl.barrier.rmb(); + rtl.barrier.fence(.acquire); if (self.sequence.load(.monotonic) == seq) break; @@ -43,7 +43,7 @@ pub fn SeqLock(comptime T: type) type { self.sequence.store(seq + 1, .monotonic); // Ensure the data write happens *after* the sequence is incremented. - rtl.barrier.wmb(); + rtl.barrier.fence(.release); // Relaxed store memcpy is needed here since data races are UB. rtl.barrier.atomic_store_memcpy(&self.raw, &data, .monotonic);