From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takuya Yoshikawa Subject: Re: [PATCH RFC 4/5] KVM: add new members to the memory slot for double buffering of bitmaps Date: Mon, 12 Apr 2010 11:15:51 +0900 Message-ID: <4BC28257.2080003@oss.ntt.co.jp> References: <20100409182732.857de4db.yoshikawa.takuya@oss.ntt.co.jp> <20100409183555.18a64dc7.yoshikawa.takuya@oss.ntt.co.jp> <4BC203C8.9010201@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: mtosatti@redhat.com, kvm@vger.kernel.org, fernando@oss.ntt.co.jp To: Avi Kivity Return-path: Received: from serv2.oss.ntt.co.jp ([222.151.198.100]:44834 "EHLO serv2.oss.ntt.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751184Ab0DLCMt (ORCPT ); Sun, 11 Apr 2010 22:12:49 -0400 In-Reply-To: <4BC203C8.9010201@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: (2010/04/12 2:15), Avi Kivity wrote: > On 04/09/2010 12:35 PM, Takuya Yoshikawa wrote: >> Currently, x86 vmalloc()s a dirty bitmap every time when we swich >> to the next dirty bitmap. To avoid this, we use the double buffering >> technique: we also move the bitmaps to userspace, so that extra >> bitmaps will not use the precious kernel resource. >> >> This idea is based on Avi's suggestion. >> > > Please fold this into the next patch. Introducing new data members > without their users is hard to follow. OK. > > >> #define KVM_MAX_VCPUS 64 >> #define KVM_MEMORY_SLOTS 32 >> /* memory slots that does not exposed to userspace */ >> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h >> index dd6bcf4..07092d6 100644 >> --- a/include/linux/kvm_host.h >> +++ b/include/linux/kvm_host.h >> @@ -110,7 +110,13 @@ struct kvm_memory_slot { >> unsigned long npages; >> unsigned long flags; >> unsigned long *rmap; >> +#ifndef __KVM_HAVE_USER_DIRTYBITMAP >> unsigned long *dirty_bitmap; >> +#else >> + unsigned long __user *dirty_bitmap; >> + unsigned long __user *dirty_bitmap_old; >> + bool is_dirty; >> +#endif > > And, if we make set_user_bit() generic, we don't need the ifdefs. OK, but we have one problem: ia64. I checked all architectures' dirty bitmap implementations and thought generalizing this work is not so hard except for ia64. It's already too different from other parts. #ifdef CONFIG_IA64 unsigned long *dirty_bitmap; #else ... #endif is acceptable? >