Anthony Liguori <anth...@codemonkey.ws> wrote:
> On 11/23/2010 05:03 PM, Juan Quintela wrote:
>> From: Juan Quintela<quint...@trasno.org>
>>
>> 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<quint...@trasno.org>
>> Signed-off-by: Juan Quintela<quint...@redhat.com>
>> ---
>> 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);
>>
