Re: [PATCH 3/3] migration: Pre-fault memory before starting background snasphot

From: Andrey Gruzdev <andrey.gruzdev@virtuozzo.com>
To: David Hildenbrand <david@redhat.com>, qemu-devel@nongnu.org
Cc: Den Lunev <den@openvz.org>, Eric Blake <eblake@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Juan Quintela <quintela@redhat.com>,
	"Dr . David Alan Gilbert" <dgilbert@redhat.com>,
	Markus Armbruster <armbru@redhat.com>,
	Peter Xu <peterx@redhat.com>
Subject: Re: [PATCH 3/3] migration: Pre-fault memory before starting background snasphot
Date: Fri, 19 Mar 2021 15:37:27 +0300	[thread overview]
Message-ID: <abac66d9-7c5a-b186-ec41-2abc9e724ac4@virtuozzo.com> (raw)
In-Reply-To: <7a83e101-ea14-df93-3cbc-014279637d2e@redhat.com>

On 19.03.2021 14:27, David Hildenbrand wrote:
> On 19.03.21 12:05, Andrey Gruzdev wrote:
>> On 19.03.2021 12:28, David Hildenbrand wrote:
>>>> +/*
>>>> + * ram_block_populate_pages: populate memory in the RAM block by
>>>> reading
>>>> + *   an integer from the beginning of each page.
>>>> + *
>>>> + * Since it's solely used for userfault_fd WP feature, here we just
>>>> + *   hardcode page size to TARGET_PAGE_SIZE.
>>>> + *
>>>> + * @bs: RAM block to populate
>>>> + */
>>>> +volatile int ram_block_populate_pages__tmp;
>>>> +static void ram_block_populate_pages(RAMBlock *bs)
>>>> +{
>>>> +    ram_addr_t offset = 0;
>>>> +    int tmp = 0;
>>>> +
>>>> +    for (char *ptr = (char *) bs->host; offset < bs->used_length;
>>>> +            ptr += TARGET_PAGE_SIZE, offset += TARGET_PAGE_SIZE) {
>>>
>>> You'll want qemu_real_host_page_size instead of TARGET_PAGE_SIZE
>>>
>> Ok.
>>>> +        /* Try to do it without memory writes */
>>>> +        tmp += *(volatile int *) ptr;
>>>> +    }
>>>
>>>
>>> The following is slightly simpler and doesn't rely on volatile
>>> semantics [1].
>>> Should work on any arch I guess.
>>>
>>> static void ram_block_populate_pages(RAMBlock *bs)
>>> {
>>>      char *ptr = (char *) bs->host;
>>>      ram_addr_t offset;
>>>
>>>      for (offset = 0; offset < bs->used_length;
>>>           offset += qemu_real_host_page_size) {
>>>      char tmp = *(volatile char *)(ptr + offset)
>>>
>>>      /* Don't optimize the read out. */
>>>      asm volatile ("" : "+r" (tmp));
>>> }
>>>
>> Thanks, good option, I'll change the code.
>>
>>> Compiles to
>>>
>>>      for (offset = 0; offset < bs->used_length;
>>>      316d:       48 8b 4b 30             mov 0x30(%rbx),%rcx
>>>      char *ptr = (char *) bs->host;
>>>      3171:       48 8b 73 18             mov 0x18(%rbx),%rsi
>>>      for (offset = 0; offset < bs->used_length;
>>>      3175:       48 85 c9                test   %rcx,%rcx
>>>      3178:       74 ce                   je     3148
>>> <ram_write_tracking_prepare+0x58>
>>>           offset += qemu_real_host_page_size) {
>>>      317a:       48 8b 05 00 00 00 00    mov 0x0(%rip),%rax        #
>>> 3181 <ram_write_tracking_prepare+0x91>
>>>      3181:       48 8b 38                mov    (%rax),%rdi
>>>      3184:       0f 1f 40 00             nopl   0x0(%rax)
>>>          char tmp = *(volatile char *)(ptr + offset);
>>>      3188:       48 8d 04 16             lea (%rsi,%rdx,1),%rax
>>>      318c:       0f b6 00                movzbl (%rax),%eax
>>>           offset += qemu_real_host_page_size) {
>>>      318f:       48 01 fa                add    %rdi,%rdx
>>>      for (offset = 0; offset < bs->used_length;
>>>      3192:       48 39 ca                cmp    %rcx,%rdx
>>>      3195:       72 f1                   jb     3188
>>> <ram_write_tracking_prepare+0x98>
>>>
>>>
>>> [1]
>>> https://programfan.github.io/blog/2015/04/27/prevent-gcc-optimize-away-code/ 
>>>
>>>
>>>
>>> I'll send patches soon to take care of virtio-mem via 
>>> RamDiscardManager -
>>> to skip populating the parts that are supposed to remain discarded and
>>> not migrated.
>>> Unfortunately, the RamDiscardManager patches are still stuck waiting 
>>> for
>>> acks ... and now we're in soft-freeze.
>>>
>> RamDiscardManager patches - do they also modify migration code?
>> I mean which part is responsible of not migrating discarded ranges.
>
> I haven't shared relevant patches yet that touch migration code. I'm 
> planning on doing that once the generic RamDiscardManager has all 
> relevant acks. I'll put you on cc.
>
Got it, thanks.

-- 
Andrey Gruzdev, Principal Engineer
Virtuozzo GmbH  +7-903-247-6397
                 virtuzzo.com