Re: [PATCH v10 3/3] migration: write zero pages when postcopy enabled

[Fabiano Rosas <farosas@suse.de>]

Prasad Pandit <ppandit@redhat.com> writes:

> From: Prasad Pandit <pjp@fedoraproject.org>
>
> During multifd migration, zero pages are are written if
> they are migrated more than ones.

s/ones/once/

>
> This may result in a migration hang issue when Multifd
> and Postcopy are enabled together.
>
> When Postcopy is enabled, always write zero pages as and
> when they are migrated.
>
> Signed-off-by: Prasad Pandit <pjp@fedoraproject.org>

This patch should come before 1/3, otherwise it'll break bisect.

> ---
>  migration/multifd-zero-page.c | 22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
>
> v10: new patch, not present in v9 or earlier versions.
>
> diff --git a/migration/multifd-zero-page.c b/migration/multifd-zero-page.c
> index dbc1184921..9bfb3ef803 100644
> --- a/migration/multifd-zero-page.c
> +++ b/migration/multifd-zero-page.c
> @@ -85,9 +85,27 @@ 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])) {
> +
> +        /*
> +         * During multifd migration zero page is written to the memory
> +         * only if it is migrated more than ones.

s/ones/once/

> +         *
> +         * It becomes a problem when both Multifd & Postcopy options are
> +         * enabled. If the zero page which was skipped during multifd phase,
> +         * is accessed during the Postcopy phase of the migration, a page
> +         * fault occurs. But this page fault is not served because the
> +         * 'receivedmap' says the zero page is already received. Thus the
> +         * migration hangs.
> +         *
> +         * When Postcopy is enabled, always write the zero page as and when
> +         * it is migrated.
> +         *

extra blank line here^

> +         */

nit: Inconsistent use of capitalization for the feature names. I'd keep
it all lowercase.

> +        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]);
>          }
>      }