Re: [PATCH v3 3/7] support UFFD write fault processing in ram_save_iterate()

[Peter Xu <peterx@redhat.com>]

On Thu, Nov 19, 2020 at 03:59:36PM +0300, Andrey Gruzdev via wrote:

[...]

> +/**
> + * ram_find_block_by_host_address: find RAM block containing host page
> + *
> + * Returns true if RAM block is found and pss->block/page are
> + * pointing to the given host page, false in case of an error
> + *
> + * @rs: current RAM state
> + * @pss: page-search-status structure
> + */
> +static bool ram_find_block_by_host_address(RAMState *rs, PageSearchStatus *pss,
> +        hwaddr page_address)
> +{
> +    bool found = false;
> +
> +    pss->block = rs->last_seen_block;
> +    do {
> +        if (page_address >= (hwaddr) pss->block->host &&
> +            (page_address + TARGET_PAGE_SIZE) <=
> +                    ((hwaddr) pss->block->host + pss->block->used_length)) {
> +            pss->page = (unsigned long)
> +                    ((page_address - (hwaddr) pss->block->host) >> TARGET_PAGE_BITS);
> +            found = true;
> +            break;
> +        }
> +
> +        pss->block = QLIST_NEXT_RCU(pss->block, next);
> +        if (!pss->block) {
> +            /* Hit the end of the list */
> +            pss->block = QLIST_FIRST_RCU(&ram_list.blocks);
> +        }
> +    } while (pss->block != rs->last_seen_block);
> +
> +    rs->last_seen_block = pss->block;
> +    /*
> +     * Since we are in the same loop with ram_find_and_save_block(),
> +     * need to reset pss->complete_round after switching to
> +     * other block/page in pss.
> +     */
> +    pss->complete_round = false;
> +
> +    return found;
> +}

I forgot whether Denis and I have discussed this, but I'll try anyways... do
you think we can avoid touching PageSearchStatus at all?

PageSearchStatus is used to track a single migration iteration for precopy, so
that we scan from the 1st ramblock until the last one.  Then we finish one
iteration.

Snapshot is really something, imho, that can easily leverage this structure
without touching it - basically we want to do two things:

  - Do the 1st iteration of precopy (when ram_bulk_stage==true), and do that
    only.  We never need the 2nd, 3rd, ... iterations because we're snapshoting.

  - Leverage the postcopy queue mechanism so that when some page got written,
    queue that page.  We should have this queue higher priority than the
    precopy scanning mentioned above.

As long as we follow above rules, then after the above 1st round precopy, we're
simply done...  If that works, the whole logic of precopy and PageSearchStatus
does not need to be touched, iiuc.

[...]

> @@ -2086,7 +2191,8 @@ static void ram_state_reset(RAMState *rs)
>      rs->last_sent_block = NULL;
>      rs->last_page = 0;
>      rs->last_version = ram_list.version;
> -    rs->ram_bulk_stage = true;
> +    rs->ram_wt_enabled = migrate_track_writes_ram();

Maybe we don't need ram_wt_enabled, but just call migrate_track_writes_ram()
anywhere needed (actually, only in get_fault_page, once).

Thanks,

-- 
Peter Xu