[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 5/6] migration/postcopy: enable random order target page arri
From: |
Wei Yang |
Subject: |
Re: [PATCH 5/6] migration/postcopy: enable random order target page arrival |
Date: |
Thu, 7 Nov 2019 14:00:11 +0800 |
User-agent: |
Mutt/1.9.4 (2018-02-28) |
On Wed, Nov 06, 2019 at 08:08:28PM +0000, Dr. David Alan Gilbert wrote:
>* Wei Yang (address@hidden) wrote:
>> After using number of target page received to track one host page, we
>> could have the capability to handle random order target page arrival in
>> one host page.
>>
>> This is a preparation for enabling compress during postcopy.
>>
>> Signed-off-by: Wei Yang <address@hidden>
>> ---
>> migration/ram.c | 16 +++-------------
>> 1 file changed, 3 insertions(+), 13 deletions(-)
>>
>> diff --git a/migration/ram.c b/migration/ram.c
>> index b5759793a9..da0596411c 100644
>> --- a/migration/ram.c
>> +++ b/migration/ram.c
>> @@ -4015,7 +4015,6 @@ static int ram_load_postcopy(QEMUFile *f)
>> MigrationIncomingState *mis = migration_incoming_get_current();
>> /* Temporary page that is later 'placed' */
>> void *postcopy_host_page = mis->postcopy_tmp_page;
>> - void *last_host = NULL;
>> bool all_zero = false;
>> int target_pages = 0;
>>
>> @@ -4062,24 +4061,15 @@ static int ram_load_postcopy(QEMUFile *f)
>> * that's moved into place later.
>> * The migration protocol uses, possibly smaller, target-pages
>> * however the source ensures it always sends all the components
>> - * of a host page in order.
>> + * of a host page in one chunk.
>> */
>> page_buffer = postcopy_host_page +
>> ((uintptr_t)host & (block->page_size - 1));
>> /* If all TP are zero then we can optimise the place */
>> if (target_pages == 1) {
>> all_zero = true;
>> - } else {
>> - /* not the 1st TP within the HP */
>> - if (host != (last_host + TARGET_PAGE_SIZE)) {
>> - error_report("Non-sequential target page %p/%p",
>> - host, last_host);
>> - ret = -EINVAL;
>> - break;
>> - }
>
>I think this is losing more protection than needed.
>I think you can still protect against a page from a different host-page
>arriving until we've placed the current host-page.
>So something like:
>
> if (((uintptr_t)host & ~(block->page_size - 1)) !=
> last_host)
>
OK, looks reasonable.
>and then set last_host to the start of the host page.
>
I think it is not necessary to update the last_host on each target page. We
can just set it at the first target page.
>Then you'll check if that flush is really working.
>
>Dave
>
>> }
>>
>> -
>> /*
>> * If it's the last part of a host page then we place the host
>> * page
>> @@ -4090,7 +4080,6 @@ static int ram_load_postcopy(QEMUFile *f)
>> }
>> place_source = postcopy_host_page;
>> }
>> - last_host = host;
>>
>> switch (flags & ~RAM_SAVE_FLAG_CONTINUE) {
>> case RAM_SAVE_FLAG_ZERO:
>> @@ -4143,7 +4132,8 @@ static int ram_load_postcopy(QEMUFile *f)
>>
>> if (!ret && place_needed) {
>> /* This gets called at the last target page in the host page */
>> - void *place_dest = host + TARGET_PAGE_SIZE - block->page_size;
>> + void *place_dest = (void *)QEMU_ALIGN_DOWN((unsigned long)host,
>> + block->page_size);
>>
>> if (all_zero) {
>> ret = postcopy_place_page_zero(mis, place_dest,
>> --
>> 2.17.1
>>
>--
>Dr. David Alan Gilbert / address@hidden / Manchester, UK
--
Wei Yang
Help you, Help me