Re: [PATCH RFC 14/21] migration: Map hugetlbfs ramblocks twice, and pre-allocate

[Peter Xu <peterx@redhat.com>]

On Wed, Feb 01, 2023 at 07:53:28PM +0100, Juan Quintela wrote:
> Peter Xu <peterx@redhat.com> wrote:
> > On Mon, Jan 30, 2023 at 06:24:20AM +0100, Juan Quintela wrote:
> >> I would consider here:
> >> 
> >>     uint8_t *host_doublemap;
> >> 
> >> as I have not a small name that means
> >>     uint8_t *host_map_smaller_size_pages;
> >> 
> >> That explains why we need it.
> >
> > Sure, I can rename this one if it helps.
> >
> > One thing worth mention is that, it's not mapping things in small page size
> > here with host_doublemap but in huge page size only.
> 
> Thanks.
> 
> 
> > It's just that UFFDIO_CONTINUE needs another mapping to resolve the page
> > faults. It'll be the guest hugetlb ramblocks that will be mapped in small
> > pages during postcopy.
> 
> ok
> >> Not initialized variables, remove the last two.
> >
> > I can do this.
> >
> >> 
> >> > +    if (!migrate_hugetlb_doublemap()) {
> >> > +        return 0;
> >> > +    }
> >> > +
> >> 
> >> I would move the declaration of the RAMBlock here.
> >
> > But isn't QEMU in most cases declaring variables at the start of any code
> > block, rather than after or in the middle of any code segments?  IIRC some
> > compiler should start to fail with it, even though not on the modern ones.
> 
> We can declare variables since c99.  Only 24 years have passed O:-)

Oh OK :)

> 
> Anyways:
> 
> Exhibit A: We already have that kind of code
> 
> static int nocomp_send_prepare(MultiFDSendParams *p, Error **errp)
> {
>     MultiFDPages_t *pages = p->pages;
> 
>     for (int i = 0; i < p->normal_num; i++) {
>         p->iov[p->iovs_num].iov_base = pages->block->host + p->normal[i];
>         p->iov[p->iovs_num].iov_len = p->page_size;
>         p->iovs_num++;
>     }
> 
>     p->next_packet_size = p->normal_num * p->page_size;
>     p->flags |= MULTIFD_FLAG_NOCOMP;
>     return nocomp;
> }
> 
> Exhibit B:
> 
> from configure:
> 
> #if defined(__clang_major__) && defined(__clang_minor__)
> # ifdef __apple_build_version__
> #  if __clang_major__ < 10 || (__clang_major__ == 10 && __clang_minor__ < 0)
> #   error You need at least XCode Clang v10.0 to compile QEMU
> #  endif
> # else
> #  if __clang_major__ < 6 || (__clang_major__ == 6 && __clang_minor__ < 0)
> #   error You need at least Clang v6.0 to compile QEMU
> #  endif
> # endif
> #elif defined(__GNUC__) && defined(__GNUC_MINOR__)
> # if __GNUC__ < 7 || (__GNUC__ == 7 && __GNUC_MINOR__ < 4)
> #  error You need at least GCC v7.4.0 to compile QEMU
> # endif
> #else
> # error You either need GCC or Clang to compiler QEMU
> #endif
> int main (void) { return 0; }
> EOF
> 
> gcc-7.4.0: supports C11, so we are good here
> https://gcc.gnu.org/onlinedocs/gcc-7.4.0/gcc/Standards.html#C-Language
> 
> clang 6.0: supports c11 and c17 standard
> https://releases.llvm.org/6.0.0/tools/clang/docs/ReleaseNotes.html
> 
> 
> So as far as I can see, we are good here.

Thanks, I'll switch over.

-- 
Peter Xu