| Message ID | 20210305101634.10745-9-david@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | RAM_NORESERVE, MAP_NORESERVE and hostmem "reserve" property | expand |
On Fri, Mar 05, 2021 at 11:16:33AM +0100, David Hildenbrand wrote: > +#define OVERCOMMIT_MEMORY_PATH "/proc/sys/vm/overcommit_memory" > +static bool map_noreserve_effective(int fd, bool readonly, bool shared) > +{ [...] > @@ -184,8 +251,7 @@ void *qemu_ram_mmap(int fd, > size_t offset, total; > void *ptr, *guardptr; > > - if (noreserve) { > - error_report("Skipping reservation of swap space is not supported"); > + if (noreserve && !map_noreserve_effective(fd, shared, readonly)) { Need to switch "shared" & "readonly"? > return MAP_FAILED; > }
On 05.03.21 16:42, Peter Xu wrote: > On Fri, Mar 05, 2021 at 11:16:33AM +0100, David Hildenbrand wrote: >> +#define OVERCOMMIT_MEMORY_PATH "/proc/sys/vm/overcommit_memory" >> +static bool map_noreserve_effective(int fd, bool readonly, bool shared) >> +{ > > [...] > >> @@ -184,8 +251,7 @@ void *qemu_ram_mmap(int fd, >> size_t offset, total; >> void *ptr, *guardptr; >> >> - if (noreserve) { >> - error_report("Skipping reservation of swap space is not supported"); >> + if (noreserve && !map_noreserve_effective(fd, shared, readonly)) { > > Need to switch "shared" & "readonly"? Indeed, interestingly it has the same effect (as we don't have anonymous read-only memory in QEMU :) ) (wouldn't have happened with flags ... hmm) Thanks!
On Fri, Mar 05, 2021 at 04:44:36PM +0100, David Hildenbrand wrote: > On 05.03.21 16:42, Peter Xu wrote: > > On Fri, Mar 05, 2021 at 11:16:33AM +0100, David Hildenbrand wrote: > > > +#define OVERCOMMIT_MEMORY_PATH "/proc/sys/vm/overcommit_memory" > > > +static bool map_noreserve_effective(int fd, bool readonly, bool shared) > > > +{ > > > > [...] > > > > > @@ -184,8 +251,7 @@ void *qemu_ram_mmap(int fd, > > > size_t offset, total; > > > void *ptr, *guardptr; > > > - if (noreserve) { > > > - error_report("Skipping reservation of swap space is not supported"); > > > + if (noreserve && !map_noreserve_effective(fd, shared, readonly)) { > > > > Need to switch "shared" & "readonly"? > > Indeed, interestingly it has the same effect (as we don't have anonymous > read-only memory in QEMU :) ) But note there is still a "g_assert(!shared || fd >= 0);" inside.. :) > > (wouldn't have happened with flags ... hmm) Right.
On 05.03.21 16:51, Peter Xu wrote: > On Fri, Mar 05, 2021 at 04:44:36PM +0100, David Hildenbrand wrote: >> On 05.03.21 16:42, Peter Xu wrote: >>> On Fri, Mar 05, 2021 at 11:16:33AM +0100, David Hildenbrand wrote: >>>> +#define OVERCOMMIT_MEMORY_PATH "/proc/sys/vm/overcommit_memory" >>>> +static bool map_noreserve_effective(int fd, bool readonly, bool shared) >>>> +{ >>> >>> [...] >>> >>>> @@ -184,8 +251,7 @@ void *qemu_ram_mmap(int fd, >>>> size_t offset, total; >>>> void *ptr, *guardptr; >>>> - if (noreserve) { >>>> - error_report("Skipping reservation of swap space is not supported"); >>>> + if (noreserve && !map_noreserve_effective(fd, shared, readonly)) { >>> >>> Need to switch "shared" & "readonly"? >> >> Indeed, interestingly it has the same effect (as we don't have anonymous >> read-only memory in QEMU :) ) > > But note there is still a "g_assert(!shared || fd >= 0);" inside.. :) > >> >> (wouldn't have happened with flags ... hmm) > > Right. > I'll probably go with /* Map PROT_READ instead of PROT_READ|PROT_WRITE. */ #define QEMU_RAM_MMAP_READONLY (1 << 0) /* Map MAP_SHARED instead of MAP_PRIVATE. */ #define QEMU_RAM_MMAP_SHARED (1 << 1) /* Map MAP_SYNC|MAP_SHARED_VALIDATE if possible, fallback and warn otherwise. */ #define QEMU_RAM_MMAP_PMEM (1 << 2) for qemu_ram_mmap(). qemu_anon_ram_alloc() will still have bools, but there it will at least be only two.
On 05.03.21 16:51, Peter Xu wrote: > On Fri, Mar 05, 2021 at 04:44:36PM +0100, David Hildenbrand wrote: >> On 05.03.21 16:42, Peter Xu wrote: >>> On Fri, Mar 05, 2021 at 11:16:33AM +0100, David Hildenbrand wrote: >>>> +#define OVERCOMMIT_MEMORY_PATH "/proc/sys/vm/overcommit_memory" >>>> +static bool map_noreserve_effective(int fd, bool readonly, bool shared) >>>> +{ >>> >>> [...] >>> >>>> @@ -184,8 +251,7 @@ void *qemu_ram_mmap(int fd, >>>> size_t offset, total; >>>> void *ptr, *guardptr; >>>> - if (noreserve) { >>>> - error_report("Skipping reservation of swap space is not supported"); >>>> + if (noreserve && !map_noreserve_effective(fd, shared, readonly)) { >>> >>> Need to switch "shared" & "readonly"? >> >> Indeed, interestingly it has the same effect (as we don't have anonymous >> read-only memory in QEMU :) ) > > But note there is still a "g_assert(!shared || fd >= 0);" inside.. :) Aaaaaand, I just figured that we actually can create shared anonymous memory in QEMU, simply via -object memory-backend-ram,share=on Introduced in 06329ccecfa0 ("mem: add share parameter to memory-backend-ram"). That's also where we introduced the "shared" flag for qemu_anon_ram_alloc(). That commit mentions a use case for "RDMA devices in order to remap non-contiguous QEMU virtual addresses to a contiguous virtual address range.". I fail to understand why that requires sharing RAM with child processes. Especially: a) qemu_ram_is_shared() returned false before patch #1. RAM_SHARED is never set. b) qemu_ram_remap() does not work as expected? c) ram_discard_range() is broken with shared anonymous memory. Instead of MADV_DONTNEED we need MADV_REMOVE. This looks like a partially broken feature and I wonder if there is an actual user. @Marcel, can you clarify if there is an actual use case for shared anonymous memory in QEMU? I.e., if the original use case that required that change is valid? (and why it wasn't able to just use proper shmem) Shared anonymous memory is weird.
Hi David, On Sun, Mar 7, 2021 at 3:18 PM David Hildenbrand <david@redhat.com> wrote: > On 05.03.21 16:51, Peter Xu wrote: > > On Fri, Mar 05, 2021 at 04:44:36PM +0100, David Hildenbrand wrote: > >> On 05.03.21 16:42, Peter Xu wrote: > >>> On Fri, Mar 05, 2021 at 11:16:33AM +0100, David Hildenbrand wrote: > >>>> +#define OVERCOMMIT_MEMORY_PATH "/proc/sys/vm/overcommit_memory" > >>>> +static bool map_noreserve_effective(int fd, bool readonly, bool > shared) > >>>> +{ > >>> > >>> [...] > >>> > >>>> @@ -184,8 +251,7 @@ void *qemu_ram_mmap(int fd, > >>>> size_t offset, total; > >>>> void *ptr, *guardptr; > >>>> - if (noreserve) { > >>>> - error_report("Skipping reservation of swap space is not > supported"); > >>>> + if (noreserve && !map_noreserve_effective(fd, shared, readonly)) > { > >>> > >>> Need to switch "shared" & "readonly"? > >> > >> Indeed, interestingly it has the same effect (as we don't have anonymous > >> read-only memory in QEMU :) ) > > > > But note there is still a "g_assert(!shared || fd >= 0);" inside.. :) > > Aaaaaand, I just figured that we actually can create shared anonymous > memory in QEMU, simply via > > -object memory-backend-ram,share=on > > Introduced in 06329ccecfa0 ("mem: add share parameter to > memory-backend-ram"). That's also where we introduced the "shared" flag > for qemu_anon_ram_alloc(). > > That commit mentions a use case for "RDMA devices in order to remap > non-contiguous QEMU virtual addresses to a contiguous virtual address > range.". I fail to understand why that requires sharing RAM with child > processes. > > Especially: > > a) qemu_ram_is_shared() returned false before patch #1. RAM_SHARED is > never set. > > b) qemu_ram_remap() does not work as expected? > > c) ram_discard_range() is broken with shared anonymous memory. Instead > of MADV_DONTNEED we need MADV_REMOVE. > > This looks like a partially broken feature and I wonder if there is an > actual user. > > @Marcel, can you clarify if there is an actual use case for shared > anonymous memory in QEMU? I.e., if the original use case that required > that change is valid? (and why it wasn't able to just use proper shmem) > > As you correctly stated, the PVRDMA device requires remapping of non-contiguous QEMU virtual addresses to a contiguous virtual address range. In order to do so it calls mremap (... , MREMAP_MAYMOVE | MREMAP_FIXED, ...) The above call succeeds only if the memory is marked as "shared". > Shared anonymous memory is weird. > In this case it is not about sharing the memory between different processes, but about being able to remap it. Thanks, Marcel > > -- > Thanks, > > David / dhildenb > >
On 07.03.21 15:11, Marcel Apfelbaum wrote: > Hi David, > > On Sun, Mar 7, 2021 at 3:18 PM David Hildenbrand <david@redhat.com > <mailto:david@redhat.com>> wrote: > > On 05.03.21 16:51, Peter Xu wrote: > > On Fri, Mar 05, 2021 at 04:44:36PM +0100, David Hildenbrand wrote: > >> On 05.03.21 16:42, Peter Xu wrote: > >>> On Fri, Mar 05, 2021 at 11:16:33AM +0100, David Hildenbrand wrote: > >>>> +#define OVERCOMMIT_MEMORY_PATH "/proc/sys/vm/overcommit_memory" > >>>> +static bool map_noreserve_effective(int fd, bool readonly, > bool shared) > >>>> +{ > >>> > >>> [...] > >>> > >>>> @@ -184,8 +251,7 @@ void *qemu_ram_mmap(int fd, > >>>> size_t offset, total; > >>>> void *ptr, *guardptr; > >>>> - if (noreserve) { > >>>> - error_report("Skipping reservation of swap space is > not supported"); > >>>> + if (noreserve && !map_noreserve_effective(fd, shared, > readonly)) { > >>> > >>> Need to switch "shared" & "readonly"? > >> > >> Indeed, interestingly it has the same effect (as we don't have > anonymous > >> read-only memory in QEMU :) ) > > > > But note there is still a "g_assert(!shared || fd >= 0);" inside.. :) > > Aaaaaand, I just figured that we actually can create shared anonymous > memory in QEMU, simply via > > -object memory-backend-ram,share=on > > Introduced in 06329ccecfa0 ("mem: add share parameter to > memory-backend-ram"). That's also where we introduced the "shared" flag > for qemu_anon_ram_alloc(). > > That commit mentions a use case for "RDMA devices in order to remap > non-contiguous QEMU virtual addresses to a contiguous virtual address > range.". I fail to understand why that requires sharing RAM with child > processes. > > Especially: > > a) qemu_ram_is_shared() returned false before patch #1. RAM_SHARED is > never set. > > b) qemu_ram_remap() does not work as expected? > > c) ram_discard_range() is broken with shared anonymous memory. Instead > of MADV_DONTNEED we need MADV_REMOVE. > > This looks like a partially broken feature and I wonder if there is an > actual user. > > @Marcel, can you clarify if there is an actual use case for shared > anonymous memory in QEMU? I.e., if the original use case that required > that change is valid? (and why it wasn't able to just use proper shmem) > > > As you correctly stated, the PVRDMA device requires remapping of > non-contiguous QEMU > virtual addresses to a contiguous virtual address range. > > In order to do so it calls > mremap (... , MREMAP_MAYMOVE | MREMAP_FIXED, ...) Thanks - I was missing who remaps and how (for a second I thought in another forked process). docs/pvrdma.txt seems to describe the situation. Having to use anonymous shared memory is a bit unfortunate. I yet haven't figured out how it is valid to remap parts of RAMBlocks to other locations via MREMAP_MAYMOVE. This sounds to me like we are punching holes into RAMBlocks - that can't be right. Or maybe we are just shuffling around pages within a RAMBlock such that we don't actually punch holes? Or does that happen when the source VM is stopped and won't ever run again?
Hi David, On Mon, Mar 8, 2021 at 10:45 AM David Hildenbrand <david@redhat.com> wrote: > On 07.03.21 15:11, Marcel Apfelbaum wrote: > > Hi David, > > > > On Sun, Mar 7, 2021 at 3:18 PM David Hildenbrand <david@redhat.com > > <mailto:david@redhat.com>> wrote: > > > > On 05.03.21 16:51, Peter Xu wrote: > > > On Fri, Mar 05, 2021 at 04:44:36PM +0100, David Hildenbrand wrote: > > >> On 05.03.21 16:42, Peter Xu wrote: > > >>> On Fri, Mar 05, 2021 at 11:16:33AM +0100, David Hildenbrand > wrote: > > >>>> +#define OVERCOMMIT_MEMORY_PATH > "/proc/sys/vm/overcommit_memory" > > >>>> +static bool map_noreserve_effective(int fd, bool readonly, > > bool shared) > > >>>> +{ > > >>> > > >>> [...] > > >>> > > >>>> @@ -184,8 +251,7 @@ void *qemu_ram_mmap(int fd, > > >>>> size_t offset, total; > > >>>> void *ptr, *guardptr; > > >>>> - if (noreserve) { > > >>>> - error_report("Skipping reservation of swap space is > > not supported"); > > >>>> + if (noreserve && !map_noreserve_effective(fd, shared, > > readonly)) { > > >>> > > >>> Need to switch "shared" & "readonly"? > > >> > > >> Indeed, interestingly it has the same effect (as we don't have > > anonymous > > >> read-only memory in QEMU :) ) > > > > > > But note there is still a "g_assert(!shared || fd >= 0);" > inside.. :) > > > > Aaaaaand, I just figured that we actually can create shared anonymous > > memory in QEMU, simply via > > > > -object memory-backend-ram,share=on > > > > Introduced in 06329ccecfa0 ("mem: add share parameter to > > memory-backend-ram"). That's also where we introduced the "shared" > flag > > for qemu_anon_ram_alloc(). > > > > That commit mentions a use case for "RDMA devices in order to remap > > non-contiguous QEMU virtual addresses to a contiguous virtual address > > range.". I fail to understand why that requires sharing RAM with > child > > processes. > > > > Especially: > > > > a) qemu_ram_is_shared() returned false before patch #1. RAM_SHARED is > > never set. > > > > b) qemu_ram_remap() does not work as expected? > > > > c) ram_discard_range() is broken with shared anonymous memory. > Instead > > of MADV_DONTNEED we need MADV_REMOVE. > > > > This looks like a partially broken feature and I wonder if there is > an > > actual user. > > > > @Marcel, can you clarify if there is an actual use case for shared > > anonymous memory in QEMU? I.e., if the original use case that > required > > that change is valid? (and why it wasn't able to just use proper > shmem) > > > > > > As you correctly stated, the PVRDMA device requires remapping of > > non-contiguous QEMU > > virtual addresses to a contiguous virtual address range. > > > > In order to do so it calls > > mremap (... , MREMAP_MAYMOVE | MREMAP_FIXED, ...) > > Thanks - I was missing who remaps and how (for a second I thought in > another forked process). > > docs/pvrdma.txt seems to describe the situation. Having to use anonymous > shared memory is a bit unfortunate. > > I yet haven't figured out how it is valid to remap parts of RAMBlocks to > other locations via MREMAP_MAYMOVE. This sounds to me like we are > punching holes into RAMBlocks - that can't be right. > Or maybe we are just shuffling around pages within a RAMBlock such that > we don't actually punch holes? > Indeed, we are adding a new mapping , but we leave the previous one in place. The VM will continue to work with the "original" RAM while the host RDMA subsystem will work with the re-mapped one. Thanks, Marcel > > Or does that happen when the source VM is stopped and won't ever run again? > > -- > Thanks, > > David / dhildenb > >
diff --git a/softmmu/physmem.c b/softmmu/physmem.c index 768e462529..3e85ca8898 100644 --- a/softmmu/physmem.c +++ b/softmmu/physmem.c @@ -2224,6 +2224,7 @@ void qemu_ram_remap(ram_addr_t addr, ram_addr_t length) abort(); } else { flags = MAP_FIXED; + flags |= (block->flags & RAM_NORESERVE) ? MAP_NORESERVE : 0; if (block->fd >= 0) { flags |= (block->flags & RAM_SHARED ? MAP_SHARED : MAP_PRIVATE); diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c index 397cb20a76..29c7a5035b 100644 --- a/util/mmap-alloc.c +++ b/util/mmap-alloc.c @@ -20,6 +20,7 @@ #include "qemu/osdep.h" #include "qemu/mmap-alloc.h" #include "qemu/host-utils.h" +#include "qemu/cutils.h" #include "qemu/error-report.h" #define HUGETLBFS_MAGIC 0x958458f6 @@ -120,7 +121,8 @@ static void *mmap_reserve(size_t size, int fd) * it accessible. */ static void *mmap_activate(void *ptr, size_t size, int fd, bool readonly, - bool shared, bool is_pmem, off_t map_offset) + bool shared, bool is_pmem, bool noreserve, + off_t map_offset) { const int prot = PROT_READ | (readonly ? 0 : PROT_WRITE); int map_sync_flags = 0; @@ -129,6 +131,7 @@ static void *mmap_activate(void *ptr, size_t size, int fd, bool readonly, flags |= fd == -1 ? MAP_ANONYMOUS : 0; flags |= shared ? MAP_SHARED : MAP_PRIVATE; + flags |= noreserve ? MAP_NORESERVE : 0; if (shared && is_pmem) { map_sync_flags = MAP_SYNC | MAP_SHARED_VALIDATE; } @@ -171,6 +174,70 @@ static inline size_t mmap_guard_pagesize(int fd) #endif } +#define OVERCOMMIT_MEMORY_PATH "/proc/sys/vm/overcommit_memory" +static bool map_noreserve_effective(int fd, bool readonly, bool shared) +{ +#if defined(__linux__) + gchar *content = NULL; + const char *endptr; + unsigned int tmp; + + /* + * Shared anonymous memory -- for sharing memory with forked children -- + * behaves in weird ways (i.e., in respect to memory accounting) and QEMU + * fortunately doesn't make use of it for guest RAM. Let's assert it + * remains that way. + */ + g_assert(!shared || fd >= 0); + + /* + * hugeltbfs accounting is different than ordinary swap reservation: + * a) Hugetlb pages from the pool are reserved for both private and + * shared mappings. For shared mappings, reservations are tracked + * per file - all mappers have to specify MAP_NORESERVE. + * b) MAP_NORESERVE is not affected by /proc/sys/vm/overcommit_memory. + */ + if (qemu_fd_getpagesize(fd) != qemu_real_host_page_size) { + return true; + } + + /* + * Accountable mappings in the kernel that can be affected by MAP_NORESEVE + * are private writable mappings (see mm/mmap.c:accountable_mapping() in + * Linux). For all shared or readonly mappings, MAP_NORESERVE is always + * implicitly active -- no reservation; this includes shmem. + */ + if (readonly || shared) { + return true; + } + + /* + * MAP_NORESERVE is globally ignored for private writable mappings when + * overcommit is set to "never". Sparse memory regions aren't really + * possible in this system configuration. + * + * Bail out now instead of silently committing way more memory than + * currently desired by the user. + */ + if (g_file_get_contents(OVERCOMMIT_MEMORY_PATH, &content, NULL, NULL) && + !qemu_strtoui(content, &endptr, 0, &tmp) && + (!endptr || *endptr == '\n')) { + if (tmp == 2) { + error_report("Skipping reservation of swap space is not supported:" + " \"" OVERCOMMIT_MEMORY_PATH "\" is \"2\""); + return false; + } + return true; + } + /* this interface has been around since Linux 2.6 */ + error_report("Skipping reservation of swap space is not supported:" + " Could not read: \"" OVERCOMMIT_MEMORY_PATH "\""); + return false; +#else + return true; +#endif +} + void *qemu_ram_mmap(int fd, size_t size, size_t align, @@ -184,8 +251,7 @@ void *qemu_ram_mmap(int fd, size_t offset, total; void *ptr, *guardptr; - if (noreserve) { - error_report("Skipping reservation of swap space is not supported"); + if (noreserve && !map_noreserve_effective(fd, shared, readonly)) { return MAP_FAILED; } @@ -207,7 +273,7 @@ void *qemu_ram_mmap(int fd, offset = QEMU_ALIGN_UP((uintptr_t)guardptr, align) - (uintptr_t)guardptr; ptr = mmap_activate(guardptr + offset, size, fd, readonly, shared, is_pmem, - map_offset); + noreserve, map_offset); if (ptr == MAP_FAILED) { munmap(guardptr, total); return MAP_FAILED;
Let's support RAM_NORESERVE via MAP_NORESERVE. At least on Linux, the flag has no effect on shared mappings - except for hugetlbfs. Linux man page: "MAP_NORESERVE: Do not reserve swap space for this mapping. When swap space is reserved, one has the guarantee that it is possible to modify the mapping. When swap space is not reserved one might get SIGSEGV upon a write if no physical memory is available. See also the discussion of the file /proc/sys/vm/overcommit_memory in proc(5). In kernels before 2.6, this flag had effect only for private writable mappings." Note that the "guarantee" part is wrong with memory overcommit in Linux. Also, in Linux hugetlbfs is treated differently - we configure reservation of huge pages from the pool, not reservation of swap space (huge pages cannot be swapped). The rough behavior is [1]: a) !Hugetlbfs: 1) Without MAP_NORESERVE *or* with memory overcommit under Linux disabled ("/proc/sys/vm/overcommit_memory == 2"), the following accounting/reservation happens: For a file backed map SHARED or READ-only - 0 cost (the file is the map not swap) PRIVATE WRITABLE - size of mapping per instance For an anonymous or /dev/zero map SHARED - size of mapping PRIVATE READ-only - 0 cost (but of little use) PRIVATE WRITABLE - size of mapping per instance 2) With MAP_NORESERVE, no accounting/reservation happens. b) Hugetlbfs: 1) Without MAP_NORESERVE, huge pages are reserved. 2) With MAP_NORESERVE, no huge pages are reserved. Note: With "/proc/sys/vm/overcommit_memory == 0", we were already able to configure it for !hugetlbfs globally; this toggle now allows configuring it more fine-grained, not for the whole system. The target use case is virtio-mem, which dynamically exposes memory inside a large, sparse memory area to the VM. [1] https://www.kernel.org/doc/Documentation/vm/overcommit-accounting Signed-off-by: David Hildenbrand <david@redhat.com> --- softmmu/physmem.c | 1 + util/mmap-alloc.c | 74 ++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 71 insertions(+), 4 deletions(-)