From b1efb343a50369efe79253c9a170149aa2257bc5 Mon Sep 17 00:00:00 2001 From: Juan Tian Date: Thu, 21 Nov 2024 21:31:48 +0000 Subject: [PATCH 1/5] call unstick_channels to mitigate the lost synic issue if the fix was not in before servicing --- vm/devices/vmbus/vmbus_server/src/lib.rs | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/vm/devices/vmbus/vmbus_server/src/lib.rs b/vm/devices/vmbus/vmbus_server/src/lib.rs index 9ccaafff5..6fc2ed008 100644 --- a/vm/devices/vmbus/vmbus_server/src/lib.rs +++ b/vm/devices/vmbus/vmbus_server/src/lib.rs @@ -43,6 +43,7 @@ use mesh::RecvError; use pal_async::task::Spawn; use pal_async::task::Task; use pal_event::Event; +use parking_lot::Mutex; use ring::PAGE_SIZE; use std::collections::HashMap; use std::future::Future; @@ -231,6 +232,8 @@ impl EventPort for ChannelEvent { pub struct SavedState { #[mesh(1)] server: channels::SavedState, + #[mesh(2)] + lost_synic_bug_fixed: Option, } const MESSAGE_CONNECTION_ID: u32 = 1; @@ -837,6 +840,7 @@ impl ServerTask { fn handle_request(&mut self, request: VmbusRequest) { tracing::debug!(?request, "handle_request"); + static IS_LOST_SYNIC_BUG_FIXED: Mutex = Mutex::new(false); match request { VmbusRequest::Reset(Rpc((), done)) => { assert!(self.inner.reset_done.is_none()); @@ -870,13 +874,23 @@ impl ServerTask { .merge(&self.server.with_notifier(&mut self.inner)); }); } - VmbusRequest::Save(rpc) => rpc.handle_sync(|()| SavedState { - server: self.server.save(), - }), + VmbusRequest::Save(rpc) => { + // TODO: update to true once the save part fix in. + let lost_synic_bug_fixed = Some(false); + rpc.handle_sync(|()| SavedState { + server: self.server.save(), + lost_synic_bug_fixed, + }) + } VmbusRequest::Restore(rpc) => { + *IS_LOST_SYNIC_BUG_FIXED.lock() = rpc.0.lost_synic_bug_fixed.unwrap_or(false); rpc.handle_sync(|state| self.server.restore(state.server)) } VmbusRequest::PostRestore(rpc) => { + if !*IS_LOST_SYNIC_BUG_FIXED.lock() { + tracing::info!("lost synic bug fix is not in yet, call unstick_channels to mitigate the issue."); + self.unstick_channels(false); + } rpc.handle_sync(|()| self.server.with_notifier(&mut self.inner).post_restore()) } VmbusRequest::Stop(rpc) => rpc.handle_sync(|()| { From f55cf5a4199e4bf8a2d6f215cbd9094d21aed346 Mon Sep 17 00:00:00 2001 From: Juan Tian Date: Thu, 21 Nov 2024 22:57:58 +0000 Subject: [PATCH 2/5] address comments --- vm/devices/vmbus/vmbus_server/src/lib.rs | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/vm/devices/vmbus/vmbus_server/src/lib.rs b/vm/devices/vmbus/vmbus_server/src/lib.rs index 6fc2ed008..f0173cbf0 100644 --- a/vm/devices/vmbus/vmbus_server/src/lib.rs +++ b/vm/devices/vmbus/vmbus_server/src/lib.rs @@ -43,7 +43,6 @@ use mesh::RecvError; use pal_async::task::Spawn; use pal_async::task::Task; use pal_event::Event; -use parking_lot::Mutex; use ring::PAGE_SIZE; use std::collections::HashMap; use std::future::Future; @@ -233,7 +232,7 @@ pub struct SavedState { #[mesh(1)] server: channels::SavedState, #[mesh(2)] - lost_synic_bug_fixed: Option, + lost_synic_bug_fixed: bool, } const MESSAGE_CONNECTION_ID: u32 = 1; @@ -488,6 +487,7 @@ impl<'a, T: Spawn> VmbusServerBuilder<'a, T> { inner, external_requests: self.external_requests, next_seq: 0, + unstick_on_start: false, }; let task = self.spawner.spawn("vmbus server", async move { @@ -604,6 +604,7 @@ struct ServerTask { external_requests: Option>, /// Next value for [`Channel::seq`]. next_seq: u64, + unstick_on_start: bool, } struct ServerTaskInner { @@ -840,7 +841,6 @@ impl ServerTask { fn handle_request(&mut self, request: VmbusRequest) { tracing::debug!(?request, "handle_request"); - static IS_LOST_SYNIC_BUG_FIXED: Mutex = Mutex::new(false); match request { VmbusRequest::Reset(Rpc((), done)) => { assert!(self.inner.reset_done.is_none()); @@ -876,21 +876,17 @@ impl ServerTask { } VmbusRequest::Save(rpc) => { // TODO: update to true once the save part fix in. - let lost_synic_bug_fixed = Some(false); + let lost_synic_bug_fixed = false; rpc.handle_sync(|()| SavedState { server: self.server.save(), lost_synic_bug_fixed, }) } - VmbusRequest::Restore(rpc) => { - *IS_LOST_SYNIC_BUG_FIXED.lock() = rpc.0.lost_synic_bug_fixed.unwrap_or(false); - rpc.handle_sync(|state| self.server.restore(state.server)) - } + VmbusRequest::Restore(rpc) => rpc.handle_sync(|state| { + self.unstick_on_start = !state.lost_synic_bug_fixed; + self.server.restore(state.server) + }), VmbusRequest::PostRestore(rpc) => { - if !*IS_LOST_SYNIC_BUG_FIXED.lock() { - tracing::info!("lost synic bug fix is not in yet, call unstick_channels to mitigate the issue."); - self.unstick_channels(false); - } rpc.handle_sync(|()| self.server.with_notifier(&mut self.inner).post_restore()) } VmbusRequest::Stop(rpc) => rpc.handle_sync(|()| { @@ -901,6 +897,10 @@ impl ServerTask { VmbusRequest::Start => { if !self.running { self.running = true; + if self.unstick_on_start { + tracing::info!("lost synic bug fix is not in yet, call unstick_channels to mitigate the issue."); + self.unstick_channels(false); + } } } } From 1b87e4e7c984a803a00d4306c34b576387fe2b7d Mon Sep 17 00:00:00 2001 From: Juan Tian Date: Thu, 21 Nov 2024 23:57:53 +0000 Subject: [PATCH 3/5] reset unstick_on_start after unstick_channels --- vm/devices/vmbus/vmbus_server/src/lib.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/vm/devices/vmbus/vmbus_server/src/lib.rs b/vm/devices/vmbus/vmbus_server/src/lib.rs index f0173cbf0..1570802e3 100644 --- a/vm/devices/vmbus/vmbus_server/src/lib.rs +++ b/vm/devices/vmbus/vmbus_server/src/lib.rs @@ -231,6 +231,9 @@ impl EventPort for ChannelEvent { pub struct SavedState { #[mesh(1)] server: channels::SavedState, + // Indicates if the lost synic bug is fixed or not. By default it's false. + // During the restore process, we check if the field is not true then + // unstick_channels() function will be called to mitigate the issue. #[mesh(2)] lost_synic_bug_fixed: bool, } @@ -900,6 +903,7 @@ impl ServerTask { if self.unstick_on_start { tracing::info!("lost synic bug fix is not in yet, call unstick_channels to mitigate the issue."); self.unstick_channels(false); + self.unstick_on_start = false; } } } From eed18730ed18fabdc7d5cbf2b168e683813b35b3 Mon Sep 17 00:00:00 2001 From: Juan Tian Date: Fri, 22 Nov 2024 18:12:28 +0000 Subject: [PATCH 4/5] set lost_synic_bug_fixed=true --- vm/devices/vmbus/vmbus_server/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/vm/devices/vmbus/vmbus_server/src/lib.rs b/vm/devices/vmbus/vmbus_server/src/lib.rs index 1570802e3..86d5b767d 100644 --- a/vm/devices/vmbus/vmbus_server/src/lib.rs +++ b/vm/devices/vmbus/vmbus_server/src/lib.rs @@ -878,8 +878,7 @@ impl ServerTask { }); } VmbusRequest::Save(rpc) => { - // TODO: update to true once the save part fix in. - let lost_synic_bug_fixed = false; + let lost_synic_bug_fixed = true; rpc.handle_sync(|()| SavedState { server: self.server.save(), lost_synic_bug_fixed, From 44b98ae8e751789ebe3f0404d1899e21eaa9865a Mon Sep 17 00:00:00 2001 From: Juan Tian Date: Fri, 22 Nov 2024 19:37:23 +0000 Subject: [PATCH 5/5] remove local lost_synic_bug_fixed --- vm/devices/vmbus/vmbus_server/src/lib.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/vm/devices/vmbus/vmbus_server/src/lib.rs b/vm/devices/vmbus/vmbus_server/src/lib.rs index 86d5b767d..d32440e87 100644 --- a/vm/devices/vmbus/vmbus_server/src/lib.rs +++ b/vm/devices/vmbus/vmbus_server/src/lib.rs @@ -877,13 +877,10 @@ impl ServerTask { .merge(&self.server.with_notifier(&mut self.inner)); }); } - VmbusRequest::Save(rpc) => { - let lost_synic_bug_fixed = true; - rpc.handle_sync(|()| SavedState { - server: self.server.save(), - lost_synic_bug_fixed, - }) - } + VmbusRequest::Save(rpc) => rpc.handle_sync(|()| SavedState { + server: self.server.save(), + lost_synic_bug_fixed: true, + }), VmbusRequest::Restore(rpc) => rpc.handle_sync(|state| { self.unstick_on_start = !state.lost_synic_bug_fixed; self.server.restore(state.server)