From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:53417) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QlQdj-0003fp-27 for qemu-devel@nongnu.org; Mon, 25 Jul 2011 15:16:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QlQdh-00086K-Ce for qemu-devel@nongnu.org; Mon, 25 Jul 2011 15:16:31 -0400 Received: from mail-yw0-f45.google.com ([209.85.213.45]:51638) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QlQdh-000860-A7 for qemu-devel@nongnu.org; Mon, 25 Jul 2011 15:16:29 -0400 Received: by ywb3 with SMTP id 3so2911666ywb.4 for ; Mon, 25 Jul 2011 12:16:28 -0700 (PDT) Message-ID: <4E2DC10A.4040005@codemonkey.ws> Date: Mon, 25 Jul 2011 14:16:26 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <1311602584-23409-1-git-send-email-avi@redhat.com> <1311602584-23409-15-git-send-email-avi@redhat.com> In-Reply-To: <1311602584-23409-15-git-send-email-avi@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 14/23] memory: transaction API List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Avi Kivity Cc: qemu-devel@nongnu.org, kvm@vger.kernel.org On 07/25/2011 09:02 AM, Avi Kivity wrote: > Allow changes to the memory hierarchy to be accumulated and > made visible all at once. This reduces computational effort, > especially when an accelerator (e.g. kvm) is involved. > > Useful when a single register update causes multiple changes > to an address space. > > Signed-off-by: Avi Kivity What's the motivation for this? Was this just because it seemed neat to do or did you run into a performance issue you were trying to solve? > --- > memory.c | 20 ++++++++++++++++++++ > memory.h | 8 ++++++++ > 2 files changed, 28 insertions(+), 0 deletions(-) > > diff --git a/memory.c b/memory.c > index 009ad33..370e189 100644 > --- a/memory.c > +++ b/memory.c > @@ -18,6 +18,8 @@ > #include "kvm.h" > #include > > +unsigned memory_region_transaction_depth = 0; Shouldn't this be static? Regards, Anthony Liguori > + > typedef struct AddrRange AddrRange; > > struct AddrRange { > @@ -623,6 +625,10 @@ static void address_space_update_topology(AddressSpace *as) > > static void memory_region_update_topology(void) > { > + if (memory_region_transaction_depth) { > + return; > + } > + > if (address_space_memory.root) { > address_space_update_topology(&address_space_memory); > } > @@ -631,6 +637,20 @@ static void memory_region_update_topology(void) > } > } > > +void memory_region_transaction_begin(void) > +{ > + ++memory_region_transaction_depth; > +} > + > +void memory_region_transaction_commit(void) > +{ > + if (!memory_region_transaction_depth) { > + abort(); > + } > + --memory_region_transaction_depth; > + memory_region_update_topology(); > +} > + > void memory_region_init(MemoryRegion *mr, > const char *name, > uint64_t size) > diff --git a/memory.h b/memory.h > index e4c0ad1..cb3a9b6 100644 > --- a/memory.h > +++ b/memory.h > @@ -246,6 +246,14 @@ void memory_region_add_subregion_overlap(MemoryRegion *mr, > void memory_region_del_subregion(MemoryRegion *mr, > MemoryRegion *subregion); > > +/* Start a transaction; changes will be accumulated and made visible only > + * when the transaction ends. > + */ > +void memory_region_transaction_begin(void); > +/* Commit a transaction and make changes visible to the guest. > + */ > +void memory_region_transaction_commit(void); > + > #endif > > #endif