Re: [PATCH v3 6/7] migration/postcopy: Handle RAMBlocks with a RamDiscardManager on the destination

From: David Hildenbrand <david@redhat.com>
To: "Philippe Mathieu-Daudé" <philmd@redhat.com>, qemu-devel@nongnu.org
Cc: Alex Williamson <alex.williamson@redhat.com>,
	Eduardo Habkost <ehabkost@redhat.com>,
	Juan Quintela <quintela@redhat.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>,
	Peter Xu <peterx@redhat.com>,
	Andrey Gruzdev <andrey.gruzdev@virtuozzo.com>,
	Pankaj Gupta <pankaj.gupta@cloud.ionos.com>,
	teawater <teawaterz@linux.alibaba.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Marek Kedzierski <mkedzier@redhat.com>,
	Wei Yang <richard.weiyang@linux.alibaba.com>
Subject: Re: [PATCH v3 6/7] migration/postcopy: Handle RAMBlocks with a RamDiscardManager on the destination
Date: Thu, 5 Aug 2021 10:07:20 +0200	[thread overview]
Message-ID: <a5ef8f64-0336-c5fa-a81e-2caed5296dee@redhat.com> (raw)
In-Reply-To: <5f8c6173-046d-9fc2-c649-93ede45ca77d@redhat.com>

On 05.08.21 09:48, Philippe Mathieu-Daudé wrote:
> On 7/30/21 10:52 AM, David Hildenbrand wrote:
>> Currently, when someone (i.e., the VM) accesses discarded parts inside a
>> RAMBlock with a RamDiscardManager managing the corresponding mapped memory
>> region, postcopy will request migration of the corresponding page from the
>> source. The source, however, will never answer, because it refuses to
>> migrate such pages with undefined content ("logically unplugged"): the
>> pages are never dirty, and get_queued_page() will consequently skip
>> processing these postcopy requests.
>>
>> Especially reading discarded ("logically unplugged") ranges is supposed to
>> work in some setups (for example with current virtio-mem), although it
>> barely ever happens: still, not placing a page would currently stall the
>> VM, as it cannot make forward progress.
>>
>> Let's check the state via the RamDiscardManager (the state e.g.,
>> of virtio-mem is migrated during precopy) and avoid sending a request
>> that will never get answered. Place a fresh zero page instead to keep
>> the VM working. This is the same behavior that would happen
>> automatically without userfaultfd being active, when accessing virtual
>> memory regions without populated pages -- "populate on demand".
>>
>> For now, there are valid cases (as documented in the virtio-mem spec) where
>> a VM might read discarded memory; in the future, we will disallow that.
>> Then, we might want to handle that case differently, e.g., warning the
>> user that the VM seems to be mis-behaving.
>>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>>   migration/postcopy-ram.c | 31 +++++++++++++++++++++++++++----
>>   migration/ram.c          | 21 +++++++++++++++++++++
>>   migration/ram.h          |  1 +
>>   3 files changed, 49 insertions(+), 4 deletions(-)
>>
>> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
>> index 2e9697bdd2..38cdfc09c3 100644
>> --- a/migration/postcopy-ram.c
>> +++ b/migration/postcopy-ram.c
>> @@ -671,6 +671,29 @@ int postcopy_wake_shared(struct PostCopyFD *pcfd,
>>       return ret;
>>   }
>>   
>> +static int postcopy_request_page(MigrationIncomingState *mis, RAMBlock *rb,
>> +                                 ram_addr_t start, uint64_t haddr)
>> +{
>> +    void *aligned = (void *)(uintptr_t)(haddr & -qemu_ram_pagesize(rb));
> 
>    void *aligned = QEMU_ALIGN_PTR_DOWN(haddr, qemu_ram_pagesize(rb)));
> 

Does not compile as haddr is not a pointer.

void *aligned = QEMU_ALIGN_PTR_DOWN((void *)haddr, qemu_ram_pagesize(rb)));

should work.

I can also add a patch to adjust a similar call in
migrate_send_rp_req_pages()!

Thanks!

-- 
Thanks,

David / dhildenb