disk-load: cpu_top's shares use the unrounded busy time, and key units by cgroup path (issue 741)

2026-10-15 15:06:06 +02:00
parent ab739d02dd
commit 2b1678f315
3 changed files with 99 additions and 12 deletions
@@ -105,3 +105,78 @@ func TestCPUTopLeavesOutACgroupThatAppearsOrVanishesWithinTheWindow(t *testing.T
 		t.Errorf("the container runtime was never asked: %v", f.calls)
 	}
 }
+
+// aTinyLoad is a machine with 12 CPUs, only 0.04 of them busy over a 2 s window (busy 1 of 300 total
+// ticks): rounded to one decimal for display, CPUsBusy reads 0.0, but a unit using half of that busy
+// time still has a non-zero share — review round 1's decision that share_pct and unaccounted_pct are
+// computed from the unrounded busy time, never from the rounded CPUsBusy.
+func aTinyLoad(t *testing.T) *fake {
+	f := newFake(t)
+	cpus := "cpu0 0 0 0 0 0 0 0 0 0 0\ncpu1 0 0 0 0 0 0 0 0 0 0\ncpu2 0 0 0 0 0 0 0 0 0 0\ncpu3 0 0 0 0 0 0 0 0 0 0\n" +
+		"cpu4 0 0 0 0 0 0 0 0 0 0\ncpu5 0 0 0 0 0 0 0 0 0 0\ncpu6 0 0 0 0 0 0 0 0 0 0\ncpu7 0 0 0 0 0 0 0 0 0 0\n" +
+		"cpu8 0 0 0 0 0 0 0 0 0 0\ncpu9 0 0 0 0 0 0 0 0 0 0\ncpu10 0 0 0 0 0 0 0 0 0 0\ncpu11 0 0 0 0 0 0 0 0 0 0\n"
+	f.file("/proc/stat", "cpu  0 0 0 0 0 0 0 0 0 0\n"+cpus)
+	f.then("/proc/stat", "cpu  1 0 0 299 0 0 0 0 0 0\n"+cpus)
+
+	f.file("/sys/fs/cgroup/system.slice/tiny.service/cpu.stat", "usage_usec 0\n")
+	f.then("/sys/fs/cgroup/system.slice/tiny.service/cpu.stat", "usage_usec 60000\n") // 3 % of one CPU over 2 s
+	return f
+}
+
+func TestCPUTopSharesAreNonZeroEvenWhenRoundedCPUsBusyIsZero(t *testing.T) {
+	out, err := tool(t, aTinyLoad(t).machine(), "cpu_top").Run(map[string]any{"seconds": 2.0})
+	if err != nil {
+		t.Fatal(err)
+	}
+	m := out.(map[string]any)
+	cpu := m["cpu"].(MachineCPU)
+	if cpu.CPUs != 12 || cpu.CPUsBusy != 0 {
+		t.Fatalf("the machine 0.04 CPUs busy, rounded for display: %+v", cpu)
+	}
+	top := m["top"].([]CPUUsage)
+	if len(top) != 1 || top[0].Unit != "tiny.service" || top[0].CPUPct != 3 || top[0].SharePct != 75 {
+		t.Fatalf("a non-zero share from the unrounded busy time: %+v", top)
+	}
+	if m["unaccounted_pct"] != 25.0 {
+		t.Fatalf("unaccounted from the unrounded busy time: %+v (want 25, so shares plus unaccounted add to 100)",
+			m["unaccounted_pct"])
+	}
+}
+
+// TestCPUTopKeepsCgroupsWithTheSameUnitNameAsSeparateRows is review round 1's other decision: units are keyed
+// by their cgroup path, never by filepath.Base, so two cgroups that both end in "init.scope" — one at the
+// root, one under a user's session — are two rows, not one merged by their shared leaf name.
+func TestCPUTopKeepsCgroupsWithTheSameUnitNameAsSeparateRows(t *testing.T) {
+	f := aCPUHog(t)
+	cg := func(path string, usec int) {
+		f.file("/sys/fs/cgroup/"+path+"/cpu.stat", "usage_usec 0\n")
+		f.then("/sys/fs/cgroup/"+path+"/cpu.stat", "usage_usec "+itoa(usec)+"\n")
+	}
+	cg("init.scope", 1000000)                             // the root init, 50 % of one CPU over 2 s
+	cg("user.slice/user@1000.service/init.scope", 400000) // a user session's init, 20 %
+
+	out, err := tool(t, f.machine(), "cpu_top").Run(map[string]any{"seconds": 2.0, "limit": 10.0})
+	if err != nil {
+		t.Fatal(err)
+	}
+	top := out.(map[string]any)["top"].([]CPUUsage)
+	var inits []CPUUsage
+	for _, u := range top {
+		if u.Unit == "init.scope" {
+			inits = append(inits, u)
+		}
+	}
+	if len(inits) != 2 {
+		t.Fatalf("two cgroups named init.scope, merged into %d row(s): %+v", len(inits), top)
+	}
+	byCgroup := map[string]CPUUsage{}
+	for _, u := range inits {
+		byCgroup[u.Cgroup] = u
+	}
+	if u, ok := byCgroup["init.scope"]; !ok || u.CPUPct != 50 {
+		t.Errorf("the root init.scope: %+v", byCgroup)
+	}
+	if u, ok := byCgroup["user.slice/user@1000.service/init.scope"]; !ok || u.CPUPct != 20 {
+		t.Errorf("the user session's init.scope: %+v", byCgroup)
+	}
+}