Re: [PATCH v1 2/2] migration: Do not perform RAMBlock dirty sync during the first iteration

[David Hildenbrand <david@redhat.com>]

On 13.11.24 21:12, Peter Xu wrote:
> On Wed, Nov 13, 2024 at 07:49:44PM +0100, David Hildenbrand wrote:
>> I think I had precisely that, and I recall you suggested to have it only
>> after the initial sync. Would work for me, but I'd still like to understand
>> why essentially none of the "discard" was effective -- all of guest RAM got
>> touched.
> 
> Yes it'll be interesting to know..
> 
> One thing I'm wildly guessing is dirty_memory_extend(), so maybe after the
> ramblock is created nobody yet to clear the "1"s there for each of the
> client, including DIRTY_MEMORY_MIGRATION.  Then it'll be synced to ramblock
> bmap only in the initial sync, once for each qemu lifecycle.


In ram_block_add() we do the

cpu_physical_memory_set_dirty_range(new_block->offset,
				    new_block->used_length,
				    DIRTY_CLIENTS_ALL);

ramblock_dirty_bitmap_clear_discarded_pages()->...->migration_clear_memory_region_dirty_bitmap_range()->migration_clear_memory_region_dirty_bitmap() 
won't end up clearing the bits in the dirty bitmap.

First I thought because of:

if (!rb->clear_bmap || !clear_bmap_test_and_clear(rb, page)) {
     return;
}

But then I realized that even memory_region_clear_dirty_bitmap() will 
not clear the ramblock_dirty_bitmap_ bits! It's only concerned about 
listener->log_clear() calls.

Looking for DIRTY_MEMORY_BLOCK_SIZE users, only 
cpu_physical_memory_sync_dirty_bitmap() and 
cpu_physical_memory_clear_dirty_range() clear them, whereby the latter 
is only used when resizing RAMblocks.

At first I wondered whether 
ramblock_dirty_bitmap_clear_discarded_pages() should also call 
cpu_physical_memory_clear_dirty_range(), but then I am not so sure if 
that is really the right approach.


virtio-balloon() calls qemu_guest_free_page_hint() which calls

migration_clear_memory_region_dirty_bitmap_range()
bitmap_clear()

So it would maybe have the same issue.

-- 
Cheers,

David / dhildenb