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

[David Hildenbrand <david@redhat.com>]

On 19.03.21 10: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
> 
>> +        /* 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)

I wanted to do a "= *(ptr + offset)" here.

> 
> 	/* Don't optimize the read out. */
> 	asm volatile ("" : "+r" (tmp));

So this is the only volatile thing that the compiler must guarantee to 
not optimize away.


-- 
Thanks,

David / dhildenb