Re: [PATCH v4 8/9] migration/ram: Factor out populating pages readable in ram_block_populate_pages()

[David Hildenbrand]

On 03.09.21 00:28, Peter Xu wrote:
> On Thu, Sep 02, 2021 at 03:14:31PM +0200, David Hildenbrand wrote:
> > Let's factor out prefaulting/populating to make further changes easier to
> > review. While at it, use the actual page size of the ramblock, which
> > defaults to qemu_real_host_page_size for anonymous memory.
> >
> > Signed-off-by: David Hildenbrand <david@redhat.com>
> > ---
> >   migration/ram.c | 21 ++++++++++++---------
> >   1 file changed, 12 insertions(+), 9 deletions(-)
> >
> > diff --git a/migration/ram.c b/migration/ram.c
> > index e1c158dc92..de47650c90 100644
> > --- a/migration/ram.c
> > +++ b/migration/ram.c
> > @@ -1639,6 +1639,17 @@ out:
> >       return ret;
> >   }
> >   
> > +static inline void populate_range(RAMBlock *block, ram_addr_t offset,
> > +                                  ram_addr_t size)
> > +{
> > +    for (; offset < size; offset += block->page_size) {
> > +        char tmp = *((char *)block->host + offset);
> > +
> > +        /* Don't optimize the read out */
> > +        asm volatile("" : "+r" (tmp));
> > +    }
> > +}
>
> If to make it a common function, make it populate_range_read()?

Indeed, makes sense.

> Just to identify from RW, as we'll fill the holes with zero pages only, not
> doing page allocations yet, so not a complete "populate".

Well, depending on the actual memory backend ...

> That'll be good enough for live snapshot as uffd-wp works for zero pages,
> however I'm just afraid it may stop working for some new users of it when zero
> pages won't suffice.

I thought about that as well. But snapshots/migration will read all 
memory either way and consume real memory when there is no shared zero 
page. So it's just shifting the point in time when we allocate all these 
pages I guess.

> Maybe some comment would help too?
Yes, will do, thanks!

--
Thanks,

David / dhildenb