Re: [PATCH v9 0/7] Allow to enable multifd and postcopy migration together

[Peter Xu <peterx@redhat.com>]

On Tue, Apr 29, 2025 at 06:21:13PM +0530, Prasad Pandit wrote:
> Hi,
> 
> > On Thu, Apr 17, 2025 at 01:05:37PM -0300, Fabiano Rosas wrote:
> > > It's not that page faults happen during multifd. The page was already
> > > sent during precopy, but multifd-recv didn't write to it, it just marked
> > > the receivedmap. When postcopy starts, the page gets accessed and
> > > faults. Since postcopy is on, the migration wants to request the page
> > > from the source, but it's present in the receivedmap, so it doesn't
> > > ask. No page ever comes and the code hangs waiting for the page fault to
> > > be serviced (or potentially faults continuously? I'm not sure on the
> > > details).
> >
> > I think your previous analysis is correct on the zero pages.  I am not 100%
> > sure if that's the issue but very likely.  I tend to also agree with you
> > that we could skip zero page optimization in multifd code when postcopy is
> > enabled (maybe plus some comment right above..).
> 
>    migration/multifd: solve zero page causing multiple page faults
>      -> https://gitlab.com/qemu-project/qemu/-/commit/5ef7e26bdb7eda10d6d5e1b77121be9945e5e550
> 
> * Is this the optimization that is causing the migration hang issue?

I think that's what Fabiano mentioned, but ultimately we need to verify it
on a reproducer to know.

> 
> ===
> diff --git a/migration/multifd-zero-page.c b/migration/multifd-zero-page.c
> index dbc1184921..00f69ff965 100644
> --- a/migration/multifd-zero-page.c
> +++ b/migration/multifd-zero-page.c
> @@ -85,7 +85,8 @@ void multifd_recv_zero_page_process(MultiFDRecvParams *p)
>  {
>      for (int i = 0; i < p->zero_num; i++) {
>          void *page = p->host + p->zero[i];
> -        if (ramblock_recv_bitmap_test_byte_offset(p->block, p->zero[i])) {
> +        if (!migrate_postcopy() &&
> +            ramblock_recv_bitmap_test_byte_offset(p->block, p->zero[i])) {
>              memset(page, 0, multifd_ram_page_size());
>          } else {
>              ramblock_recv_bitmap_set_offset(p->block, p->zero[i]);
> ===
> 
> * Would the above patch help to resolve it?

Looks ok, but please add some comments explain why postcopy needs to do it,
and especially do it during precopy phase.

I'd use migrate_postcopy_ram() instead. I wished migrate_dirty_bitmaps()
has a better name, maybe migrate_postcopy_block()..  I have totally no idea
who is using the feature, especially when postcopy-ram is off.

> 
> * Another way could be when the page fault occurs during postcopy
> phase, if we know (from receivedmap) that the faulted page is a
> zero-page, maybe we could write it locally on the destination to
> service the page-fault?

I don't think we can know that - receivedmap set doesn't mean it's a zero
page, but only says it's been received before.  It can also happen e.g. >1
threads faulted on the same page then the 2nd thread faulted on it may see
receivedmap set because the 1st thread got faulted already got the fault
resolved.

-- 
Peter Xu