perf(hypervisor): move pause VM teardown off the pause critical path

[Eulogizethesun]

This PR optimizes pause performance.

Summary

Pausing a sandbox ends with tearing the paused VM down. That teardown used to run entirely before the pause reply: for a 2 GiB sandbox, the guest memory unmap and the KVM fd close alone cost about 60 ms.

This PR defers exactly those two steps. The reply still waits for the whole baseline teardown chain, the same one the delete path runs: Vm::shutdown() wakes the parked workers, and dropping the Vm kills them (each device's Drop writes its kill event), removes the vsock and vhost-user socket files, and closes the tap and disk fds and the queue eventfds. One reference of the memory manager -- which owns both the guest RAM and the hypervisor VM handle -- is held past that drop and released on a background thread, so the unmap and the KVM fd close happen after the reply. The unmap still cannot overtake anyone: it happens when the last reference goes, so a worker still exiting keeps the mapping alive until it is done.

What the reply means: vcpus joined, every worker killed, the vsock and vhost-user socket files removed, the tap and disk fds and the queue eventfds closed. Worker thread exits are asynchronous (the codebase joins only the net workers), so a thread may still be completing its exit at the reply.

Performance

Measured on v0.7.2 (2 GiB sandbox, pause to snapshot), before/after this change:

concurrency	op	before	after	delta
1	pause	149.8 ms	88.4 ms	-41%
50	pause (avg request)	515.9 ms	411.8 ms	-20%

Resume latency is unchanged.

Testing

• cargo test -p virtio-devices -p vmm --features vmm/kvm --lib: 52 passed, 44 passed / 15 failed (no /dev/kvm in the build container, pre-existing)
• 30/30 same-ID pause→restore adversarial cycles
• c1/c50 pause/resume benchmark (table above); the measurement host was restored to its stock binary afterwards

[ls-ggg]

wait_notify(VmShutdown) after pause-to-snapshot is historical: returning before the VM was fully gone used to race host resources (TAP, and similar). That context is old — happy to re-evaluate whether teardown can stay async. Please confirm TAP / Device busy / same-ID resume are still safe without the wait @lisongqian cc

[lisongqian]

Thank you for your contribution! It's a good idea to speed up vm_pause_to_snapshot. Some comments.

[cubesandboxbot]

AI-generated review

This review was produced by an automated AI agent (Claude Code, deepseek/deepseek-flash) against the base-branch workspace and the prefetched diff. It is not human approval, and it does not certify anything. A human must review and take responsibility for the change.

Scope reviewed: hypervisor/vmm/src/lib.rs (+37/-1), hypervisor/vmm/src/vm.rs (+6). Diff was complete (no truncation marker in review-input/).

---

What the change does

vm_pause_to_snapshot no longer calls vm_delete(). It inlines the baseline teardown (take the Vm, log counters, vm.shutdown(), drop it, clear vm_config, emit vm deleted) and then keeps one clone of the Arc<Mutex<MemoryManager>> alive on a detached pause-release thread, so the guest-RAM munmap and the KVM VM-fd close leave the pause critical path. Vm::memory_manager() (vm.rs:2489) is the new pub(crate) accessor.

Verdict

No blocking correctness defect found in the diff. The core mechanism is sound: the deferral can only make teardown later than before, never earlier, so it cannot introduce an early unmap or a use-after-free. I verified the load-bearing assumptions against the base tree:

• The reply contract the caller actually depends on is preserved. Vm::shutdown() still emits event_notify!(NotifyEvent::VmShutdown) (vm.rs:1336) before the reply, so the shim's post-pause wait at CubeShim/shim/src/sandbox/sb.rs:1608 still returns and the pause flow does not stall.
• The described teardown really is the pre-reply part. DeviceManager::drop (device_manager.rs:5086) wakes the device workers and calls each device's shutdown(), which is what removes the vsock/vhost-user socket files and closes the tap/disk/queue fds; it runs at drop(vm), i.e. before the reply.
• Seccomp will not kill the release thread. vm_pause_to_snapshot runs on the vmm thread (lib.rs:2055), whose filter (vmm_thread_rules, seccomp_filters.rs:497-698) whitelists clone/clone3 (508-509), munmap (563), madvise (557), mmap (560) and unlink/unlinkat (630-631) — thread creation and the deferred unmap/drop are covered.
• No fixed-address collision risk from the deferred unmap. Guest RAM is mapped without MAP_FIXED (memory_manager.rs:1495+); the only MAP_FIXED uses are the virtio-fs DAX window in the separate virtiofsd address space (virtio-devices/src/fs.rs:185). A subsequent same-process restore therefore cannot be handed the not-yet-unmapped range, and the eventual munmap cannot clobber the new VM's mappings.
• The fallback logic is correct. On Builder::spawn failure the closure (and its reference) is dropped by std, so the release runs inline on the vmm thread, as the comment says.

The findings below are about semantics documentation, failure observability and drift risk — see the four inline comments.

Findings

1. lib.rs:701 — pause re-implements vm_delete(). Two copies of the same teardown now have to be kept in sync, and they already differ (the vm_config.is_none() guard, the vm_shutdown() indirection). Future cleanup added to the delete path will silently skip pause. Suggest a shared vm_teardown() returning the held memory-manager reference, with each caller deciding when to drop it.
2. lib.rs:716 — the reply is no longer a memory-release barrier. vm_config = None and the vm deleted event now land while up to a guest RAM's worth of RSS and the KVM fd are still live; a same-process vm_resume_from_snapshot is accepted in that window (lib.rs:727), so a pause→restore can transiently need ~2× the guest RAM, and at the PR's c50 workload that is concurrency × guest RAM of extra live RSS. Correct trade for a latency win, but it belongs in the pause2snapshot API docs, not only in an inline comment.
3. lib.rs:720 — the release thread is untracked, unbounded and swallows failures. The handle is dropped; vmm_shutdown cannot wait for it; a panic in the deferred MemoryManager::drop kills only the detached thread while pause still reports Ok (previously it ran on the vmm thread inside vm_shutdown); and a pause burst spawns one such thread per pause. The file already has the right pattern for this — load_payload_handle is stored and joined (vm.rs:502, vm.rs:2123).
4. lib.rs:708 — comment precision. "The last reference" conflates two Arcs: the held MemoryManager reference does keep the KVM fd open, but the guest RAM is separately Arc-shared via GuestMemoryAtomic clones given to device workers, and only the net workers are joined. The munmap is therefore after-the-reply but not guaranteed to be on this thread or prompt. Worth spelling out, since the whole justification rests on that paragraph.

Testing / process notes

• No test covers the new behavior. The deferral itself is hard to unit-test, but even the spawn-failure fallback (which silently degrades to the old latency) is unverified. At minimum, a check that the release completes shortly after a pause in the KVM-enabled integration suite would guard the refactor suggested in finding 1.
• The error-path comment ("Cleared only on success: on failure the VM is already gone and only deleting the sandbox recovers") matches the code, and that state matches the pre-change vm_delete behavior. No action needed.
• hypervisor/vmm/src/vm.rs — pub(crate) fn memory_manager() returning a clone is fine and used once; the name reads like a borrow accessor, memory_manager_handle() or similar would signal the clone.
• Per AGENTS.md, the commit message or PR description must carry an Assisted-by: AGENT_NAME:MODEL_VERSION (or Autonomously-by:) trailer. The prefetched PR body has none; commit trailers were not part of the review inputs, so please confirm they are present — otherwise this needs adding before merge.

[lkml-likexu]

@Eulogizethesun Thanks for your contribution. Please resolve the CI failure issue AND click "Resolve conversation" to ensure that all comments from cubesandboxbot are either approved or rejected.

[lkml-likexu]

@Eulogizethesun You don't have to resolve every issue in the AI report; you just need to make sure there are no issues of moderate severity or higher in the critical patch and make sure tests are covered.

[Eulogizethesun]

@lkml-likexu @lisongqian @ls-ggg

The code is ready. Only the guest RAM unmap and the KVM fd close are deferred to a background thread; tap, vsock and the rest of the teardown are unchanged and still finish before the reply, so the TAP race mentioned earlier does not exist. All review feedback has been addressed, and CI is green. Could you take a look when you have a chance?

[lisongqian]

Thank you for your efforts in improving performance. Can you update the performance data based on the latest code? @Eulogizethesun

[Eulogizethesun]

@lisongqian Updated. The data in the description is now measured with this change on top of v0.7.2: pause c1 149.8 → 88.4 ms (-41%), c50 515.9 → 411.8 ms (-20%). Is the code OK to merge?

[lisongqian]

LGTM. Just a code style question; it doesn't block the PR.

[lkml-likexu]

Perf data (rebase commit 3350a09 on e02976a) on Intel: