On 08/04/2015 08:34, Xiao Guangrong wrote:
> We noticed that KVM keeps tracking dirty for the memslots when
> live migration failed which causes bad performance due to huge
> page mapping disallowed for this kind of memslot
>
> It is caused by slot flags does not properly sync-ed between Qemu
> and KVM. Current code doing slot update depends on slot->flags
> which hopes to omit unnecessary ioctl. However, slot->flags only
> reflects the stauts of corresponding memory region, vmsave and
> live migration do dirty tracking which overset
> KVM_MEM_LOG_DIRTY_PAGES for the slot. That causes the slot status
> recorded in the flags does not exactly match the stauts in kernel.
>
> We fixed it by introducing slot->is_dirty_logging which indicates
> the dirty status in kernel so that it helps us to sync the status
> between userspace and kernel
>
> Wanpeng Li <wanpeng.li@xxxxxxxxxxxxxxx>
> Signed-off-by: Xiao Guangrong <guangrong.xiao@xxxxxxxxxxxxxxx>
Hi Xiao,
the patch looks good.
However, I am planning to remove s->migration_log completely from QEMU
2.4 and have slot->flags also track the migration state. This has the
side effect of fixing this bug. I'll Cc you on the patches when I post
them (next week probably).
Thanks!
Paolo
> ---
> kvm-all.c | 19 ++++++++++++++++++-
> 1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/kvm-all.c b/kvm-all.c
> index dd44f8c..69fa233 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -60,6 +60,15 @@
>
> #define KVM_MSI_HASHTAB_SIZE 256
>
> +/*
> + * @flags only reflects the stauts of corresponding memory region,
> however,
> + * vmsave and live migration do dirty tracking which overset
> + * KVM_MEM_LOG_DIRTY_PAGES for the slot. That causes the slot status
> recorded
> + * in @flags does not exactly match the stauts in kernel.
> + *
> + * @is_dirty_logging indicating the dirty status in kernel helps us to
> sync
> + * the status between userspace and kernel.
> + */
> typedef struct KVMSlot
> {
> hwaddr start_addr;
> @@ -67,6 +76,7 @@ typedef struct KVMSlot
> void *ram;
> int slot;
> int flags;
> + bool is_dirty_logging;
> } KVMSlot;
>
> typedef struct kvm_dirty_log KVMDirtyLog;
> @@ -245,6 +255,7 @@ static int kvm_set_user_memory_region(KVMState *s,
> KVMSlot *slot)
> kvm_vm_ioctl(s, KVM_SET_USER_MEMORY_REGION, &mem);
> }
> mem.memory_size = slot->memory_size;
> + slot->is_dirty_logging = !!(mem.flags & KVM_MEM_LOG_DIRTY_PAGES);
> return kvm_vm_ioctl(s, KVM_SET_USER_MEMORY_REGION, &mem);
> }
>
> @@ -312,6 +323,7 @@ static int kvm_slot_dirty_pages_log_change(KVMSlot
> *mem, bool log_dirty)
> int old_flags;
>
> old_flags = mem->flags;
> + old_flags |= mem->is_dirty_logging ? KVM_MEM_LOG_DIRTY_PAGES : 0;
>
> flags = (mem->flags & ~mask) | kvm_mem_flags(s, log_dirty, false);
> mem->flags = flags;
> @@ -376,12 +388,17 @@ static int kvm_set_migration_log(bool enable)
> s->migration_log = enable;
>
> for (i = 0; i < s->nr_slots; i++) {
> + int dirty_enable;
> +
> mem = &s->slots[i];
>
> if (!mem->memory_size) {
> continue;
> }
> - if (!!(mem->flags & KVM_MEM_LOG_DIRTY_PAGES) == enable) {
> +
> + /* Keep the dirty bit if it is tracked by the memory region. */
> + dirty_enable = enable | (mem->flags & KVM_MEM_LOG_DIRTY_PAGES);
> + if (mem->is_dirty_logging == dirty_enable) {
> continue;
> }
> err = kvm_set_user_memory_region(s, mem);
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
