Re: [PATCH] migration/postcopy: not necessary to discard all RAM at the beginning

[Wei Yang <richardw.yang@linux.intel.com>]

On Thu, Feb 13, 2020 at 10:17:04AM +0000, Dr. David Alan Gilbert wrote:
>* Wei Yang (richardw.yang@linux.intel.com) wrote:
>> ram_discard_range() unmap page for specific range. To be specific, this
>> clears related page table entries so that userfault would be triggered.
>> But this step is not necessary at the very beginning.
>> 
>> ram_postcopy_incoming_init() is called when destination gets ADVISE
>> command. ADVISE command is sent when migration thread just starts, which
>> implies destination is not running yet. This means no page fault
>> happened and memory region's page tables entries are empty.
>> 
>> This patch removes the discard at the beginning.
>> 
>> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
>> ---
>>  migration/postcopy-ram.c | 46 ----------------------------------------
>>  migration/postcopy-ram.h |  7 ------
>>  migration/ram.c          | 16 --------------
>>  migration/ram.h          |  1 -
>>  migration/savevm.c       |  4 ----
>>  5 files changed, 74 deletions(-)
>> 
>> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
>> index 5da6de8c8b..459be8e780 100644
>> --- a/migration/postcopy-ram.c
>> +++ b/migration/postcopy-ram.c
>> @@ -443,32 +443,6 @@ out:
>>      return ret;
>>  }
>>  
>> -/*
>> - * Setup an area of RAM so that it *can* be used for postcopy later; this
>> - * must be done right at the start prior to pre-copy.
>> - * opaque should be the MIS.
>> - */
>> -static int init_range(RAMBlock *rb, void *opaque)
>> -{
>> -    const char *block_name = qemu_ram_get_idstr(rb);
>> -    void *host_addr = qemu_ram_get_host_addr(rb);
>> -    ram_addr_t offset = qemu_ram_get_offset(rb);
>> -    ram_addr_t length = qemu_ram_get_used_length(rb);
>> -    trace_postcopy_init_range(block_name, host_addr, offset, length);
>> -
>> -    /*
>> -     * We need the whole of RAM to be truly empty for postcopy, so things
>> -     * like ROMs and any data tables built during init must be zero'd
>> -     * - we're going to get the copy from the source anyway.
>> -     * (Precopy will just overwrite this data, so doesn't need the discard)
>> -     */
>
>But this comment explains why we want to do the discard; we want to make
>sure that any memory that's been populated by the destination during the
>init process is discarded and replaced by content from the source.
>

OK, you are right. I missed the init stage.


>Dave
>

-- 
Wei Yang
Help you, Help me