From 99d9a3d299343255efedf5805bd4a6d9e31df144 Mon Sep 17 00:00:00 2001 From: Rob Bradford Date: Sun, 30 Oct 2022 10:32:52 +0000 Subject: [PATCH] vmm: memory_manager: Avoid MAP_PRIVATE CoW with VFIO for hugepages too We can't use MAP_ANONYMOUS and still have huge pages so MAP_SHARED is effectively required when using huge pages. Unfortunately it is not as simple as always forcing MAP_SHARED if hugepages is on as this might be inappropriate in the backing file case hence why there is additional complexity of assigning to mmap_flags on each case and the MAP_SHARED is only turned on for the anonymous file huge page case as well as anonymous shared file case. See: #4805 Signed-off-by: Rob Bradford --- vmm/src/memory_manager.rs | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/vmm/src/memory_manager.rs b/vmm/src/memory_manager.rs index d84af91f8..d422a2ab9 100644 --- a/vmm/src/memory_manager.rs +++ b/vmm/src/memory_manager.rs @@ -1299,30 +1299,36 @@ impl MemoryManager { host_numa_node: Option, existing_memory_file: Option, ) -> Result, Error> { + let mut mmap_flags = libc::MAP_NORESERVE; + + // The duplication of mmap_flags ORing here is unfortunate but it also makes + // the complexity of the handling clear. let fo = if let Some(f) = existing_memory_file { + // It must be MAP_SHARED as we wouldn't already have an FD + mmap_flags |= libc::MAP_SHARED; Some(FileOffset::new(f, file_offset)) } else if let Some(backing_file) = backing_file { + if shared { + mmap_flags |= libc::MAP_SHARED; + } else { + mmap_flags |= libc::MAP_PRIVATE; + } Some(Self::open_backing_file(backing_file, file_offset, size)?) } else if shared || hugepages { + // For hugepages we must also MAP_SHARED otherwise we will trigger #4805 + // because the MAP_PRIVATE will trigger CoW against the backing file with + // the VFIO pinning + mmap_flags |= libc::MAP_SHARED; Some(Self::create_anonymous_file(size, hugepages, hugepage_size)?) } else { + mmap_flags |= libc::MAP_PRIVATE | libc::MAP_ANONYMOUS; None }; - let mut mmap_flags = libc::MAP_NORESERVE - | if shared { - libc::MAP_SHARED - } else { - libc::MAP_PRIVATE - }; if prefault { mmap_flags |= libc::MAP_POPULATE; } - if fo.is_none() { - mmap_flags |= libc::MAP_ANONYMOUS; - } - let region = GuestRegionMmap::new( MmapRegion::build(fo, size, libc::PROT_READ | libc::PROT_WRITE, mmap_flags) .map_err(Error::GuestMemoryRegion)?,