Re: [PATCH RFC] KVM: use kmalloc() for small dirty bitmaps

[Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>]

(2010/10/20 18:34), Takuya Yoshikawa wrote:
> ** NOT TESTED WELL YET **
>
> Currently, we are using vmalloc() for all dirty bitmaps even though
> they are small enough, say 256 bytes or less.
>
> So we use kmalloc() if dirty bitmap size is less than or equal to
> PAGE_SIZE.


Ah, I forgot about the plan to do double buffering.

Making the size of bitmap twice may reduce the benefit of this patch.

Sorry, just ignore this one!
   -- I was using this patch to see frame buffer corruption rate
      would change.

   Takuya


>
> Signed-off-by: Takuya Yoshikawa<yoshikawa.takuya@oss.ntt.co.jp>
> ---
>   arch/x86/kvm/x86.c       |    4 ++--
>   include/linux/kvm_host.h |   24 ++++++++++++++++++++++++
>   virt/kvm/kvm_main.c      |    6 +++---
>   3 files changed, 29 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index f3f86b2..e137c34 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -3172,7 +3172,7 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
>   		spin_unlock(&kvm->mmu_lock);
>
>   		r = -ENOMEM;
> -		dirty_bitmap = vmalloc(n);
> +		dirty_bitmap = kvm_alloc_dirty_bitmap(n);
>   		if (!dirty_bitmap)
>   			goto out;
>   		memset(dirty_bitmap, 0, n);
> @@ -3197,7 +3197,7 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
>   			vfree(dirty_bitmap);
>   			goto out;
>   		}
> -		vfree(dirty_bitmap);
> +		kvm_free_dirty_bitmap(dirty_bitmap, n);
>   	} else {
>   		r = -EFAULT;
>   		if (clear_user(log->dirty_bitmap, n))
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 0b89d00..fe19118 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -14,6 +14,8 @@
>   #include<linux/signal.h>
>   #include<linux/sched.h>
>   #include<linux/mm.h>
> +#include<linux/vmalloc.h>
> +#include<linux/slab.h>
>   #include<linux/preempt.h>
>   #include<linux/msi.h>
>   #include<asm/signal.h>
> @@ -133,6 +135,28 @@ static inline unsigned long kvm_dirty_bitmap_bytes(struct kvm_memory_slot *memsl
>   	return ALIGN(memslot->npages, BITS_PER_LONG) / 8;
>   }
>
> +/*
> + * Dirty bitmap allocation function.
> + * Some memslots don't need so long bitmaps and we can eliminate vmalloc()
> + * for them.
> + */
> +static inline unsigned long *kvm_alloc_dirty_bitmap(unsigned long bytes)
> +{
> +	if (bytes<  PAGE_SIZE)
> +		return kmalloc(bytes, GFP_KERNEL);
> +	else
> +		return vmalloc(bytes);
> +}
> +
> +static inline void kvm_free_dirty_bitmap(unsigned long *dirty_bitmap,
> +					 unsigned long bytes)
> +{
> +	if (bytes<  PAGE_SIZE)
> +		kfree(dirty_bitmap);
> +	else
> +		vfree(dirty_bitmap);
> +}
> +
>   struct kvm_kernel_irq_routing_entry {
>   	u32 gsi;
>   	u32 type;
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 1aeeb7f..1a8ba85 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -454,8 +454,8 @@ static void kvm_free_physmem_slot(struct kvm_memory_slot *free,
>   		vfree(free->rmap);
>
>   	if (!dont || free->dirty_bitmap != dont->dirty_bitmap)
> -		vfree(free->dirty_bitmap);
> -
> +		kvm_free_dirty_bitmap(free->dirty_bitmap,
> +				      kvm_dirty_bitmap_bytes(free));
>
>   	for (i = 0; i<  KVM_NR_PAGE_SIZES - 1; ++i) {
>   		if (!dont || free->lpage_info[i] != dont->lpage_info[i]) {
> @@ -663,7 +663,7 @@ skip_lpage:
>   	if ((new.flags&  KVM_MEM_LOG_DIRTY_PAGES)&&  !new.dirty_bitmap) {
>   		unsigned long dirty_bytes = kvm_dirty_bitmap_bytes(&new);
>
> -		new.dirty_bitmap = vmalloc(dirty_bytes);
> +		new.dirty_bitmap = kvm_alloc_dirty_bitmap(dirty_bytes);
>   		if (!new.dirty_bitmap)
>   			goto out_free;
>   		memset(new.dirty_bitmap, 0, dirty_bytes);