mirror of
https://github.com/cloud-hypervisor/cloud-hypervisor.git
synced 2026-08-05 02:19:16 +00:00
d39b56544a
Fix a race condition that happens in resume()-pause() cycles.
It is odd that for pause(), the CpuManager waited via `state.paused`
for the vCPU thread to ACK the state change but not for `resume()`.
In the `resume()` case, oddly CpuManager "owned" the state change
in `state.paused`. This commit changes this so that the vCPU ACKs
its state change itself in `state.paused` when it transitions from
pause->run. Further, `CpuManager::resume()` now gracefully waits for
the vCPU to be resumed.
More technical:
This change ensures proper synchronization and prevents situations
in that park() follows right after unpark(), causing deadlocks and
other weird behavior due to race conditions.
Calling resume() now takes slightly longer, very similar to pause().
This is, however, even for 254 vCPUs in the range of less than 10ms,
and ultimately we now have correct behaviour.
## Reproducer
Since [0] is merged, the underlying problem can be tested without this
commit by modifying the pause() API call to run `CpuManager::pause()`
and `CpuManager::resume()` in a loop a thousand times.
`ch-remote --api-socket ... pause`
```patch
diff --git a/vmm/src/vm.rs b/vmm/src/vm.rs
index d7bba25cc..35557d58f 100644
--- a/vmm/src/vm.rs
+++ b/vmm/src/vm.rs
@@ -2687,6 +2687,10 @@ impl Pausable for Vm {
MigratableError::Pause(anyhow!("Error activating pending virtio devices: {:?}", e))
})?;
+ for _ in 0..1000 {
+ self.cpu_manager.lock().unwrap().pause()?;
+ self.cpu_manager.lock().unwrap().resume()?;
+ }
self.cpu_manager.lock().unwrap().pause()?;
self.device_manager.lock().unwrap().pause()?;
```
## Outlook
Decades of experience in VMM development showed us that using
many AtomicBools is a footgun. They are not synchronized with each
other at all. On the long term, we might want to refactor things to
have a single shared AtomicU64 with different bits having different
meanings.
[0] https://github.com/cloud-hypervisor/cloud-hypervisor/pull/7290
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com