Re: [PATCH v3 02/12] softmmu/physmem: Fix ram_block_discard_range() to handle shared anonymous memory

[David Hildenbrand]

On 11.03.21 18:22, Peter Xu wrote:
> On Thu, Mar 11, 2021 at 06:15:15PM +0100, David Hildenbrand wrote:
> > On 11.03.21 18:11, Peter Xu wrote:
> > > On Thu, Mar 11, 2021 at 05:45:46PM +0100, David Hildenbrand wrote:
> > > > On 11.03.21 17:39, Dr. David Alan Gilbert wrote:
> > > > > * David Hildenbrand (david@redhat.com) wrote:
> > > > > > We can create shared anonymous memory via
> > > > > >        "-object memory-backend-ram,share=on,..."
> > > > > > which is, for example, required by PVRDMA for mremap() to work.
> > > > > >
> > > > > > Shared anonymous memory is weird, though. Instead of MADV_DONTNEED, we
> > > > > > have to use MADV_REMOVE. MADV_DONTNEED fails silently and does nothing.
> > > > >
> > > > > OK, I wonder how stable these rules are; is it defined anywhere that
> > > > > it's required?
> > > >
> > > > I had a look at the Linux implementation: it's essentially shmem ... but we
> > > > don't have an fd exposed, so we cannot use fallocate() ... :)
> > > >
> > > > MADV_REMOVE documents (man):
> > > >
> > > > "In the initial implementation, only tmpfs(5) was supported MADV_REMOVE; but
> > > > since Linux 3.5, any filesystem which supports the fallocate(2)
> > > > FALLOC_FL_PUNCH_HOLE mode also supports MADV_REMOVE."
> > >
> > > Hmm, I see that MADV_DONTNEED will still tear down all mappings even for
> > > anonymous shmem.. what did I miss?
> >
> > Where did you see that?
>
> I see madvise_dontneed_free() calls zap_page_range().
>
> > >
> >
> > MADV_DONTNEED only invalidates private copies in the pagecache. It's
> > essentially useless for any kind of shared mappings.

Let me rephrase because it was wrong: MADV_DONTNEED invalidates private 
COW pages referenced in the page tables :)

> Since it's about zapping page tables, then I don't understand why it won't work
> for shmem..

It zaps the page tables but the shmem pages are still referenced (in the 
pagecache AFAIU). On next user space access, you would fill the page 
tables with the previous content.

That's why MADV_DONTNEED works properly on private anonymous memory, but 
not on shared anonymous memory - the only valid references are in the 
page tables in case of private mappings (well, unless we have other 
references like GUP etc.).


I did wonder, however, if there is benefit in doing both:

MADV_REMOVE followed by MADV_DONTNEED or the other way around. Like, 
will the extra MADV_DONTNEED also remove page tables and not just 
invalidate/zap the entries. Doesn't make a difference 
functionality-wise, but memory-consumption-wise.

I'll still have to have a look.

--
Thanks,

David / dhildenb