From 574d9855ce460887c8f4f9d6274753f9046f4b57 Mon Sep 17 00:00:00 2001 From: Mitchell-Johnson Date: Tue, 10 Nov 2026 18:42:13 -0800 Subject: [PATCH] GPU: a slow read is late, not stalled; a stall clears when the read returns nvidia-smi on a busy card can take past the 3 s deadline, which latched the card as stalled until restart. A late read now carries the last readings and is used when it lands; no second read starts while one is out; only one out for 15 s (past both nvidia-smi timeouts) marks the card stalled, and its return clears it. Co-Authored-By: Claude Opus 5.5 --- internal/metrics/gpu.go | 82 ++++++++++++++++++++++-------- internal/metrics/gpu_stall_test.go | 52 +++++++++++++++++++ web/src/Top.tsx | 4 +- 3 files changed, 116 insertions(+), 22 deletions(-) create mode 100644 internal/metrics/gpu_stall_test.go diff --git a/internal/metrics/gpu.go b/internal/metrics/gpu.go index 30560e9..bdf7006 100644 --- a/internal/metrics/gpu.go +++ b/internal/metrics/gpu.go @@ -56,7 +56,7 @@ type GPU struct { MemTotal uint64 `json:"mem_total,omitempty"` Clients []GPUClient `json:"clients"` ClientsOK bool `json:"clients_ok"` // false when fdinfo of other users' processes could not be read - Stalled bool `json:"stalled,omitempty"` // a sysfs read of this card never returned; its readings are paused until restart + Stalled bool `json:"stalled,omitempty"` // a read of this card has been out for gpuStallAfter; its readings are paused until it returns } // GPUClient is one process with the device open. @@ -78,7 +78,17 @@ type gpuDev struct { integrated, stalled bool prevIdle, prevEnergy float64 prevAt time.Time - nv []GPUClient // nvidia: the process table from the last read (pmon), fdinfo says nothing there + nv []GPUClient // nvidia: the process table from the last read (pmon), fdinfo says nothing there + nvRead []GPUClient // the same, as the read in flight left it: only that read touches it + pending chan gpuRead // the read in flight, nil when none is + pendingAt time.Time + last *GPU // the last read that came back + readFn func(*GPU, time.Time) // nil is read (tests stand a slow one in) +} + +type gpuRead struct { + x GPU + nv []GPUClient } type gpuState struct { @@ -348,28 +358,60 @@ func lspciName(desc string) string { return strings.TrimSpace(vendor + " " + desc) } -// readWithin is read with a deadline. A sysfs read that does not come back must not take -// the whole sampler with it — every graph on every page of this machine stops — so the -// device is marked stalled, its goroutine abandoned, and the rest of the sample goes on. -const gpuReadWithin = 3 * time.Second +// readWithin is read with a deadline. A read that does not come back must not take the +// whole sampler with it — every graph on every page of this machine stops — so one still out +// after gpuReadWithin is left running, the sample goes on with the card's last readings, and +// no second read starts until it returns. Late is normal for nvidia-smi on a busy card; only a +// read out for gpuStallAfter marks the card stalled, and that clears the moment it returns. A +// sysfs read that never returns keeps it stalled, at the cost of one goroutine. +var ( + gpuReadWithin = 3 * time.Second + gpuStallAfter = 15 * time.Second // past both of readNvidia's nvidia-smi calls timing out (runOut, 5 s each) +) func (d *gpuDev) readWithin(x *GPU, now time.Time) { - if d.stalled { - x.Stalled = true + var r gpuRead + back := false + if d.pending == nil { + done := make(chan gpuRead, 1) + y := *x + read := d.read + if d.readFn != nil { + read = d.readFn + } + go func() { + read(&y, now) + done <- gpuRead{y, d.nvRead} + }() + d.pending, d.pendingAt = done, now + select { + case r, back = <-done: + case <-time.After(gpuReadWithin): + } + } else { // started on an earlier sample: taken if it is back, never waited on again + select { + case r, back = <-d.pending: + default: + } + } + ok := x.ClientsOK + defer func() { x.ClientsOK = ok }() + if back { + if d.stalled { + log.Printf("gpu: %s (%s): its read came back after %s; readings resume", d.name, d.card, now.Sub(d.pendingAt).Round(time.Second)) + } + d.pending, d.nv, d.stalled, d.last = nil, r.nv, false, &r.x + *x = r.x return } - done := make(chan GPU, 1) - go func() { - y := *x - d.read(&y, now) - done <- y - }() - select { - case y := <-done: - *x = y - case <-time.After(gpuReadWithin): + if d.last != nil { + *x = *d.last + } + if now.Sub(d.pendingAt) >= gpuStallAfter { + if !d.stalled { + log.Printf("gpu: %s (%s): a read has not returned in %s; its readings are paused until it does", d.name, d.card, gpuStallAfter) + } d.stalled, x.Stalled = true, true - log.Printf("gpu: %s (%s): a sysfs read did not return within %s; its readings are paused until restart", d.name, d.card, gpuReadWithin) } } @@ -724,7 +766,7 @@ func (d *gpuDev) readNvidia(x *GPU) { parseNvidiaQuery(x, out) } if out, ok := runOut("nvidia-smi", "pmon", "-c", "1", "-s", "um", "--id="+d.bus); ok { - d.nv = parseNvidiaPmon(out) + d.nvRead = parseNvidiaPmon(out) } } diff --git a/internal/metrics/gpu_stall_test.go b/internal/metrics/gpu_stall_test.go new file mode 100644 index 0000000..bd6bd89 --- /dev/null +++ b/internal/metrics/gpu_stall_test.go @@ -0,0 +1,52 @@ +package metrics + +import ( + "testing" + "time" +) + +// A read that comes back late carries the last readings meanwhile and is used when it lands; +// only one out past gpuStallAfter marks the card stalled, and its return clears that. +func TestGPUReadLateThenStalledThenBack(t *testing.T) { + defer func(w, s time.Duration) { gpuReadWithin, gpuStallAfter = w, s }(gpuReadWithin, gpuStallAfter) + gpuReadWithin, gpuStallAfter = 20*time.Millisecond, 10*time.Second + release := make(chan struct{}) + n := 0.0 // only the read goroutine writes it, one read at a time + d := &gpuDev{name: "test", card: "card0"} + d.readFn = func(x *GPU, _ time.Time) { + n++ + if n == 2 { + <-release + } + x.Usage = n * 10 + } + t0 := time.Now() + sample := func(at time.Duration) GPU { + x := GPU{Name: "test"} + d.readWithin(&x, t0.Add(at)) + return x + } + if x := sample(0); x.Usage != 10 || x.Stalled { + t.Fatalf("first: %+v", x) + } + if x := sample(time.Second); x.Usage != 10 || x.Stalled { // late: the last readings + t.Fatalf("late: %+v", x) + } + if x := sample(5 * time.Second); x.Usage != 10 || x.Stalled { + t.Fatalf("still late: %+v", x) + } + if x := sample(12 * time.Second); !x.Stalled { + t.Fatalf("out past gpuStallAfter, not stalled: %+v", x) + } + if x := sample(12*time.Second + 1); !x.Stalled { + t.Fatal("stall did not hold") + } + close(release) + time.Sleep(10 * time.Millisecond) + if x := sample(13 * time.Second); x.Usage != 20 || x.Stalled { + t.Fatalf("back: %+v", x) + } + if x := sample(14 * time.Second); x.Usage != 30 || x.Stalled { // 30: no extra read ran while one was out + t.Fatalf("next: %+v", x) + } +} diff --git a/web/src/Top.tsx b/web/src/Top.tsx index 7419c45..e58e43e 100644 --- a/web/src/Top.tsx +++ b/web/src/Top.tsx @@ -598,7 +598,7 @@ function GPUBox({ g, values, tick, advance, now, cap }: { g: GPU; values: number
- {g.stalled &&
a sysfs read of this card stopped answering; its readings are paused until the console restarts
} + {g.stalled &&
a read of this card stopped answering; its readings are paused until it does
}
{g.name}{g.driver}{g.integrated ? ' · integrated' : ''}
@@ -649,7 +649,7 @@ function GPULine({ g }: { g: GPU }) {
{g.driver}{g.integrated ? ' · integrated' : ''} {g.stalled - ? readings paused until the console restarts + ? readings paused until it answers : nothing measurable} {g.clients_ok ? 'no process has it open' : 'other users’ processes are hidden'}