From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takuya Yoshikawa Subject: Re: [PATCH] KVM x86: remove memset, use vzalloc and don't assign the same value to a variable twice Date: Mon, 01 Nov 2010 09:49:54 +0900 Message-ID: <4CCE0EB2.9070302@oss.ntt.co.jp> References: Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org, Avi Kivity , Avi Kivity , Marcelo Tosatti , Yaniv Kamay , Amit Shah , Ben-Ami Yassour , linux-kernel@vger.kernel.org To: Jesper Juhl Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org List-Id: kvm.vger.kernel.org (2010/10/31 3:28), Jesper Juhl wrote: > Hi, > > We can improve kvm_vm_ioctl_get_dirty_log() slightly by using vzalloc() > rather than first allocating and then manually zero the memory with > memset(). Also, while I was looking at this I noticed that we assign I personally prefer this new vzalloc() to vmalloc() + memset(). Just from my interest, is there real performance difference not just the cleanup effect? If so, we'd better do this for other places too. > -ENOMEM to the 'r' variable twice even though none of the code inbetween > the two assignments can change 'r', so I removed the second assignment. > > Patch has been compile tested only. > > Please consider merging and please CC me on all replies as I'm not > subscribed to the kvm mailing list. > > > Signed-off-by: Jesper Juhl > --- > x86.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 2288ad8..29f9c0a 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -3174,12 +3174,10 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm, > spin_unlock(&kvm->mmu_lock); This patch is not based on kvm.git, I guess. > > r = -ENOMEM; > - dirty_bitmap = vmalloc(n); > + dirty_bitmap = vzalloc(n); > if (!dirty_bitmap) > goto out; > - memset(dirty_bitmap, 0, n); > > - r = -ENOMEM; This one is here because this belongs to a different code block from the previous one. This keeps it easy to insert another codes in between these two blocks. The optimization will be done at compile time. IIRC, I did like this based on Avi's advise. Takuya > slots = kzalloc(sizeof(struct kvm_memslots), GFP_KERNEL); > if (!slots) { > vfree(dirty_bitmap); > >