From d4fb892ce45c35e8e80cc5a5c698e36b00078988 Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Wed, 11 Nov 2026 08:07:48 +0000 Subject: [PATCH] Give VFIO guests time to exit when many are terminated together A VMM with VFIO devices releases them on its way out, and the host driver is slower at that when many guests go down at once. The grace after SIGTERM was fixed, so deleting 16 or more vGPU guests together hard-killed them mid-release and then failed most of the deletes about a second before the VMMs were gone, leaving the instances for a retry. The grace now grows by one second for every other VFIO teardown in flight, up to a minute. A guest terminated alone waits as before. --- lib/instances/manager.go | 3 + lib/instances/process_identity.go | 39 ++++++++++- lib/instances/process_identity_linux_test.go | 74 ++++++++++++++++++++ 3 files changed, 115 insertions(+), 1 deletion(-) diff --git a/lib/instances/manager.go b/lib/instances/manager.go index ed85f22f4..a99e14305 100644 --- a/lib/instances/manager.go +++ b/lib/instances/manager.go @@ -8,6 +8,7 @@ import ( "os" "path/filepath" "sync" + "sync/atomic" "time" "github.com/kernel/hypeman/lib/devices" @@ -228,6 +229,8 @@ type manager struct { vgpuReconcileInterval time.Duration vfioTermGrace time.Duration + // vfioTeardowns counts the VMMs with VFIO devices being terminated now. + vfioTeardowns atomic.Int32 // Hypervisor support vmStarters map[hypervisor.Type]hypervisor.VMStarter diff --git a/lib/instances/process_identity.go b/lib/instances/process_identity.go index 57aac3af7..08ac860bb 100644 --- a/lib/instances/process_identity.go +++ b/lib/instances/process_identity.go @@ -27,6 +27,21 @@ const linuxBootIDPath = "/proc/sys/kernel/random/boot_id" // does not unstick it, so the wait is short to keep stop and delete fast. const hypervisorSIGKILLWaitTimeout = 2 * time.Second +const ( + // vfioTeardownQueueAllowance is the extra time a VMM with VFIO devices + // gets to exit on SIGTERM for every other such VMM being terminated at + // the same time. The device is released on the VMM's way out, and the + // host driver is slower at that when many go at once: vGPU guests exited + // about a second after SIGTERM when up to 8 were terminated together, but + // took up to 10 s when 16 or 26 were. That outlasted the fixed grace, so + // they were hard-killed mid-release and their deletes failed about a + // second before the VMMs were gone. + vfioTeardownQueueAllowance = time.Second + // vfioTeardownMaxAllowance caps the allowance for a queue of any length. + vfioTeardownMaxAllowance = time.Minute + vfioTeardownPollInterval = 200 * time.Millisecond +) + func (m *manager) vfioTerminationGrace() time.Duration { if m.vfioTermGrace > 0 { return m.vfioTermGrace @@ -37,7 +52,9 @@ func (m *manager) vfioTerminationGrace() time.Duration { // SIGKILL during guest driver init can wedge a VF until the parent GPU is reset. func (m *manager) terminateThenKill(ctx context.Context, inst *Instance, pid int) error { if hasVFIODevices(&inst.StoredMetadata) { - if syscall.Kill(pid, syscall.SIGTERM) == nil && WaitForProcessExit(pid, m.vfioTerminationGrace()) { + m.vfioTeardowns.Add(1) + defer m.vfioTeardowns.Add(-1) + if syscall.Kill(pid, syscall.SIGTERM) == nil && m.waitForVFIOTeardown(pid, m.vfioTerminationGrace()) { return nil } logger.FromContext(ctx).WarnContext(ctx, "hypervisor with VFIO devices did not exit on SIGTERM; hard-killing, device may wedge if the guest driver was initializing", @@ -46,6 +63,26 @@ func (m *manager) terminateThenKill(ctx context.Context, inst *Instance, pid int return killProcessAndWait(pid) } +// waitForVFIOTeardown waits for pid to exit for base plus the queue allowance. +// The allowance follows the largest number of VFIO teardowns seen in flight +// during the wait: a VMM that started exiting alone is still slowed down by +// the ones that join it. +func (m *manager) waitForVFIOTeardown(pid int, base time.Duration) bool { + start := time.Now() + peak := int32(1) + for { + peak = max(peak, m.vfioTeardowns.Load()) + allowance := min(time.Duration(peak-1)*vfioTeardownQueueAllowance, vfioTeardownMaxAllowance) + remaining := base + allowance - time.Since(start) + if remaining <= 0 { + return false + } + if WaitForProcessExit(pid, min(remaining, vfioTeardownPollInterval)) { + return true + } + } +} + // killProcessAndWait SIGKILLs pid and waits for it to exit. A process that // survives the first wait gets its process group killed too (the hypervisor // may have spawned children in its own group) and a short grace period. An diff --git a/lib/instances/process_identity_linux_test.go b/lib/instances/process_identity_linux_test.go index 0bf65d7af..8ce958ff1 100644 --- a/lib/instances/process_identity_linux_test.go +++ b/lib/instances/process_identity_linux_test.go @@ -721,6 +721,80 @@ func TestKillHypervisorEscalatesToSIGKILLWhenSIGTERMIgnored(t *testing.T) { assert.ErrorIs(t, syscall.Kill(pid, 0), syscall.ESRCH, "SIGTERM-ignoring hypervisor must still be hard-killed") } +// A VMM with VFIO devices exits only after the host driver has released its +// device, and the driver is slower at that when many guests go down together. +// The wait has to grow with their number, or a mass delete hard-kills and +// then gives up on VMMs that were about to exit. +func TestKillHypervisorWaitsLongerWhileOtherVFIOTeardownsAreInFlight(t *testing.T) { + for _, tc := range []struct { + name string + othersInQueue int32 + exitsCleanly bool + }{ + {name: "alone", othersInQueue: 0, exitsCleanly: false}, + {name: "behind two others", othersInQueue: 2, exitsCleanly: true}, + } { + t.Run(tc.name, func(t *testing.T) { + markerPath := filepath.Join(t.TempDir(), "released") + // Stands in for a VMM whose device release takes 0.6 s after SIGTERM. + pid, identity := startTrapProcess(t, "sleep 0.6; touch "+markerPath+"; exit 0") + + m := &manager{vfioTermGrace: 100 * time.Millisecond} + m.vfioTeardowns.Store(tc.othersInQueue) + require.NoError(t, m.killHypervisor(context.Background(), &Instance{ + State: StateRunning, + StoredMetadata: StoredMetadata{ + Id: "kill-test", + GPUProfile: "NVIDIA L40S-1Q", + GPUFramework: devices.VGPUFrameworkVendorVFIO, + GPUDevicePath: "/sys/bus/pci/devices/0000:82:00.4", + HypervisorProcessIdentity: identity, + SocketPath: filepath.Join(t.TempDir(), "missing.sock"), + }, + })) + + assert.ErrorIs(t, syscall.Kill(pid, 0), syscall.ESRCH) + assert.Equal(t, tc.othersInQueue, m.vfioTeardowns.Load(), "the teardown must leave the queue when it returns") + if tc.exitsCleanly { + assert.FileExists(t, markerPath, "a queued VMM must be allowed to finish releasing its device") + } else { + assert.NoFileExists(t, markerPath, "without a queue the short grace still applies") + } + }) + } +} + +// The queue is made of the teardowns themselves: one that is in flight has to +// count for the next one that starts. +func TestKillHypervisorCountsVFIOTeardownsInFlight(t *testing.T) { + m := &manager{vfioTermGrace: 250 * time.Millisecond} + kill := func(identity HypervisorProcessIdentity) error { + return m.killHypervisor(context.Background(), &Instance{ + State: StateRunning, + StoredMetadata: StoredMetadata{ + Id: "kill-test", + GPUProfile: "NVIDIA L40S-1Q", + GPUFramework: devices.VGPUFrameworkVendorVFIO, + GPUDevicePath: "/sys/bus/pci/devices/0000:82:00.4", + HypervisorProcessIdentity: identity, + SocketPath: filepath.Join(t.TempDir(), "missing.sock"), + }, + }) + } + markerPath := filepath.Join(t.TempDir(), "released") + _, first := startTrapProcess(t, "sleep 0.6; exit 0") + _, second := startTrapProcess(t, "sleep 0.6; touch "+markerPath+"; exit 0") + + firstDone := make(chan error, 1) + go func() { firstDone <- kill(first) }() + require.Eventually(t, func() bool { return m.vfioTeardowns.Load() == 1 }, 5*time.Second, time.Millisecond) + + require.NoError(t, kill(second)) + assert.FileExists(t, markerPath, "a teardown in flight must lengthen the wait of one that starts meanwhile") + require.NoError(t, <-firstDone) + assert.Zero(t, m.vfioTeardowns.Load()) +} + func TestKillHypervisorHardKillsNonVGPUHypervisor(t *testing.T) { markerPath := filepath.Join(t.TempDir(), "terminated") pid, identity := startTrapProcess(t, "touch "+markerPath+"; exit 0")