mirror of
https://github.com/rdmsr/zag.git
synced 2026-10-27 13:58:20 +00:00
ke/sched: fix lock inversion in scheduler
Taking next's lock can be a lock inversion with the turnstile code. Use a switching field instead and spin on that.
This commit is contained in:
@@ -25,7 +25,7 @@ const ThreadFrame = extern struct {
|
|||||||
|
|
||||||
extern fn asm_thread_entry() void;
|
extern fn asm_thread_entry() void;
|
||||||
|
|
||||||
extern fn do_context_switch(old: *ThreadContext, new: *ThreadContext, lock: *u8) callconv(.c) void;
|
extern fn do_context_switch(old: *ThreadContext, new: *ThreadContext, lock: *u8, switching: *bool) callconv(.c) void;
|
||||||
|
|
||||||
pub const ThreadContext = extern struct {
|
pub const ThreadContext = extern struct {
|
||||||
rsp: u64 align(1),
|
rsp: u64 align(1),
|
||||||
@@ -60,7 +60,7 @@ pub const ThreadContext = extern struct {
|
|||||||
|
|
||||||
pub fn switch_to(self: *ThreadContext, new: *ThreadContext) callconv(.c) void {
|
pub fn switch_to(self: *ThreadContext, new: *ThreadContext) callconv(.c) void {
|
||||||
const thread: *ke.Thread = @alignCast(@fieldParentPtr("context", self));
|
const thread: *ke.Thread = @alignCast(@fieldParentPtr("context", self));
|
||||||
do_context_switch(self, new, &thread.lock.inner.locked.raw);
|
do_context_switch(self, new, &thread.lock.inner.locked.raw, &thread.switching.raw);
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|||||||
@@ -149,9 +149,13 @@ do_context_switch:
|
|||||||
/* Load new stack pointer. */
|
/* Load new stack pointer. */
|
||||||
mov (%rsi), %rsp
|
mov (%rsi), %rsp
|
||||||
|
|
||||||
|
/* Set switching to false. */
|
||||||
|
xorl %eax, %eax
|
||||||
|
xchgb %al, (%rcx)
|
||||||
|
|
||||||
/* Unlock passed lock. */
|
/* Unlock passed lock. */
|
||||||
xorl %ecx, %ecx
|
xorl %eax, %eax
|
||||||
xchgb %cl, (%rdx)
|
xchgb %al, (%rdx)
|
||||||
|
|
||||||
/* Restore callee-saved registers. */
|
/* Restore callee-saved registers. */
|
||||||
pop %rbp
|
pop %rbp
|
||||||
|
|||||||
+11
-5
@@ -355,6 +355,8 @@ pub fn handle_preemption(cpu: *PerCpu) void {
|
|||||||
cpu.queues_lock.release_no_ipl();
|
cpu.queues_lock.release_no_ipl();
|
||||||
cur.lock.acquire_no_ipl();
|
cur.lock.acquire_no_ipl();
|
||||||
|
|
||||||
|
cur.switching.store(true, .monotonic);
|
||||||
|
|
||||||
if (cur != cpu.idle_thread and cur.state.load(.monotonic) != .Blocked) {
|
if (cur != cpu.idle_thread and cur.state.load(.monotonic) != .Blocked) {
|
||||||
// Put it back in the queue.
|
// Put it back in the queue.
|
||||||
cpu.queues_lock.acquire_no_ipl();
|
cpu.queues_lock.acquire_no_ipl();
|
||||||
@@ -363,8 +365,9 @@ pub fn handle_preemption(cpu: *PerCpu) void {
|
|||||||
cpu.queues_lock.release_no_ipl();
|
cpu.queues_lock.release_no_ipl();
|
||||||
}
|
}
|
||||||
|
|
||||||
next.lock.acquire_no_ipl();
|
while (next.switching.load(.monotonic) == true) {
|
||||||
next.lock.release_no_ipl();
|
std.atomic.spinLoopHint();
|
||||||
|
}
|
||||||
|
|
||||||
do_switch(cpu, cur, next);
|
do_switch(cpu, cur, next);
|
||||||
|
|
||||||
@@ -435,6 +438,7 @@ pub fn yield() void {
|
|||||||
const td = percpu.local().current_thread.?;
|
const td = percpu.local().current_thread.?;
|
||||||
|
|
||||||
td.lock.acquire_no_ipl();
|
td.lock.acquire_no_ipl();
|
||||||
|
td.switching.store(true, .monotonic);
|
||||||
|
|
||||||
const cpu = percpu.local();
|
const cpu = percpu.local();
|
||||||
if (td != cpu.idle_thread and td.state.load(.monotonic) == .Running) {
|
if (td != cpu.idle_thread and td.state.load(.monotonic) == .Running) {
|
||||||
@@ -512,14 +516,16 @@ pub fn yield_locked() void {
|
|||||||
sched_cpu.queues_lock.release_no_ipl();
|
sched_cpu.queues_lock.release_no_ipl();
|
||||||
|
|
||||||
if (next != null and cur.? != next.?) {
|
if (next != null and cur.? != next.?) {
|
||||||
// Switch into the thread. Take its lock here to ensure it is not still on its stack.
|
// Switch into the thread. Ensure it's not switching.
|
||||||
next.?.lock.acquire_no_ipl();
|
while (next.?.switching.load(.monotonic) == true) {
|
||||||
next.?.lock.release_no_ipl();
|
std.atomic.spinLoopHint();
|
||||||
|
}
|
||||||
|
|
||||||
do_switch(sched_cpu, cur.?, next.?);
|
do_switch(sched_cpu, cur.?, next.?);
|
||||||
} else {
|
} else {
|
||||||
// Ensure curthread is not marked as selected.
|
// Ensure curthread is not marked as selected.
|
||||||
cur.?.state.store(.Running, .monotonic);
|
cur.?.state.store(.Running, .monotonic);
|
||||||
|
cur.?.switching.store(false, .monotonic);
|
||||||
cur.?.lock.release_no_ipl();
|
cur.?.lock.release_no_ipl();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -114,6 +114,10 @@ pub const Thread = struct {
|
|||||||
stack: r.VAddr,
|
stack: r.VAddr,
|
||||||
/// PELT load average,
|
/// PELT load average,
|
||||||
avg: ki.sched.Average,
|
avg: ki.sched.Average,
|
||||||
|
/// Set whenever the thread is switching off its stack.
|
||||||
|
/// This is used to avoid taking thread next lock to wait for switch off
|
||||||
|
/// to complete.
|
||||||
|
switching: std.atomic.Value(bool),
|
||||||
|
|
||||||
/// Initialize a thread.
|
/// Initialize a thread.
|
||||||
/// - `stack`: Address of the **base** of the stack on which the initial context for the thread is built
|
/// - `stack`: Address of the **base** of the stack on which the initial context for the thread is built
|
||||||
@@ -149,6 +153,7 @@ pub const Thread = struct {
|
|||||||
.queue = null,
|
.queue = null,
|
||||||
.queue_item = null,
|
.queue_item = null,
|
||||||
.stack = stack,
|
.stack = stack,
|
||||||
|
.switching = .init(false),
|
||||||
.avg = .{
|
.avg = .{
|
||||||
.last_update = 0,
|
.last_update = 0,
|
||||||
// New threads are considered heavy until they prove themselves,
|
// New threads are considered heavy until they prove themselves,
|
||||||
|
|||||||
Reference in New Issue
Block a user