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

[David Hildenbrand <david@redhat.com>]

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.

-- 
Thanks,

David / dhildenb