On Tue, Apr 29, 2025 at 06:58:29PM +0530, Prasad Pandit wrote:
> On Tue, 29 Apr 2025 at 18:34, Peter Xu <pet...@redhat.com> wrote:
> > I think that's what Fabiano mentioned, but ultimately we need to verify it
> > on a reproducer to know.
> ...
> > 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.
> 
> * Okay. It should be '||' instead of '&&' in the first conditional I
> think, we want to write zeropage when postcopy is enabled.
> ===
> diff --git a/migration/multifd-zero-page.c b/migration/multifd-zero-page.c
> index dbc1184921..4d6677feab 100644
> --- a/migration/multifd-zero-page.c
> +++ b/migration/multifd-zero-page.c
> @@ -85,9 +85,11 @@ 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_ram() ||
> +            ramblock_recv_bitmap_test_byte_offset(p->block, p->zero[i])) {
>              memset(page, 0, multifd_ram_page_size());
> -        } else {
> +        }
> +        if (!ramblock_recv_bitmap_test_byte_offset(p->block, p->zero[i])) {
>              ramblock_recv_bitmap_set_offset(p->block, p->zero[i]);
>          }
>      }
> ===
> * I'll send this one if it looks okay.

Please don't rush to send. Again, let's verify the issue first before
resending anything.

If you could reproduce it it would be perfect, then we can already verify
it.  Otherwise we may need help from Fabiano.  Let's not send anything if
you're not yet sure whether it works..  It can confuse people thinking
problem solved, but maybe not yet.

-- 
Peter Xu


Reply via email to