From f38056fc9eec3e7f5f7bfa8dac23f6bf2e776681 Mon Sep 17 00:00:00 2001 From: Sebastien Boeuf Date: Fri, 16 Sep 2022 18:18:22 +0200 Subject: [PATCH] virtio-devices, vmm: Simplify virtio-mem resize operation There's no need to delegate the resize operation to the virtio-mem thread. This can come directly from the vmm thread which will use the Mem object to update the VIRTIO configuration and trigger the interrupt for the guest to be notified. In order to achieve what's described above, the VirtioMemZone structure now has a handle onto the Mem object directly. This avoids the need for intermediate Resize and ResizeSender structures. Signed-off-by: Sebastien Boeuf --- virtio-devices/src/mem.rs | 144 ++++++-------------------------------- vmm/src/device_manager.rs | 18 +++-- vmm/src/memory_manager.rs | 30 +++++--- 3 files changed, 50 insertions(+), 142 deletions(-) diff --git a/virtio-devices/src/mem.rs b/virtio-devices/src/mem.rs index 36222fa0f..c8ff84238 100644 --- a/virtio-devices/src/mem.rs +++ b/virtio-devices/src/mem.rs @@ -22,14 +22,13 @@ use crate::thread_helper::spawn_virtio_thread; use crate::{GuestMemoryMmap, GuestRegionMmap}; use crate::{VirtioInterrupt, VirtioInterruptType}; use anyhow::anyhow; -use libc::EFD_NONBLOCK; use seccompiler::SeccompAction; use std::collections::BTreeMap; use std::io; use std::mem::size_of; use std::os::unix::io::{AsRawFd, RawFd}; use std::result; -use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; +use std::sync::atomic::AtomicBool; use std::sync::mpsc; use std::sync::{Arc, Barrier, Mutex}; use thiserror::Error; @@ -96,10 +95,8 @@ const VIRTIO_MEM_REQ_UNPLUG_ALL: u16 = 2; // request information about the plugged state of memory blocks const VIRTIO_MEM_REQ_STATE: u16 = 3; -// Get resize event. -const RESIZE_EVENT: u16 = EPOLL_HELPER_EVENT_LAST + 1; // New descriptors are pending on the virtio queue. -const QUEUE_AVAIL_EVENT: u16 = EPOLL_HELPER_EVENT_LAST + 2; +const QUEUE_AVAIL_EVENT: u16 = EPOLL_HELPER_EVENT_LAST + 1; // Virtio features const VIRTIO_MEM_F_ACPI_PXM: u8 = 0; @@ -328,78 +325,6 @@ impl Request { } } -pub struct ResizeSender { - hotplugged_size: Arc, - activated: Arc, - tx: mpsc::Sender>, - evt: EventFd, -} - -impl ResizeSender { - fn size(&self) -> u64 { - self.hotplugged_size.load(Ordering::Acquire) - } - - fn send(&self, r: Result<(), Error>) -> Result<(), mpsc::SendError>> { - self.tx.send(r) - } -} - -impl Clone for ResizeSender { - fn clone(&self) -> Self { - ResizeSender { - hotplugged_size: self.hotplugged_size.clone(), - activated: self.activated.clone(), - tx: self.tx.clone(), - evt: self - .evt - .try_clone() - .expect("Failed cloning EventFd from ResizeSender"), - } - } -} - -pub struct Resize { - hotplugged_size: Arc, - activated: Arc, - tx: mpsc::Sender>, - rx: mpsc::Receiver>, - evt: EventFd, -} - -impl Resize { - pub fn new(hotplugged_size: u64) -> io::Result { - let (tx, rx) = mpsc::channel(); - - Ok(Resize { - hotplugged_size: Arc::new(AtomicU64::new(hotplugged_size)), - activated: Arc::new(AtomicBool::default()), - tx, - rx, - evt: EventFd::new(EFD_NONBLOCK)?, - }) - } - - pub fn new_resize_sender(&self) -> Result { - Ok(ResizeSender { - hotplugged_size: self.hotplugged_size.clone(), - activated: self.activated.clone(), - tx: self.tx.clone(), - evt: self.evt.try_clone().map_err(Error::EventFdTryCloneFail)?, - }) - } - - pub fn work(&self, desired_size: u64) -> Result<(), Error> { - if !self.activated.load(Ordering::Acquire) { - return Err(Error::NotActivatedByGuest); - } - - self.hotplugged_size.store(desired_size, Ordering::Release); - self.evt.write(1).map_err(Error::EventFdWriteFail)?; - self.rx.recv().map_err(Error::MpscRecvFail)? - } -} - #[derive(Clone, Versionize)] pub struct BlocksState { bitmap: Vec, @@ -468,7 +393,6 @@ struct MemEpollHandler { host_fd: Option, blocks_state: Arc>, config: Arc>, - resize: ResizeSender, queue: Queue, interrupt_cb: Arc, queue_evt: EventFd, @@ -715,7 +639,6 @@ impl MemEpollHandler { paused_sync: Arc, ) -> result::Result<(), EpollHelperError> { let mut helper = EpollHelper::new(&self.kill_evt, &self.pause_evt)?; - helper.add_event(self.resize.evt.as_raw_fd(), RESIZE_EVENT)?; helper.add_event(self.queue_evt.as_raw_fd(), QUEUE_AVAIL_EVENT)?; helper.run(paused, paused_sync, self)?; @@ -731,41 +654,6 @@ impl EpollHelperHandler for MemEpollHandler { ) -> result::Result<(), EpollHelperError> { let ev_type = event.data as u16; match ev_type { - RESIZE_EVENT => { - self.resize.evt.read().map_err(|e| { - EpollHelperError::HandleEvent(anyhow!("Failed to get resize event: {:?}", e)) - })?; - - let mut config = self.config.lock().unwrap(); - if let Err(e) = config.resize(self.resize.size()) { - self.resize.send(Err(e)).map_err(|e| { - EpollHelperError::HandleEvent(anyhow!( - "Sending \"resize\" response: {:?}", - e - )) - })?; - return Ok(()); - } - - self.signal(VirtioInterruptType::Config).map_err(|e| { - let e = Error::ResizeTriggerFail(e); - let ret_err = anyhow!("{:?}", e); - self.resize - .send(Err(e)) - .map_err(|e| { - EpollHelperError::HandleEvent(anyhow!( - "Sending \"resize\" response: {:?}", - e - )) - }) - .ok(); - EpollHelperError::HandleEvent(ret_err) - })?; - - self.resize.send(Ok(())).map_err(|e| { - EpollHelperError::HandleEvent(anyhow!("Sending \"resize\" response: {:?}", e)) - })?; - } QUEUE_AVAIL_EVENT => { self.queue_evt.read().map_err(|e| { EpollHelperError::HandleEvent(anyhow!("Failed to get queue event: {:?}", e)) @@ -810,7 +698,6 @@ impl VersionMapped for MemState {} pub struct Mem { common: VirtioCommon, id: String, - resize: ResizeSender, host_addr: u64, host_fd: Option, config: Arc>, @@ -819,6 +706,7 @@ pub struct Mem { dma_mapping_handlers: Arc>>>, blocks_state: Arc>, exit_evt: EventFd, + interrupt_cb: Option>, } impl Mem { @@ -827,7 +715,6 @@ impl Mem { pub fn new( id: String, region: &Arc, - resize: ResizeSender, seccomp_action: SeccompAction, numa_node_id: Option, initial_size: u64, @@ -899,7 +786,6 @@ impl Mem { ..Default::default() }, id, - resize, host_addr: region.as_ptr() as u64, host_fd, config: Arc::new(Mutex::new(config)), @@ -908,9 +794,27 @@ impl Mem { dma_mapping_handlers: Arc::new(Mutex::new(BTreeMap::new())), blocks_state, exit_evt, + interrupt_cb: None, }) } + pub fn resize(&mut self, size: u64) -> result::Result<(), Error> { + let mut config = self.config.lock().unwrap(); + config.resize(size).map_err(|e| { + Error::ResizeError(anyhow!("Failed to update virtio configuration: {:?}", e)) + })?; + + if let Some(interrupt_cb) = self.interrupt_cb.as_ref() { + interrupt_cb + .trigger(VirtioInterruptType::Config) + .map_err(|e| { + Error::ResizeError(anyhow!("Failed to signal the guest about resize: {:?}", e)) + }) + } else { + Ok(()) + } + } + pub fn add_dma_mapping_handler( &mut self, source: VirtioMemMappingSource, @@ -1022,13 +926,14 @@ impl VirtioDevice for Mem { let (_, queue, queue_evt) = queues.remove(0); + self.interrupt_cb = Some(interrupt_cb.clone()); + let mut handler = MemEpollHandler { mem, host_addr: self.host_addr, host_fd: self.host_fd, blocks_state: Arc::clone(&self.blocks_state), config: self.config.clone(), - resize: self.resize.clone(), queue, interrupt_cb, queue_evt, @@ -1067,14 +972,11 @@ impl VirtioDevice for Mem { )?; self.common.epoll_threads = Some(epoll_threads); - self.resize.activated.store(true, Ordering::Release); - event!("virtio-device", "activated", "id", &self.id); Ok(()) } fn reset(&mut self) -> Option> { - self.resize.activated.store(false, Ordering::Release); let result = self.common.reset(); event!("virtio-device", "reset", "id", &self.id); result diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs index 20d7557da..672e4cb00 100644 --- a/vmm/src/device_manager.rs +++ b/vmm/src/device_manager.rs @@ -363,9 +363,6 @@ pub enum DeviceManagerError { /// Cannot create virtio-mem device CreateVirtioMem(io::Error), - /// Cannot generate a ResizeSender from the Resize object. - CreateResizeSender(virtio_devices::mem::Error), - /// Cannot find a memory range for virtio-mem memory VirtioMemRangeAllocation, @@ -2723,9 +2720,9 @@ impl DeviceManager { let mut devices = Vec::new(); let mm = self.memory_manager.clone(); - let mm = mm.lock().unwrap(); - for (memory_zone_id, memory_zone) in mm.memory_zones().iter() { - if let Some(virtio_mem_zone) = memory_zone.virtio_mem_zone() { + let mut mm = mm.lock().unwrap(); + for (memory_zone_id, memory_zone) in mm.memory_zones_mut().iter_mut() { + if let Some(virtio_mem_zone) = memory_zone.virtio_mem_zone_mut() { info!("Creating virtio-mem device: id = {}", memory_zone_id); let node_id = numa_node_id_from_memory_zone_id(&self.numa_nodes, memory_zone_id) @@ -2735,10 +2732,6 @@ impl DeviceManager { virtio_devices::Mem::new( memory_zone_id.clone(), virtio_mem_zone.region(), - virtio_mem_zone - .resize_handler() - .new_resize_sender() - .map_err(DeviceManagerError::CreateResizeSender)?, self.seccomp_action.clone(), node_id, virtio_mem_zone.hotplugged_size(), @@ -2751,6 +2744,11 @@ impl DeviceManager { .map_err(DeviceManagerError::CreateVirtioMem)?, )); + // Update the virtio-mem zone so that it has a handle onto the + // virtio-mem device, which will be used for triggering a resize + // if needed. + virtio_mem_zone.set_virtio_device(Arc::clone(&virtio_mem_device)); + self.virtio_mem_devices.push(Arc::clone(&virtio_mem_device)); devices.push(MetaVirtioDevice { diff --git a/vmm/src/memory_manager.rs b/vmm/src/memory_manager.rs index 9e68d2d84..f170d571e 100644 --- a/vmm/src/memory_manager.rs +++ b/vmm/src/memory_manager.rs @@ -87,7 +87,7 @@ struct HotPlugState { pub struct VirtioMemZone { region: Arc, - resize_handler: virtio_devices::Resize, + virtio_device: Option>>, hotplugged_size: u64, hugepages: bool, blocks_state: Arc>, @@ -97,8 +97,8 @@ impl VirtioMemZone { pub fn region(&self) -> &Arc { &self.region } - pub fn resize_handler(&self) -> &virtio_devices::Resize { - &self.resize_handler + pub fn set_virtio_device(&mut self, virtio_device: Arc>) { + self.virtio_device = Some(virtio_device); } pub fn hotplugged_size(&self) -> u64 { self.hotplugged_size @@ -130,6 +130,9 @@ impl MemoryZone { pub fn virtio_mem_zone(&self) -> &Option { &self.virtio_mem_zone } + pub fn virtio_mem_zone_mut(&mut self) -> Option<&mut VirtioMemZone> { + self.virtio_mem_zone.as_mut() + } } pub type MemoryZones = HashMap; @@ -579,8 +582,7 @@ impl MemoryManager { let region_size = region.len(); memory_zone.virtio_mem_zone = Some(VirtioMemZone { region, - resize_handler: virtio_devices::Resize::new(hotplugged_size) - .map_err(Error::EventFdFail)?, + virtio_device: None, hotplugged_size, hugepages: zone_config.hugepages, blocks_state: Arc::new(Mutex::new(BlocksState::new(region_size))), @@ -1002,8 +1004,7 @@ impl MemoryManager { let region_size = region.len(); memory_zone.virtio_mem_zone = Some(VirtioMemZone { region, - resize_handler: virtio_devices::Resize::new(hotplugged_size) - .map_err(Error::EventFdFail)?, + virtio_device: None, hotplugged_size, hugepages: zone.hugepages, blocks_state: Arc::new(Mutex::new(BlocksState::new(region_size))), @@ -1628,10 +1629,13 @@ impl MemoryManager { pub fn virtio_mem_resize(&mut self, id: &str, size: u64) -> Result<(), Error> { if let Some(memory_zone) = self.memory_zones.get_mut(id) { if let Some(virtio_mem_zone) = &mut memory_zone.virtio_mem_zone { - virtio_mem_zone - .resize_handler() - .work(size) - .map_err(Error::VirtioMemResizeFail)?; + if let Some(virtio_mem_device) = virtio_mem_zone.virtio_device.as_ref() { + virtio_mem_device + .lock() + .unwrap() + .resize(size) + .map_err(Error::VirtioMemResizeFail)?; + } // Keep the hotplugged_size up to date. virtio_mem_zone.hotplugged_size = size; @@ -1821,6 +1825,10 @@ impl MemoryManager { &self.memory_zones } + pub fn memory_zones_mut(&mut self) -> &mut MemoryZones { + &mut self.memory_zones + } + pub fn memory_range_table( &self, snapshot: bool,