Give VFIO guests time to exit when many are terminated together

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

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

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