Re: [PATCH v2 8/9] util/mmap-alloc: Support RAM_NORESERVE via MAP_NORESERVE

[David Hildenbrand <david@redhat.com>]

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?

-- 
Thanks,

David / dhildenb