Re: [Qemu-devel] [PATCH 06/15] memory: protect current_map by RCU

2015-01-25 Thread Fam Zheng 
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 pbonz...@redhat.com
 ---
  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);
 +}
 +

[Qemu-devel] [PATCH 06/15] memory: protect current_map by RCU

2015-01-22 Thread Paolo Bonzini 
Replace the flat_view_mutex by RCU, avoiding futex contention for
dataplane on large systems and many iothreads.

Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 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