From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takuya Yoshikawa Subject: Re: [PATCH RFC] KVM: use kmalloc() for small dirty bitmaps Date: Thu, 21 Oct 2010 10:51:43 +0900 Message-ID: <4CBF9CAF.4010305@oss.ntt.co.jp> References: <20101020183401.b8241b73.yoshikawa.takuya@oss.ntt.co.jp> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org, takuya.yoshikawa@gmail.com To: avi@redhat.com, mtosatti@redhat.com Return-path: Received: from serv2.oss.ntt.co.jp ([222.151.198.100]:47555 "EHLO serv2.oss.ntt.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756471Ab0JUBuH (ORCPT ); Wed, 20 Oct 2010 21:50:07 -0400 In-Reply-To: <20101020183401.b8241b73.yoshikawa.takuya@oss.ntt.co.jp> Sender: kvm-owner@vger.kernel.org List-ID: (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 > --- > 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 > #include > #include > +#include > +#include > #include > #include > #include > @@ -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);