server: replace the VRAM doorbell with a want-lock
The doorbell was edge-triggered: a waiter wrote <arena>/doorbell/<pid> and the holder's warden turned the inotify event into a queue flag. A ring carried no notion of "still wanted", so it was lost whenever the holder was not already warm and listening - during its model load (the watch does not exist yet, and the kernel does not queue events for a watch that is not there) or inside restore_device - and any ring that survived past the point its sender had been satisfied caused a spurious release. Lost rings wedged the waiter permanently, because flock(LOCK_EX) never times out and, with idle-sleep disabled, the holder had no other reason to release. The wait blocks under mutex_tasks, so the whole server stopped answering while /health still returned 200. Express the request as kernel state instead. A waiter holds <arena>/want.lock shared while it waits and drops it once it owns the token; the sleep decision probes that lock non-blocking and releases the GPU while anyone is waiting. The probe needs its own fd - flock treats two open file descriptions of one file independently, so probing on the waiter's fd would convert our own lock rather than conflict with it. Nothing can be missed, nothing goes stale, and a waiter that dies is cleaned up by the kernel. Probe from should_sleep() on the loop thread rather than from a warden thread. Routing it through a flag is what made the first attempts fail: start_loop() holds mutex_tasks from should_sleep() through the callbacks to the wait, so a warden's request_yield() blocks on that mutex and is admitted only after the flag has been consumed, latching a release for the next wake. Reading live state where the decision is made has no edge to latch, and drops the warden's poll latency. Two sleep-path bugs this exposed: wait_until_no_sleep() waited on !sleeping but the loop clears req_stop_sleeping on the way in, so a loop that slept again before the waiter ran stranded it forever - re-ask on every wake. And a task queued after the waiter saw us awake could not wake us by itself, so sleep now also breaks on a non-empty queue. Hold the GPU for 100 ms after a wake: the request that woke us is not queued yet, and yielding at once only sends it round again. Measured on the RX 580 pod, two servers contending, 60 alternating handoffs: 0 stranded, median 0.309 s against the doorbell's 0.314 s. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PZz44SLQvTXMyWGio6t9DZ
This commit is contained in:
@@ -114,27 +114,19 @@ void server_queue::pop_deferred_task(int id_slot) {
|
||||
|
||||
void server_queue::wait_until_no_sleep() {
|
||||
std::unique_lock<std::mutex> lock(mutex_tasks);
|
||||
if (!sleeping) {
|
||||
return;
|
||||
} else {
|
||||
// re-ask on every wake: the loop clears req_stop_sleeping on the way into sleep, so a loop that
|
||||
// goes back to sleep before we run would strand us here forever
|
||||
while (sleeping) {
|
||||
if (!req_stop_sleeping) {
|
||||
QUE_DBG("%s", "requesting to stop sleeping\n");
|
||||
req_stop_sleeping = true;
|
||||
condition_tasks.notify_one(); // only main thread is waiting on this
|
||||
condition_tasks.notify_all(); // other threads may wait on this too
|
||||
}
|
||||
QUE_DBG("%s", "waiting until no sleep\n");
|
||||
condition_tasks.wait(lock, [&]{
|
||||
return !sleeping;
|
||||
});
|
||||
condition_tasks.wait(lock);
|
||||
}
|
||||
}
|
||||
|
||||
void server_queue::request_yield() {
|
||||
std::unique_lock<std::mutex> lock(mutex_tasks);
|
||||
yield_requested = true;
|
||||
condition_tasks.notify_all();
|
||||
}
|
||||
|
||||
void server_queue::terminate() {
|
||||
std::unique_lock<std::mutex> lock(mutex_tasks);
|
||||
running = false;
|
||||
@@ -292,11 +284,17 @@ void server_queue::start_loop(int64_t idle_sleep_ms) {
|
||||
worker.yielding = false;
|
||||
worker.thread = std::thread([this]() { worker_loop(); });
|
||||
|
||||
constexpr auto max_wait_time = std::chrono::seconds(1);
|
||||
// the arbiter predicate is read from live state, so this timeout bounds how fast we notice a
|
||||
// process waiting for the VRAM token
|
||||
const auto max_wait_time = should_yield_cb ? std::chrono::milliseconds(10) : std::chrono::milliseconds(1000);
|
||||
// after a wake, keep the VRAM for at least this long: the request that woke us is not in the
|
||||
// queue yet, so yielding at once would only send it through another wake
|
||||
constexpr int64_t yield_grace_ms = 100;
|
||||
int64_t time_last_wake = 0;
|
||||
auto should_sleep = [&]() -> bool {
|
||||
// caller must hold mutex_tasks
|
||||
if (yield_requested) {
|
||||
return true; // another process rang the VRAM doorbell - release now
|
||||
if (should_yield_cb && ggml_time_ms() - time_last_wake >= yield_grace_ms && should_yield_cb()) {
|
||||
return true; // another process waits for the VRAM token - release now
|
||||
}
|
||||
if (idle_sleep_ms < 0) {
|
||||
return false;
|
||||
@@ -336,15 +334,16 @@ void server_queue::start_loop(int64_t idle_sleep_ms) {
|
||||
if (should_sleep()) {
|
||||
QUE_INF("%s", "entering sleeping state\n");
|
||||
sleeping = true;
|
||||
yield_requested = false; // consumed
|
||||
// Call order cb0 -> cb1 -> cb{N}
|
||||
for (auto & cb : callback_sleeping_state) {
|
||||
cb(true);
|
||||
}
|
||||
req_stop_sleeping = false;
|
||||
// wait until we are requested to exit sleeping state
|
||||
// wait until we are requested to exit sleeping state, or a task arrives: post() only
|
||||
// notifies, so a task queued right after wait_until_no_sleep() saw us awake must be
|
||||
// able to wake us by itself, else it waits here for an unrelated request
|
||||
condition_tasks.wait(lock, [&]{
|
||||
return (!running || req_stop_sleeping);
|
||||
return (!running || req_stop_sleeping || !queue_tasks.empty());
|
||||
});
|
||||
if (!running) { // may changed during sleep
|
||||
break; // terminate
|
||||
@@ -357,12 +356,13 @@ void server_queue::start_loop(int64_t idle_sleep_ms) {
|
||||
}
|
||||
sleeping = false;
|
||||
time_last_task = ggml_time_ms();
|
||||
time_last_wake = time_last_task;
|
||||
condition_tasks.notify_all(); // notify wait_until_no_sleep()
|
||||
break; // process new tasks
|
||||
} else {
|
||||
// wait for new tasks, a VRAM yield request, or timeout for checking sleeping condition
|
||||
// wait for new tasks, or timeout for checking sleeping condition
|
||||
bool res = condition_tasks.wait_for(lock, max_wait_time, [&]{
|
||||
return (!queue_tasks.empty() || !running || yield_requested);
|
||||
return (!queue_tasks.empty() || !running);
|
||||
});
|
||||
if (res && !queue_tasks.empty()) {
|
||||
break; // new task arrived or terminate
|
||||
@@ -370,7 +370,7 @@ void server_queue::start_loop(int64_t idle_sleep_ms) {
|
||||
if (!running) {
|
||||
break;
|
||||
}
|
||||
// otherwise (timeout or yield request), loop again to re-check should_sleep
|
||||
// otherwise (timeout), loop again to re-check should_sleep
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user