From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45853) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YFcyS-00032i-Dy for qemu-devel@nongnu.org; Mon, 26 Jan 2015 01:16:37 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YFcyP-0003qd-4U for qemu-devel@nongnu.org; Mon, 26 Jan 2015 01:16:36 -0500 Received: from mx1.redhat.com ([209.132.183.28]:46103) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YFcyO-0003qT-T6 for qemu-devel@nongnu.org; Mon, 26 Jan 2015 01:16:33 -0500 Date: Mon, 26 Jan 2015 14:16:25 +0800 From: Fam Zheng Message-ID: <20150126061625.GF2354@ad.nay.redhat.com> References: <1421938053-10318-1-git-send-email-pbonzini@redhat.com> <1421938053-10318-7-git-send-email-pbonzini@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1421938053-10318-7-git-send-email-pbonzini@redhat.com> Subject: Re: [Qemu-devel] [PATCH 06/15] memory: protect current_map by RCU List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: borntraeger@de.ibm.com, qemu-devel@nongnu.org, stefanha@redhat.com On Thu, 01/22 15:47, Paolo Bonzini wrote: > Replace the flat_view_mutex by RCU, avoiding futex contention for > dataplane on large systems and many iothreads. > > Signed-off-by: Paolo Bonzini > --- > include/exec/memory.h | 5 +++++ > memory.c | 54 ++++++++++++++++++++++----------------------------- > 2 files changed, 28 insertions(+), 31 deletions(-) > > diff --git a/include/exec/memory.h b/include/exec/memory.h > index 0cd96b1..06ffa1d 100644 > --- a/include/exec/memory.h > +++ b/include/exec/memory.h > @@ -33,6 +33,7 @@ > #include "qemu/notify.h" > #include "qapi/error.h" > #include "qom/object.h" > +#include "qemu/rcu.h" > > #define MAX_PHYS_ADDR_SPACE_BITS 62 > #define MAX_PHYS_ADDR (((hwaddr)1 << MAX_PHYS_ADDR_SPACE_BITS) - 1) > @@ -207,9 +208,13 @@ struct MemoryListener { > */ > struct AddressSpace { > /* All fields are private. */ > + struct rcu_head rcu; > char *name; > MemoryRegion *root; > + > + /* Accessed via RCU. */ > struct FlatView *current_map; > + > int ioeventfd_nb; > struct MemoryRegionIoeventfd *ioeventfds; > struct AddressSpaceDispatch *dispatch; > diff --git a/memory.c b/memory.c > index 8c3d8c0..a844ced 100644 > --- a/memory.c > +++ b/memory.c > @@ -33,26 +33,12 @@ static bool memory_region_update_pending; > static bool ioeventfd_update_pending; > static bool global_dirty_log = false; > > -/* flat_view_mutex is taken around reading as->current_map; the critical > - * section is extremely short, so I'm using a single mutex for every AS. > - * We could also RCU for the read-side. > - * > - * The BQL is taken around transaction commits, hence both locks are taken > - * while writing to as->current_map (with the BQL taken outside). > - */ > -static QemuMutex flat_view_mutex; > - > static QTAILQ_HEAD(memory_listeners, MemoryListener) memory_listeners > = QTAILQ_HEAD_INITIALIZER(memory_listeners); > > static QTAILQ_HEAD(, AddressSpace) address_spaces > = QTAILQ_HEAD_INITIALIZER(address_spaces); > > -static void memory_init(void) > -{ > - qemu_mutex_init(&flat_view_mutex); > -} > - > typedef struct AddrRange AddrRange; > > /* > @@ -242,6 +228,7 @@ struct FlatRange { > * order. > */ > struct FlatView { > + struct rcu_head rcu; > unsigned ref; > FlatRange *ranges; > unsigned nr; > @@ -654,10 +641,10 @@ static FlatView *address_space_get_flatview(AddressSpace *as) > { > FlatView *view; > > - qemu_mutex_lock(&flat_view_mutex); > - view = as->current_map; > + rcu_read_lock(); > + view = atomic_rcu_read(&as->current_map); > flatview_ref(view); > - qemu_mutex_unlock(&flat_view_mutex); > + rcu_read_unlock(); > return view; > } > > @@ -766,10 +753,9 @@ static void address_space_update_topology(AddressSpace *as) > address_space_update_topology_pass(as, old_view, new_view, false); > address_space_update_topology_pass(as, old_view, new_view, true); > > - qemu_mutex_lock(&flat_view_mutex); > - flatview_unref(as->current_map); > - as->current_map = new_view; > - qemu_mutex_unlock(&flat_view_mutex); > + /* Writes are protected by the BQL. */ > + atomic_rcu_set(&as->current_map, new_view); > + call_rcu(old_view, flatview_unref, rcu); > > /* Note that all the old MemoryRegions are still alive up to this > * point. This relieves most MemoryListeners from the need to > @@ -1957,10 +1943,6 @@ void memory_listener_unregister(MemoryListener *listener) > > void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name) > { > - if (QTAILQ_EMPTY(&address_spaces)) { > - memory_init(); > - } > - > memory_region_transaction_begin(); > as->root = root; > as->current_map = g_new(FlatView, 1); > @@ -1974,15 +1956,10 @@ void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name) > memory_region_transaction_commit(); > } > > -void address_space_destroy(AddressSpace *as) > +static void do_address_space_destroy(AddressSpace *as) > { > MemoryListener *listener; > > - /* Flush out anything from MemoryListeners listening in on this */ > - memory_region_transaction_begin(); > - as->root = NULL; > - memory_region_transaction_commit(); > - QTAILQ_REMOVE(&address_spaces, as, address_spaces_link); > address_space_destroy_dispatch(as); > > QTAILQ_FOREACH(listener, &memory_listeners, link) { > @@ -1994,6 +1971,21 @@ void address_space_destroy(AddressSpace *as) > g_free(as->ioeventfds); > } > > +void address_space_destroy(AddressSpace *as) > +{ > + /* Flush out anything from MemoryListeners listening in on this */ > + memory_region_transaction_begin(); > + as->root = NULL; > + memory_region_transaction_commit(); > + QTAILQ_REMOVE(&address_spaces, as, address_spaces_link); > + > + /* At this point, as->dispatch and as->current_map are dummy > + * entries that the guest should never use. Wait for the old > + * values to expire before freeing the data. > + */ > + call_rcu(as, do_address_space_destroy, rcu); > +} > + > bool io_mem_read(MemoryRegion *mr, hwaddr addr, uint64_t *pval, unsigned size) > { > return memory_region_dispatch_read(mr, addr, pval, size); > -- > 1.8.3.1 > > Reviewed-by: Fam Zheng