kernel: contain a fatal fault — name the task, release the BKL before halt
Two gaps a real-hardware crash exposed, both in onException's terminal path (and the panic path): - The report was anonymous. Add the faulting task's id + name and whether it trapped in ring 3 (a user process) or ring 0 (the trusted base) — so a fatal fault says WHAT crashed and WHERE, not just the vector. scheduler gains currentIdSafe/currentNameSafe (early-boot-guarded, like currentCpuIndex, so the reporter can't fault a second time). - halt() never released the big kernel lock, so a core that died holding it deadlocked every other core spinning in acquire() — the whole machine hangs, not just the one core the design promises. The BKL now records its owner (architecture.cpuLocal(), a unique per-core token); sync.releaseIfHeldHere() frees the lock only if this core holds it, called before halt on both fatal paths. Caveat: if we held it mid-mutation the shared state may be inconsistent, but letting the other cores + the supervisor keep running is strictly more recoverable than a guaranteed total hang.
This commit is contained in:
parent
1e80c57484
commit
e1605e3235
|
|
@ -9,6 +9,7 @@ const wall_clock = @import("wall-clock.zig");
|
|||
const pmm = @import("pmm.zig");
|
||||
const heap = @import("heap.zig");
|
||||
const scheduler = @import("scheduler.zig");
|
||||
const sync = @import("sync.zig");
|
||||
const process = @import("process.zig");
|
||||
const devices_broker = @import("devices-broker.zig");
|
||||
const irq = @import("irq.zig");
|
||||
|
|
@ -508,6 +509,10 @@ fn onException(state: *const architecture.CpuState) noreturn {
|
|||
// console back on even if a display service was holding the framebuffer — on top of the
|
||||
// diagnostic log.
|
||||
fatalPrint("\nCPU EXCEPTION on core {d}: {s} (vector {d})\n", .{ core, architecture.exceptionName(state.vector), state.vector });
|
||||
// Name the culprit: which task, and whether it faulted in ring 3 (a process the
|
||||
// kernel would normally kill — landing here means it had no address space) or ring 0
|
||||
// (the trusted base itself). Without this the fatal report is anonymous.
|
||||
fatalPrint(" task : {d} ({s}), {s}\n", .{ scheduler.currentIdSafe(), scheduler.currentNameSafe(), if (architecture.fromUser(state)) "ring 3 (user)" else "ring 0 (kernel)" });
|
||||
fatalPrint(" error code : 0x{x}\n", .{state.error_code});
|
||||
fatalPrint(" IP : 0x{x:0>16}\n", .{architecture.instructionPointer(state)});
|
||||
fatalPrint(" SP : 0x{x:0>16}\n", .{architecture.stackPointer(state)});
|
||||
|
|
@ -515,6 +520,10 @@ fn onException(state: *const architecture.CpuState) noreturn {
|
|||
|
||||
var buffer: [128]u8 = undefined;
|
||||
log.recordPanic(std.fmt.bufPrint(&buffer, "CPU exception {s} (vector {d}) on core {d} at IP 0x{x}", .{ architecture.exceptionName(state.vector), state.vector, core, architecture.instructionPointer(state) }) catch "cpu exception");
|
||||
// Free the BKL if this core held it (a kernel-mode fault, or a nested fault in the
|
||||
// recovery teardown), so halting this one core doesn't deadlock every other core on
|
||||
// the lock. Only that core stops; the rest — and the supervisor — keep running.
|
||||
sync.releaseIfHeldHere();
|
||||
architecture.halt();
|
||||
}
|
||||
|
||||
|
|
@ -529,6 +538,8 @@ pub const panic = std.debug.FullPanic(struct {
|
|||
fatal("\nKERNEL PANIC: "); // a panic outranks any display service holding the screen
|
||||
fatal(message);
|
||||
fatal("\n");
|
||||
fatalPrint(" task : {d} ({s})\n", .{ scheduler.currentIdSafe(), scheduler.currentNameSafe() });
|
||||
sync.releaseIfHeldHere(); // don't deadlock the other cores on the lock we may hold
|
||||
architecture.halt();
|
||||
}
|
||||
}.panic);
|
||||
|
|
|
|||
|
|
@ -791,6 +791,21 @@ pub fn currentCpuIndex() u32 {
|
|||
return thisCpu().index;
|
||||
}
|
||||
|
||||
/// The running task's id, or 0 if this core's scheduler isn't up yet (early boot, no GS
|
||||
/// base). Safe for a fault reporter to call unconditionally — like `currentCpuIndex`,
|
||||
/// it never dereferences an unpublished per-CPU pointer and so can't fault a second time.
|
||||
pub fn currentIdSafe() u32 {
|
||||
if (architecture.cpuLocal() == 0) return 0;
|
||||
return thisCpu().current.id;
|
||||
}
|
||||
|
||||
/// The running task's name (argv[0]), or "" if this core's scheduler isn't up yet.
|
||||
/// The companion to `currentIdSafe` for naming the culprit in a fatal fault report.
|
||||
pub fn currentNameSafe() []const u8 {
|
||||
if (architecture.cpuLocal() == 0) return "";
|
||||
return thisCpu().current.name();
|
||||
}
|
||||
|
||||
/// Change the running task's priority (takes effect next time it's enqueued).
|
||||
pub fn setPriority(p: Priority) void {
|
||||
current().priority = p;
|
||||
|
|
|
|||
|
|
@ -33,6 +33,13 @@ const architecture = @import("architecture");
|
|||
/// 0 = free, 1 = held. A single global lock for the whole kernel.
|
||||
var held = std.atomic.Value(u32).init(0);
|
||||
|
||||
/// The per-CPU base pointer (`architecture.cpuLocal()`) of the core currently holding
|
||||
/// the lock, or 0 when free. Metadata only — `held` is what enforces exclusion — read
|
||||
/// solely by `releaseIfHeldHere` on the fatal-fault path. `cpuLocal()` is a unique,
|
||||
/// architecture-level token per core (0 before this core's GS base is published, which
|
||||
/// is fine: that window is single-core early boot, where no other core can deadlock).
|
||||
var owner = std.atomic.Value(usize).init(0);
|
||||
|
||||
/// Enter the kernel: disable interrupts on this core, then spin until we own the
|
||||
/// lock. Returns the caller's prior interrupt flags for `leave` to restore.
|
||||
/// Interrupts stay off for the whole critical section so this core's timer tick
|
||||
|
|
@ -67,14 +74,29 @@ export fn releaseForFreshTask() callconv(.c) void {
|
|||
release();
|
||||
}
|
||||
|
||||
/// Release the big kernel lock **only if this core is the one holding it** — a no-op
|
||||
/// otherwise. For the fatal-fault path (a kernel-mode fault, or a nested fault inside the
|
||||
/// recovery teardown, both of which run under the lock): a core that dies holding the BKL
|
||||
/// must free it, or every other core spins forever in `acquire` and the whole machine
|
||||
/// deadlocks instead of just that core stopping. It must NOT free a lock another core
|
||||
/// owns, hence the owner check. Caveat: if we held it mid-mutation the shared state may be
|
||||
/// inconsistent — but letting the other cores (and the supervisor) run on possibly-degraded
|
||||
/// state is strictly more recoverable than a guaranteed total hang.
|
||||
pub fn releaseIfHeldHere() void {
|
||||
const me = architecture.cpuLocal();
|
||||
if (me != 0 and owner.load(.monotonic) == me) release();
|
||||
}
|
||||
|
||||
fn acquire() void {
|
||||
// Test-and-test-and-set: try once, then spin read-only until the lock looks
|
||||
// free before retrying the (bus-locked) swap — cheaper on the coherency fabric.
|
||||
while (held.swap(1, .acquire) != 0) {
|
||||
while (held.load(.monotonic) != 0) architecture.cpuRelax();
|
||||
}
|
||||
owner.store(architecture.cpuLocal(), .monotonic);
|
||||
}
|
||||
|
||||
fn release() void {
|
||||
owner.store(0, .monotonic);
|
||||
held.store(0, .release);
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue