From a7f0f9dfead9ead40a9a891a4a403b84f39331ed Mon Sep 17 00:00:00 2001 From: Sebastien Boeuf Date: Thu, 25 Jun 2020 10:08:05 +0200 Subject: [PATCH] vm-virtio: Ensure pause event is caught by every virtio thread Each virtio thread was reading/draining the pause_evt pipe when detecting the associated event. Problem is, when a virtio device has multiple threads, they all share the same pause_evt pipe, which can prevent some threads from receiving the event. If the first thread to catch the event is quickly clearing the pipe, some other threads might simply miss the event and they will not enter the "paused" state as expected. This is a behavior that was spotted with virtio-net as it usually uses 2 threads by default (1 for TX/RX queues and 1 for the control queue). The way to solve this issue is by letting each thread drain the pipe during the resume codepath, that is after the thread has been unparked. Signed-off-by: Sebastien Boeuf --- vm-virtio/src/block.rs | 7 +++++-- vm-virtio/src/console.rs | 7 +++++-- vm-virtio/src/iommu.rs | 7 +++++-- vm-virtio/src/mem.rs | 5 +++++ vm-virtio/src/net.rs | 7 +++++-- vm-virtio/src/net_util.rs | 7 +++++-- vm-virtio/src/pmem.rs | 7 +++++-- vm-virtio/src/rng.rs | 7 +++++-- vm-virtio/src/vhost_user/handler.rs | 7 +++++-- vm-virtio/src/vsock/device.rs | 7 +++++-- 10 files changed, 50 insertions(+), 18 deletions(-) diff --git a/vm-virtio/src/block.rs b/vm-virtio/src/block.rs index b3051e7b1..6f14eb140 100644 --- a/vm-virtio/src/block.rs +++ b/vm-virtio/src/block.rs @@ -832,8 +832,6 @@ impl BlockEpollHandler { break 'epoll; } PAUSE_EVENT => { - // Drain pause event - let _ = self.pause_evt.read(); debug!("PAUSE_EVENT received, pausing virtio-block epoll loop"); // We loop here to handle spurious park() returns. // Until we have not resumed, the paused boolean will @@ -841,6 +839,11 @@ impl BlockEpollHandler { while paused.load(Ordering::SeqCst) { thread::park(); } + + // Drain pause event after the device has been resumed. + // This ensures the pause event has been seen by each + // and every thread related to this virtio device. + let _ = self.pause_evt.read(); } _ => { error!("Unknown event for virtio-block"); diff --git a/vm-virtio/src/console.rs b/vm-virtio/src/console.rs index 9086915f8..1ec8d8048 100644 --- a/vm-virtio/src/console.rs +++ b/vm-virtio/src/console.rs @@ -313,8 +313,6 @@ impl ConsoleEpollHandler { break 'epoll; } PAUSE_EVENT => { - // Drain pause event - let _ = self.pause_evt.read(); debug!("PAUSE_EVENT received, pausing virtio-console epoll loop"); // We loop here to handle spurious park() returns. // Until we have not resumed, the paused boolean will @@ -322,6 +320,11 @@ impl ConsoleEpollHandler { while paused.load(Ordering::SeqCst) { thread::park(); } + + // Drain pause event after the device has been resumed. + // This ensures the pause event has been seen by each + // and every thread related to this virtio device. + let _ = self.pause_evt.read(); } _ => { error!("Unknown event for virtio-console"); diff --git a/vm-virtio/src/iommu.rs b/vm-virtio/src/iommu.rs index 1ec1b7c89..59b71c04d 100644 --- a/vm-virtio/src/iommu.rs +++ b/vm-virtio/src/iommu.rs @@ -744,8 +744,6 @@ impl IommuEpollHandler { break 'epoll; } PAUSE_EVENT => { - // Drain pause event - let _ = self.pause_evt.read(); debug!("PAUSE_EVENT received, pausing virtio-iommu epoll loop"); // We loop here to handle spurious park() returns. // Until we have not resumed, the paused boolean will @@ -753,6 +751,11 @@ impl IommuEpollHandler { while paused.load(Ordering::SeqCst) { thread::park(); } + + // Drain pause event after the device has been resumed. + // This ensures the pause event has been seen by each + // and every thread related to this virtio device. + let _ = self.pause_evt.read(); } _ => { error!("Unknown event for virtio-iommu"); diff --git a/vm-virtio/src/mem.rs b/vm-virtio/src/mem.rs index 248978caf..dc95fbf27 100644 --- a/vm-virtio/src/mem.rs +++ b/vm-virtio/src/mem.rs @@ -747,6 +747,11 @@ impl MemEpollHandler { while paused.load(Ordering::SeqCst) { thread::park(); } + + // Drain pause event after the device has been resumed. + // This ensures the pause event has been seen by each + // and every thread related to this virtio device. + let _ = self.pause_evt.read(); } _ => { return Err(DeviceError::EpollHander(String::from( diff --git a/vm-virtio/src/net.rs b/vm-virtio/src/net.rs index 0cadc1c41..529f8cb21 100644 --- a/vm-virtio/src/net.rs +++ b/vm-virtio/src/net.rs @@ -395,8 +395,6 @@ impl NetEpollHandler { break 'epoll; } PAUSE_EVENT => { - // Drain pause event - let _ = self.pause_evt.read(); debug!("PAUSE_EVENT received, pausing virtio-net epoll loop"); // We loop here to handle spurious park() returns. @@ -405,6 +403,11 @@ impl NetEpollHandler { while paused.load(Ordering::SeqCst) { thread::park(); } + + // Drain pause event after the device has been resumed. + // This ensures the pause event has been seen by each + // and every thread related to this virtio device. + let _ = self.pause_evt.read(); } _ => { error!("Unknown event for virtio-net"); diff --git a/vm-virtio/src/net_util.rs b/vm-virtio/src/net_util.rs index 4bfd2287e..3f25ad73e 100644 --- a/vm-virtio/src/net_util.rs +++ b/vm-virtio/src/net_util.rs @@ -316,8 +316,6 @@ impl NetCtrlEpollHandler { break 'epoll; } PAUSE_EVENT => { - // Drain pause event - let _ = self.pause_evt.read(); debug!("PAUSE_EVENT received, pausing vhost-user epoll loop"); // We loop here to handle spurious park() returns. // Until we have not resumed, the paused boolean will @@ -325,6 +323,11 @@ impl NetCtrlEpollHandler { while paused.load(Ordering::SeqCst) { std::thread::park(); } + + // Drain pause event after the device has been resumed. + // This ensures the pause event has been seen by each + // and every thread related to this virtio device. + let _ = self.pause_evt.read(); } _ => { error!("Unknown event for virtio-net"); diff --git a/vm-virtio/src/pmem.rs b/vm-virtio/src/pmem.rs index ebb474eba..e833c08e5 100644 --- a/vm-virtio/src/pmem.rs +++ b/vm-virtio/src/pmem.rs @@ -324,8 +324,6 @@ impl PmemEpollHandler { break 'epoll; } PAUSE_EVENT => { - // Drain pause event - let _ = self.pause_evt.read(); debug!("PAUSE_EVENT received, pausing virtio-pmem epoll loop"); // We loop here to handle spurious park() returns. // Until we have not resumed, the paused boolean will @@ -333,6 +331,11 @@ impl PmemEpollHandler { while paused.load(Ordering::SeqCst) { thread::park(); } + + // Drain pause event after the device has been resumed. + // This ensures the pause event has been seen by each + // and every thread related to this virtio device. + let _ = self.pause_evt.read(); } _ => { error!("Unknown event for virtio-block"); diff --git a/vm-virtio/src/rng.rs b/vm-virtio/src/rng.rs index 295e06b86..6bb156165 100644 --- a/vm-virtio/src/rng.rs +++ b/vm-virtio/src/rng.rs @@ -167,8 +167,6 @@ impl RngEpollHandler { break 'epoll; } PAUSE_EVENT => { - // Drain pause event - let _ = self.pause_evt.read(); debug!("PAUSE_EVENT received, pausing virtio-rng epoll loop"); // We loop here to handle spurious park() returns. // Until we have not resumed, the paused boolean will @@ -176,6 +174,11 @@ impl RngEpollHandler { while paused.load(Ordering::SeqCst) { thread::park(); } + + // Drain pause event after the device has been resumed. + // This ensures the pause event has been seen by each + // and every thread related to this virtio device. + let _ = self.pause_evt.read(); } _ => { error!("Unknown event for virtio-block"); diff --git a/vm-virtio/src/vhost_user/handler.rs b/vm-virtio/src/vhost_user/handler.rs index 8197c6938..2ac8b3276 100644 --- a/vm-virtio/src/vhost_user/handler.rs +++ b/vm-virtio/src/vhost_user/handler.rs @@ -163,8 +163,6 @@ impl VhostUserEpollHandler { break 'poll; } x if pause_evt_index == x => { - // Drain pause event - let _ = self.vu_epoll_cfg.pause_evt.read(); debug!("PAUSE_EVENT received, pausing vhost-user epoll loop"); // We loop here to handle spurious park() returns. // Until we have not resumed, the paused boolean will @@ -172,6 +170,11 @@ impl VhostUserEpollHandler { while paused.load(Ordering::SeqCst) { thread::park(); } + + // Drain pause event after the device has been resumed. + // This ensures the pause event has been seen by each + // and every thread related to this virtio device. + let _ = self.vu_epoll_cfg.pause_evt.read(); } x if (slave_evt_index.is_some() && slave_evt_index.unwrap() == x) => { if let Some(slave_req_handler) = diff --git a/vm-virtio/src/vsock/device.rs b/vm-virtio/src/vsock/device.rs index 6dd01e392..10e0f45e2 100644 --- a/vm-virtio/src/vsock/device.rs +++ b/vm-virtio/src/vsock/device.rs @@ -365,8 +365,6 @@ where return Ok(true); } PAUSE_EVENT => { - // Drain pause event - let _ = self.pause_evt.read(); debug!("PAUSE_EVENT received, pausing virtio-vsock epoll loop"); // We loop here to handle spurious park() returns. // Until we have not resumed, the paused boolean will @@ -374,6 +372,11 @@ where while paused.load(Ordering::SeqCst) { thread::park(); } + + // Drain pause event after the device has been resumed. + // This ensures the pause event has been seen by each + // and every thread related to this virtio device. + let _ = self.pause_evt.read(); } other => { error!("Unknown event for virtio-vsock");