[Qemu-devel] Re: [PATCH 10/10] Maintaing number of dirty pages

[Juan Quintela]

Anthony Liguori <address@hidden> wrote:
> On 11/23/2010 05:03 PM, Juan Quintela wrote:
>> From: Juan Quintela<address@hidden>
>>
>> Calculate the number of dirty pages takes a lot on hosts with lots
>> of memory.  Just maintain how many pages are dirty.  Only sync bitmaps
>> if number is small enough.
>>    
>
> There needs to be numbers that justify this as part of the commit message.

They are on patch 0/6.

Additionally, with 64GB of RAM, this bitmap is HUGE, having to walk over
it in each ram_save_live() call is too onerous.

> This only works for KVM because it happens to use set_dirty() whenever
> it dirties memory.  I don't think will work with TCG.

The TCG is already broken with respect to migration.  Nothing else
sets that bitmap nowadays.

> I also think that the fundamental problem is the kernel dirty bitmap.
> Perhaps it should return a multiple level table instead of a simple
> linear bitmap.

KVM interface is suboptimal, no doubt here.  But the bigger problem is
qemu implementation.  Having a byte arry where it only uses 2 bits is
wasteful at least.  And having to transform the array forth<->back with
kvm is worse still.

Longer term my plan is:
- make kvm return the number of dirty pages somehow (i.e. no need to
  calculate them)
- split bitmaps, migration bitmap only needs to be handled during
  migration, and CODE bitmap only for TCG.
- interface with kvm would better be something like array of pages
  or array (pages, count).  But I need to measure if there is any
  differences/improvements with one/another.

We were discussing yesterday what made ram_live_block() staying on top
of the profiles, it was this.  This makes ram_save_live() to dissapear
from profiles.

Later, Juan.

> Regards,
>
> Anthony Liguori
>
>> Signed-off-by: Juan Quintela<address@hidden>
>> Signed-off-by: Juan Quintela<address@hidden>
>> ---
>>   arch_init.c |   15 +--------------
>>   cpu-all.h   |    7 +++++++
>>   exec.c      |    1 +
>>   3 files changed, 9 insertions(+), 14 deletions(-)
>>
>> diff --git a/arch_init.c b/arch_init.c
>> index b463798..c2bc144 100644
>> --- a/arch_init.c
>> +++ b/arch_init.c
>> @@ -176,20 +176,7 @@ static uint64_t bytes_transferred;
>>
>>   static uint64_t ram_save_remaining(void)
>>   {
>> -    RAMBlock *block;
>> -    uint64_t count = 0;
>> -
>> -    QLIST_FOREACH(block,&ram_list.blocks, next) {
>> -        ram_addr_t addr;
>> -        for (addr = block->offset; addr<  block->offset + block->length;
>> -             addr += TARGET_PAGE_SIZE) {
>> -            if (cpu_physical_memory_get_dirty(addr, MIGRATION_DIRTY_FLAG)) {
>> -                count++;
>> -            }
>> -        }
>> -    }
>> -
>> -    return count;
>> +    return ram_list.dirty_pages;
>>   }
>>
>>   uint64_t ram_bytes_remaining(void)
>> diff --git a/cpu-all.h b/cpu-all.h
>> index 30ae17d..5dc27c6 100644
>> --- a/cpu-all.h
>> +++ b/cpu-all.h
>> @@ -874,6 +874,7 @@ typedef struct RAMBlock {
>>
>>   typedef struct RAMList {
>>       uint8_t *phys_dirty;
>> +    uint64_t dirty_pages;
>>       QLIST_HEAD(ram, RAMBlock) blocks;
>>   } RAMList;
>>   extern RAMList ram_list;
>> @@ -922,6 +923,9 @@ static inline int 
>> cpu_physical_memory_get_dirty(ram_addr_t addr,
>>
>>   static inline void cpu_physical_memory_set_dirty(ram_addr_t addr)
>>   {
>> +    if (!cpu_physical_memory_get_dirty(addr, MIGRATION_DIRTY_FLAG))
>> +        ram_list.dirty_pages++;
>> +
>>       ram_list.phys_dirty[addr>>  TARGET_PAGE_BITS] = 0xff;
>>   }
>>
>> @@ -942,6 +946,9 @@ static inline void 
>> cpu_physical_memory_mask_dirty_range(ram_addr_t start,
>>       mask = ~dirty_flags;
>>       p = ram_list.phys_dirty + (start>>  TARGET_PAGE_BITS);
>>       for (i = 0; i<  len; i++) {
>> +        if (cpu_physical_memory_get_dirty(start + i * TARGET_PAGE_SIZE,
>> +                                          MIGRATION_DIRTY_FLAG&  
>> dirty_flags))
>> +            ram_list.dirty_pages--;
>>           p[i]&= mask;
>>       }
>>   }
>> diff --git a/exec.c b/exec.c
>> index f5b2386..ca2506e 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -2866,6 +2866,7 @@ ram_addr_t qemu_ram_alloc_from_ptr(DeviceState *dev, 
>> const char *name,
>>                                          last_ram_offset()>>  
>> TARGET_PAGE_BITS);
>>       memset(ram_list.phys_dirty + (new_block->offset>>  TARGET_PAGE_BITS),
>>              0xff, size>>  TARGET_PAGE_BITS);
>> +    ram_list.dirty_pages += size>>  TARGET_PAGE_BITS;
>>
>>       if (kvm_enabled())
>>           kvm_setup_guest_memory(new_block->host, size);
>>