[PATCHv2,8/9] migration: do not search dirty pages in bulk stage

avoid searching for dirty pages just increment the
page offset. all pages are dirty anyway.

Signed-off-by: Peter Lieven <pl@kamp.de>
---
 arch_init.c |   12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

On 03/15/2013 09:50 AM, Peter Lieven wrote:
> avoid searching for dirty pages just increment the
> page offset. all pages are dirty anyway.
> 
> Signed-off-by: Peter Lieven <pl@kamp.de>
> ---
>  arch_init.c |   12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)

> 
> diff --git a/arch_init.c b/arch_init.c
> index a3dc20d..ca281ad 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -321,8 +321,14 @@ ram_addr_t migration_bitmap_find_and_reset_dirty(MemoryRegion *mr,
>      unsigned long nr = base + (start >> TARGET_PAGE_BITS);
>      unsigned long size = base + (int128_get64(mr->size) >> TARGET_PAGE_BITS);
>  
> -    unsigned long next = find_next_bit(migration_bitmap, size, nr);
> -
> +    unsigned long next;
> +    
> +    if (ram_bulk_stage) {
> +        next = nr + 1;
> +    } else {
> +        next = find_next_bit(migration_bitmap, size, nr);
> +    }

This part makes sense.

> +    
>      if (next < size) {
>          clear_bit(next, migration_bitmap);
>          migration_dirty_pages--;
> @@ -523,7 +529,7 @@ static void reset_ram_globals(void)
>  {
>      last_seen_block = NULL;
>      last_sent_block = NULL;
> -    last_offset = 0;
> +    last_offset = -1;

But what is this change doing?  I don't see it mentioned in the commit
message.  Does it belong to a different commit?

>      last_version = ram_list.version;
>      ram_bulk_stage = true;
>  }
>

Am 19.03.2013 um 18:40 schrieb Eric Blake <eblake@redhat.com>:

> On 03/15/2013 09:50 AM, Peter Lieven wrote:
>> avoid searching for dirty pages just increment the
>> page offset. all pages are dirty anyway.
>> 
>> Signed-off-by: Peter Lieven <pl@kamp.de>
>> ---
>> arch_init.c |   12 +++++++++---
>> 1 file changed, 9 insertions(+), 3 deletions(-)
> 
>> 
>> diff --git a/arch_init.c b/arch_init.c
>> index a3dc20d..ca281ad 100644
>> --- a/arch_init.c
>> +++ b/arch_init.c
>> @@ -321,8 +321,14 @@ ram_addr_t migration_bitmap_find_and_reset_dirty(MemoryRegion *mr,
>>     unsigned long nr = base + (start >> TARGET_PAGE_BITS);
>>     unsigned long size = base + (int128_get64(mr->size) >> TARGET_PAGE_BITS);
>> 
>> -    unsigned long next = find_next_bit(migration_bitmap, size, nr);
>> -
>> +    unsigned long next;
>> +    
>> +    if (ram_bulk_stage) {
>> +        next = nr + 1;
>> +    } else {
>> +        next = find_next_bit(migration_bitmap, size, nr);
>> +    }
> 
> This part makes sense.
> 
>> +    
>>     if (next < size) {
>>         clear_bit(next, migration_bitmap);
>>         migration_dirty_pages--;
>> @@ -523,7 +529,7 @@ static void reset_ram_globals(void)
>> {
>>     last_seen_block = NULL;
>>     last_sent_block = NULL;
>> -    last_offset = 0;
>> +    last_offset = -1;
> 
> But what is this change doing?  I don't see it mentioned in the commit
> message.  Does it belong to a different commit?

Sorry, i forgot to comment this. If in bulk stage next is nr + 1. I was aiming
to have next = 0 with last_offset = -1, but actually I think I'm wrong here,
I missed the (start >> TARGET_PAGE_BITS) at the beginning of migration_bitmap_find_and_reset_dirty.
If last_offset is initialized with 0. next is set to 1.

Peter

> 
>>     last_version = ram_list.version;
>>     ram_bulk_stage = true;
>> }
>> 
> 
> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>