lkml.org 
[lkml]   [2012]   [Mar]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[PATCH 3/4 changelog-v2] KVM: Switch to srcu-less get_dirty_log()
    We have seen some problems of the current implementation of
    get_dirty_log() which uses synchronize_srcu_expedited() for updating
    dirty bitmaps; e.g. it is noticeable that this sometimes gives us ms
    order of latency when we use VGA displays.

    Furthermore the recent discussion on the following thread
    "srcu: Implement call_srcu()"
    http://lkml.org/lkml/2012/1/31/211
    also motivated us to implement get_dirty_log() without SRCU.

    This patch achieves this goal without sacrificing the performance of
    both VGA and live migration: in practice the new code is much faster
    than the old one unless we have too many dirty pages.

    Implementation:

    The key part of the implementation is the use of xchg() operation for
    clearing dirty bits atomically. Since this allows us to update only
    BITS_PER_LONG pages at once, we need to iterate over the dirty bitmap
    until every dirty bit is cleared again for the next call.

    Although some people may worry about the problem of using the atomic
    memory instruction many times to the concurrently accessible bitmap,
    it is usually accessed with mmu_lock held and we rarely see concurrent
    accesses: so what we need to care about is the pure xchg() overheads.

    Another point to note is that we do not use for_each_set_bit() to check
    which ones in each BITS_PER_LONG pages are actually dirty. Instead we
    simply use __ffs() in a loop. This is much faster than repeatedly call
    find_next_bit().

    Performance:

    The dirty-log-perf unit test showed nice improvements, some times faster
    than before, except for some extreme cases; for such cases the speed of
    getting dirty page information is much faster than we process it in the
    userspace.

    For real workloads, both VGA and live migration, we have observed pure
    improvements: when the guest was reading a file during live migration,
    we originally saw a few ms of latency, but with the new method the
    latency was less than 200us.

    Signed-off-by: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>
    ---
    arch/x86/kvm/x86.c | 116 +++++++++++++++++++--------------------------------
    1 files changed, 43 insertions(+), 73 deletions(-)

    diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
    index 3bc1922..0748bab 100644
    --- a/arch/x86/kvm/x86.c
    +++ b/arch/x86/kvm/x86.c
    @@ -3041,55 +3041,32 @@ static int kvm_vm_ioctl_reinject(struct kvm *kvm,
    }

    /**
    - * write_protect_slot - write protect a slot for dirty logging
    - * @kvm: the kvm instance
    - * @memslot: the slot we protect
    - * @dirty_bitmap: the bitmap indicating which pages are dirty
    - * @nr_dirty_pages: the number of dirty pages
    + * kvm_vm_ioctl_get_dirty_log - get and clear the log of dirty pages in a slot
    + * @kvm: kvm instance
    + * @log: slot id and address to which we copy the log
    *
    - * We have two ways to find all sptes to protect:
    - * 1. Use kvm_mmu_slot_remove_write_access() which walks all shadow pages and
    - * checks ones that have a spte mapping a page in the slot.
    - * 2. Use kvm_mmu_rmap_write_protect() for each gfn found in the bitmap.
    + * We need to keep it in mind that VCPU threads can write to the bitmap
    + * concurrently. So, to avoid losing data, we keep the following order for
    + * each bit:
    *
    - * Generally speaking, if there are not so many dirty pages compared to the
    - * number of shadow pages, we should use the latter.
    + * 1. Take a snapshot of the bit and clear it if needed.
    + * 2. Write protect the corresponding page.
    + * 3. Flush TLB's if needed.
    + * 4. Copy the snapshot to the userspace.
    *
    - * Note that letting others write into a page marked dirty in the old bitmap
    - * by using the remaining tlb entry is not a problem. That page will become
    - * write protected again when we flush the tlb and then be reported dirty to
    - * the user space by copying the old bitmap.
    + * Between 2 and 3, the guest may write to the page using the remaining TLB
    + * entry. This is not a problem because the page will be reported dirty at
    + * step 4 using the snapshot taken before and step 3 ensures that successive
    + * writes will be logged for the next call.
    */
    -static void write_protect_slot(struct kvm *kvm,
    - struct kvm_memory_slot *memslot,
    - unsigned long *dirty_bitmap,
    - unsigned long nr_dirty_pages)
    -{
    - spin_lock(&kvm->mmu_lock);
    -
    - /* Not many dirty pages compared to # of shadow pages. */
    - if (nr_dirty_pages < kvm->arch.n_used_mmu_pages) {
    - gfn_t offset;
    -
    - for_each_set_bit(offset, dirty_bitmap, memslot->npages)
    - kvm_mmu_write_protect_pt_masked(kvm, memslot, offset, 1);
    -
    - kvm_flush_remote_tlbs(kvm);
    - } else
    - kvm_mmu_slot_remove_write_access(kvm, memslot->id);
    -
    - spin_unlock(&kvm->mmu_lock);
    -}
    -
    -/*
    - * Get (and clear) the dirty memory log for a memory slot.
    - */
    -int kvm_vm_ioctl_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)
    {
    int r;
    struct kvm_memory_slot *memslot;
    - unsigned long n, nr_dirty_pages;
    + unsigned long n, i;
    + unsigned long *dirty_bitmap;
    + unsigned long *dirty_bitmap_buffer;
    + bool is_dirty = false;

    mutex_lock(&kvm->slots_lock);

    @@ -3098,49 +3075,42 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
    goto out;

    memslot = id_to_memslot(kvm->memslots, log->slot);
    +
    + dirty_bitmap = memslot->dirty_bitmap;
    r = -ENOENT;
    - if (!memslot->dirty_bitmap)
    + if (!dirty_bitmap)
    goto out;

    n = kvm_dirty_bitmap_bytes(memslot);
    - nr_dirty_pages = memslot->nr_dirty_pages;

    - /* If nothing is dirty, don't bother messing with page tables. */
    - if (nr_dirty_pages) {
    - struct kvm_memslots *slots, *old_slots;
    - unsigned long *dirty_bitmap, *dirty_bitmap_head;
    + dirty_bitmap_buffer = dirty_bitmap + n / sizeof(long);
    + memset(dirty_bitmap_buffer, 0, n);

    - dirty_bitmap = memslot->dirty_bitmap;
    - dirty_bitmap_head = memslot->dirty_bitmap_head;
    - if (dirty_bitmap == dirty_bitmap_head)
    - dirty_bitmap_head += n / sizeof(long);
    - memset(dirty_bitmap_head, 0, n);
    + spin_lock(&kvm->mmu_lock);

    - r = -ENOMEM;
    - slots = kmemdup(kvm->memslots, sizeof(*kvm->memslots), GFP_KERNEL);
    - if (!slots)
    - goto out;
    + for (i = 0; i < n / sizeof(long); i++) {
    + unsigned long mask;
    + gfn_t offset;

    - memslot = id_to_memslot(slots, log->slot);
    - memslot->nr_dirty_pages = 0;
    - memslot->dirty_bitmap = dirty_bitmap_head;
    - update_memslots(slots, NULL);
    + if (!dirty_bitmap[i])
    + continue;

    - old_slots = kvm->memslots;
    - rcu_assign_pointer(kvm->memslots, slots);
    - synchronize_srcu_expedited(&kvm->srcu);
    - kfree(old_slots);
    + is_dirty = true;

    - write_protect_slot(kvm, memslot, dirty_bitmap, nr_dirty_pages);
    + mask = xchg(&dirty_bitmap[i], 0);
    + dirty_bitmap_buffer[i] = mask;

    - r = -EFAULT;
    - if (copy_to_user(log->dirty_bitmap, dirty_bitmap, n))
    - goto out;
    - } else {
    - r = -EFAULT;
    - if (clear_user(log->dirty_bitmap, n))
    - goto out;
    + offset = i * BITS_PER_LONG;
    + kvm_mmu_write_protect_pt_masked(kvm, memslot, offset, mask);
    }
    + if (is_dirty)
    + kvm_flush_remote_tlbs(kvm);
    +
    + spin_unlock(&kvm->mmu_lock);
    +
    + r = -EFAULT;
    + if (copy_to_user(log->dirty_bitmap, dirty_bitmap_buffer, n))
    + goto out;

    r = 0;
    out:
    --
    1.7.5.4
    --
    To unsubscribe from this list: send the line "unsubscribe kvm" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html


    \
     
     \ /
      Last update: 2012-03-03 06:25    [W:0.037 / U:0.684 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site