From 193d68404643c55558dba5145453666a4adbd2e8 Mon Sep 17 00:00:00 2001 From: Matt Delco Date: Thu, 10 Oct 2019 14:13:23 -0700 Subject: plugin: only pause on EINTR In the case of 1) an IO exit & callout to plugin, then 2) a pause request by another thread, the vcpu thread will eagerly check for a pause request and might cause another callout to the plugin for the pause. We haven't yet run KVM again for it to emulate the completion of the IO. It's probably less risky to call back into KVM again and let it finish the emulation before we callout to the plugin to make other potential state changes to the VM. This change also reduces the overhead of the non-pause case by not checking for a pause request on each VM exit. The tradeoff is that a pause request might take longer, but these are (or should be) relatively rare so it's better to slow these down (and be more conserative/sane about the state of the VM when pause is reported) in favor of making the non-pause VM exits faster. BUG=None TEST=Local build and run of "build_test". Change-Id: I38609eccd9a2196835f99de5ea84a586928fab30 Signed-off-by: Matt Delco Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1863725 Reviewed-by: Zach Reizner --- src/plugin/mod.rs | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) (limited to 'src/plugin/mod.rs') diff --git a/src/plugin/mod.rs b/src/plugin/mod.rs index e62fd39..451ac66 100644 --- a/src/plugin/mod.rs +++ b/src/plugin/mod.rs @@ -479,16 +479,22 @@ pub fn run_vcpus( break; } - // Try to clear the signal that we use to kick VCPU if it is - // pending before attempting to handle pause requests. + // Only handle the pause request if kvm reported that it was + // interrupted by a signal. This helps to entire that KVM has had a chance + // to finish emulating any IO that may have immediately happened. + // If we eagerly check pre_run() then any IO that we + // just reported to the plugin won't have been processed yet by KVM. + // Not eagerly calling pre_run() also helps to reduce + // any overhead from checking if a pause request is pending. + // The assumption is that pause requests aren't common + // or frequent so it's better to optimize for the non-pause execution paths. if interrupted_by_signal { clear_signal(SIGRTMIN() + 0) .expect("failed to clear pending signal"); - } - - if let Err(e) = vcpu_plugin.pre_run(&vcpu) { - error!("failed to process pause on vcpu {}: {}", cpu_id, e); - break; + if let Err(e) = vcpu_plugin.pre_run(&vcpu) { + error!("failed to process pause on vcpu {}: {}", cpu_id, e); + break; + } } } } -- cgit 1.4.1