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

From: Marcel Apfelbaum <mapfelba@redhat.com>
To: David Hildenbrand <david@redhat.com>
Cc: "Juan Quintela" <quintela@redhat.com>,
	"Thomas Huth" <thuth@redhat.com>,
	"Cornelia Huck" <cohuck@redhat.com>,
	"Eduardo Habkost" <ehabkost@redhat.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	"Stefan Weil" <sw@weilnetz.de>,
	"Murilo Opsfelder Araujo" <muriloo@linux.ibm.com>,
	"Richard Henderson" <richard.henderson@linaro.org>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>,
	"Peter Xu" <peterx@redhat.com>,
	qemu-devel@nongnu.org, "Halil Pasic" <pasic@linux.ibm.com>,
	"Christian Borntraeger" <borntraeger@de.ibm.com>,
	"Greg Kurz" <groug@kaod.org>,
	"Stefan Hajnoczi" <stefanha@redhat.com>,
	"Igor Mammedov" <imammedo@redhat.com>,
	"Marcel Apfelbaum" <marcel@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>,
	"Igor Kotrasinski" <i.kotrasinsk@partner.samsung.com>
Subject: Re: [PATCH v2 8/9] util/mmap-alloc: Support RAM_NORESERVE via MAP_NORESERVE
Date: Sun, 7 Mar 2021 16:11:51 +0200	[thread overview]
Message-ID: <CA+aaKfAc95Y2Eg2AkHELTzOa8DUt=-8feOufuUHF1jbFg5Z8SQ@mail.gmail.com> (raw)
In-Reply-To: <26dc6c36-5137-5d5e-36f0-2650e42e40ad@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 2687 bytes --]

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
>
>

[-- Attachment #2: Type: text/html, Size: 3959 bytes --]