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")