>
> On Sun, Mar 3, 2024 at 11:16 PM Peter Xu <pet...@redhat.com> wrote:
>
> >
> > On Fri, Mar 01, 2024 at 02:28:24AM +0000, Hao Xiang wrote:
> >
> > -GlobalProperty hw_compat_8_2[] = {};
> >
> > +GlobalProperty hw_compat_8_2[] = {
> >
> > + { "migration", "zero-page-detection", "legacy"},
> >
> > +};
> >
> > I hope we can make it for 9.0, then this (and many rest places) can be kept
> >
> > as-is. Let's see.. soft-freeze is March 12th.
> >
> > One thing to mention is I just sent a pull which has mapped-ram feature
> >
> > merged. You may need a rebase onto that, and hopefully mapped-ram can also
> >
> > use your feature too within the same patch when you repost.
> >
> > https://lore.kernel.org/all/20240229153017.2221-1-faro...@suse.de/
> >
> > That rebase may or may not need much caution, I apologize for that:
> >
> > mapped-ram as a feature was discussed 1+ years, so it was a plan to merge
> >
> > it (actually still partly of it) into QEMU 9.0.
Let's see if we can catch that.
> >
> > [...]
> >
> > +static bool multifd_zero_page(void)
> >
> > multifd_zero_page_enabled()?
Changed.
> >
> > +{
> >
> > + return migrate_zero_page_detection() == ZERO_PAGE_DETECTION_MULTIFD;
> >
> > +}
> >
> > +
> >
> > +static void swap_page_offset(ram_addr_t *pages_offset, int a, int b)
> >
> > +{
> >
> > + ram_addr_t temp;
> >
> > +
> >
> > + if (a == b) {
> >
> > + return;
> >
> > + }
> >
> > +
> >
> > + temp = pages_offset[a];
> >
> > + pages_offset[a] = pages_offset[b];
> >
> > + pages_offset[b] = temp;
> >
> > +}
> >
> > +
> >
> > +/**
> >
> > + * multifd_send_zero_page_check: Perform zero page detection on all pages.
> >
> > + *
> >
> > + * Sorts normal pages before zero pages in p->pages->offset and updates
> >
> > + * p->pages->normal_num.
> >
> > + *
> >
> > + * @param p A pointer to the send params.
> >
> > Nit: the majority of doc style in QEMU (it seems to me) is:
> >
> > @p: pointer to @MultiFDSendParams.
> >
> > + */
> >
> > +void multifd_send_zero_page_check(MultiFDSendParams *p)
> >
> > multifd_send_zero_page_detect()?
> >
> > This patch used "check" on both sides, but neither of them is a pure check
> >
> > to me. For the other side, maybe multifd_recv_zero_page_process()? As
> >
> > that one applies the zero pages.
Renamed.
> >
> > +{
> >
> > + MultiFDPages_t *pages = p->pages;
> >
> > + RAMBlock *rb = pages->block;
> >
> > + int i = 0;
> >
> > + int j = pages->num - 1;
> >
> > +
> >
> > + /*
> >
> > + * QEMU older than 9.0 don't understand zero page
> >
> > + * on multifd channel. This switch is required to
> >
> > + * maintain backward compatibility.
> >
> > + */
> >
> > IMHO we can drop this comment; it is not accurate as the user can disable
> >
> > it explicitly through the parameter, then it may not always about
> > compatibility.
Dropped.
> >
> > + if (multifd_zero_page()) {
> >
> > Shouldn't this be "!multifd_zero_page_enabled()"?
Thanks for catching this! My bad. Fixed.
> >
> > + pages->normal_num = pages->num;
> >
> > + return;
> >
> > + }
> >
> > The rest looks all sane.
> >
> > Thanks,
> >
> > --
> >
> > Peter Xu
> >
>
