Re: [PATCH 2/3] exec: posix_madvise usage on SunOS.

[David Hildenbrand <david@redhat.com>]

On 21.07.20 11:31, Peter Maydell wrote:
> On Tue, 21 Jul 2020 at 09:22, David Hildenbrand <david@redhat.com> wrote:
>> virtio-mem depends on Linux (hw/virtio/Kconfig). I guess
>> userfaultfd/postcopy is also not relevant in the context of SunOS. So
>> what remains is virtio-balloon.
>>
>> virito-balloon ideally wants to discard the actual mapped pages to free
>> up memory. When memory is re-accessed, a fresh page is faulted in (->
>> zero-page under Linux). Now, we already have other cases where it looks
>> like "the balloon works" but it really doesn't. One example is using
>> vfio+virtio-balloon under Linux - inflating the balloon is simply a NOP,
>> no memory is actually discarded.
>>
>> I agree that POSIX_MADV_DONTNEED is not a proper match - different
>> guarantees. If SunOS cannot implement ram_block_discard_range() as
>> documented, we should disable it.
> 
> Could we also improve the documentation comment to make it clearer
> what ram_block_discard_range() is actually supposed to be doing?
> At the moment I don't think you can figure it out from the comments,
> which are a confusing mix of claiming it unmaps memory and that it
> zeroes it.

Certainly. It really means (as the name suggests) "Discard the pages
used for backing the virtual address range, effectively freeing them
back to the OS. When accessing pages within the virtual address range
again, populate fresh, zeroed-out pages, just as when accessing memory
inside a new mmap for the first time.". It really tries to mimic the
"populate on demand" mechanism used on fresh, anonymous mmaps. If no
page is populated, a page fault in the OS is triggered and a fresh
(physical) page is populated. Of course, when only reading, the single
COW zero page can be used until written to the page.

"Unmap" is a misleading terminology, I agree.

> 
> Is the Linux-specific stuff (userfaultfd) a *requirement* for the
> function, or just a "this would be nice" extra and a valid
> implementation would be eg "zero out the memory" ?

postcopy/userfaultfd really needs all backing pages for the VMA to be
gone (discarded/zapped), so that a pagefault will be triggered. As
postcopy will resolve the pagefault itself, it does not rely on the
"zeroed" semantics. It will tell the OS which page to place.

Supplying anything not zero already smells like the original request
(discard the pages) was not properly implemented. Of course, one could
supply random data to a user space process on a pagefault, when
populating pages, but that already smells like a severe security issue ...

Simply zeroing out memory is not what this function promises. It does
not work with all three use cases: postcopy, virito-balloon and
virtio-mem. We really have to discard.

-- 
Thanks,

David / dhildenb