[PATCH RFC v2 1/6] KVM: introduce slot level dirty state management

Takuya Yoshikawa
2010-04-20T10:52:36Z

This patch introduces is_dirty member for each memory slot.
Using this member, we remove the dirty bitmap scan which is
done in the get_dirty_log function.

This also helps us when we move the dirty bitmaps to user space:
we don't have any good way to check the bitmaps in user space with
low cost, so scanning bitmaps for checking memory slot dirtiness
will not be acceptable.

When we mark the slot dirty:
- x86 and ppc: at the timing of mark_page_dirty()
- ia64: at the timing of kvm_ia64_sync_dirty_log()
ia64 uses a different place to store the dirty bitmap and synchronize
it right before the get_dirty_log(). So we use this timing to update
the dirtiness of a memory slot.

Signed-off-by: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>
Signed-off-by: Fernando Luis Vazquez Cao <fernando@oss.ntt.co.jp>
---
arch/ia64/kvm/kvm-ia64.c | 11 +++++++----
arch/powerpc/kvm/book3s.c | 9 ++++-----
arch/x86/kvm/x86.c | 9 +++------
include/linux/kvm_host.h | 4 ++--
virt/kvm/kvm_main.c | 16 +++++-----------
5 files changed, 21 insertions(+), 28 deletions(-)

diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c
index d7bac1f..d60dafe 100644
--- a/arch/ia64/kvm/kvm-ia64.c
+++ b/arch/ia64/kvm/kvm-ia64.c
@@ -1824,6 +1824,9 @@ static int kvm_ia64_sync_dirty_log(struct kvm *kvm,
base = memslot->base_gfn / BITS_PER_LONG;

for (i = 0; i < n/sizeof(long); ++i) {
+ if (dirty_bitmap[base + i])
+ memslot->is_dirty = true;
+
memslot->dirty_bitmap[i] = dirty_bitmap[base + i];
dirty_bitmap[base + i] = 0;
}
@@ -1838,7 +1841,6 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
int r;
unsigned long n;
struct kvm_memory_slot *memslot;
- int is_dirty = 0;

mutex_lock(&kvm->slots_lock);
spin_lock(&kvm->arch.dirty_log_lock);
@@ -1847,16 +1849,17 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
if (r)
goto out;

- r = kvm_get_dirty_log(kvm, log, &is_dirty);
+ r = kvm_get_dirty_log(kvm, log);
if (r)
goto out;

+ memslot = &kvm->memslots->memslots[log->slot];
/* If nothing is dirty, don't bother messing with page tables. */
- if (is_dirty) {
+ if (memslot->is_dirty) {
kvm_flush_remote_tlbs(kvm);
- memslot = &kvm->memslots->memslots[log->slot];
n = kvm_dirty_bitmap_bytes(memslot);
memset(memslot->dirty_bitmap, 0, n);
+ memslot->is_dirty = false;
}
r = 0;
out:
diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index 41c23b6..53b45cf 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -1118,20 +1118,18 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
struct kvm_memory_slot *memslot;
struct kvm_vcpu *vcpu;
ulong ga, ga_end;
- int is_dirty = 0;
int r;
unsigned long n;

mutex_lock(&kvm->slots_lock);

- r = kvm_get_dirty_log(kvm, log, &is_dirty);
+ r = kvm_get_dirty_log(kvm, log);
if (r)
goto out;

+ memslot = &kvm->memslots->memslots[log->slot];
/* If nothing is dirty, don't bother messing with page tables. */
- if (is_dirty) {
- memslot = &kvm->memslots->memslots[log->slot];
-
+ if (memslot->is_dirty) {
ga = memslot->base_gfn << PAGE_SHIFT;
ga_end = ga + (memslot->npages << PAGE_SHIFT);

@@ -1140,6 +1138,7 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,

n = kvm_dirty_bitmap_bytes(memslot);
memset(memslot->dirty_bitmap, 0, n);
+ memslot->is_dirty = false;
}

r = 0;
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 58a96e6..76b23ed 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2724,10 +2724,9 @@ static int kvm_vm_ioctl_reinject(struct kvm *kvm,
int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
struct kvm_dirty_log *log)
{
- int r, i;
+ int r;
struct kvm_memory_slot *memslot;
unsigned long n;
- unsigned long is_dirty = 0;
unsigned long *dirty_bitmap = NULL;

mutex_lock(&kvm->slots_lock);
@@ -2749,11 +2748,8 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
goto out;
memset(dirty_bitmap, 0, n);

- for (i = 0; !is_dirty && i < n/sizeof(long); i++)
- is_dirty = memslot->dirty_bitmap[i];
-
/* If nothing is dirty, don't bother messing with page tables. */
- if (is_dirty) {
+ if (memslot->is_dirty) {
struct kvm_memslots *slots, *old_slots;

spin_lock(&kvm->mmu_lock);
@@ -2766,6 +2762,7 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,

memcpy(slots, kvm->memslots, sizeof(struct kvm_memslots));
slots->memslots[log->slot].dirty_bitmap = dirty_bitmap;
+ slots->memslots[log->slot].is_dirty = false;

old_slots = kvm->memslots;
rcu_assign_pointer(kvm->memslots, slots);
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 5583063..4446622 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -117,6 +117,7 @@ struct kvm_memory_slot {
unsigned long flags;
unsigned long *rmap;
unsigned long *dirty_bitmap;
+ bool is_dirty;
struct {
unsigned long rmap_pde;
int write_count;
@@ -329,8 +330,7 @@ long kvm_arch_vcpu_ioctl(struct file *filp,

int kvm_dev_ioctl_check_extension(long ext);

-int kvm_get_dirty_log(struct kvm *kvm,
- struct kvm_dirty_log *log, int *is_dirty);
+int kvm_get_dirty_log(struct kvm *kvm, struct kvm_dirty_log *log);
int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
struct kvm_dirty_log *log);

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 3725605..cc78e07 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -764,13 +764,11 @@ int kvm_vm_ioctl_set_memory_region(struct kvm *kvm,
return kvm_set_memory_region(kvm, mem, user_alloc);
}

-int kvm_get_dirty_log(struct kvm *kvm,
- struct kvm_dirty_log *log, int *is_dirty)
+int kvm_get_dirty_log(struct kvm *kvm, struct kvm_dirty_log *log)
{
struct kvm_memory_slot *memslot;
- int r, i;
+ int r;
unsigned long n;
- unsigned long any = 0;

r = -EINVAL;
if (log->slot >= KVM_MEMORY_SLOTS)
@@ -783,16 +781,10 @@ int kvm_get_dirty_log(struct kvm *kvm,

n = kvm_dirty_bitmap_bytes(memslot);

- for (i = 0; !any && i < n/sizeof(long); ++i)
- any = memslot->dirty_bitmap[i];
-
r = -EFAULT;
if (copy_to_user(log->dirty_bitmap, memslot->dirty_bitmap, n))
goto out;

- if (any)
- *is_dirty = 1;
-
r = 0;
out:
return r;
@@ -1189,8 +1181,10 @@ void mark_page_dirty(struct kvm *kvm, gfn_t gfn)
unsigned long rel_gfn = gfn - memslot->base_gfn;

/* avoid RMW */
- if (!generic_test_le_bit(rel_gfn, memslot->dirty_bitmap))
+ if (!generic_test_le_bit(rel_gfn, memslot->dirty_bitmap)) {
generic___set_le_bit(rel_gfn, memslot->dirty_bitmap);
+ memslot->is_dirty = true;
+ }
}
}

--
1.6.3.3