diff mbox series

[v7,1/6] memory: Reference as->current_map directly in memory commit

Message ID 20230310022425.2992472-2-xuchuangxclwt@bytedance.com (mailing list archive)
State New, archived
Headers show
Series migration: reduce time of loading non-iterable vmstate | expand

Commit Message

Chuang Xu March 10, 2023, 2:24 a.m. UTC 
From: Peter Xu <peterx@redhat.com>

Calling RCU variance of address_space_get|to_flatview() during memory
commit (flatview updates, triggering memory listeners, or updating
ioeventfds, etc.) is not 100% accurate, because commit() requires BQL
rather than RCU read lock, so the context exclusively owns current_map and
can be directly referenced.

Neither does it need a refcount to current_map because it cannot be freed
from under the caller.

Add address_space_get_flatview_raw() for the case where the context holds
BQL rather than RCU read lock and use it across the core memory updates,
Drop the extra refcounts on FlatView*.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 softmmu/memory.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

Comments

David Edmondson March 14, 2023, 11:25 a.m. UTC | #1 
Chuang Xu <xuchuangxclwt@bytedance.com> writes:

> From: Peter Xu <peterx@redhat.com>
>
> Calling RCU variance of address_space_get|to_flatview() during memory

"variants" rather than "variance", perhaps?

> commit (flatview updates, triggering memory listeners, or updating
> ioeventfds, etc.) is not 100% accurate, because commit() requires BQL
> rather than RCU read lock, so the context exclusively owns current_map and
> can be directly referenced.
>
> Neither does it need a refcount to current_map because it cannot be freed
> from under the caller.
>
> Add address_space_get_flatview_raw() for the case where the context holds
> BQL rather than RCU read lock and use it across the core memory updates,
> Drop the extra refcounts on FlatView*.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  softmmu/memory.c | 28 ++++++++++++++++------------
>  1 file changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/softmmu/memory.c b/softmmu/memory.c
> index 4699ba55ec..a992a365d9 100644
> --- a/softmmu/memory.c
> +++ b/softmmu/memory.c
> @@ -61,6 +61,13 @@ struct AddrRange {
>      Int128 size;
>  };
>  
> +/* Called with BQL held */
> +static inline FlatView *address_space_to_flatview_raw(AddressSpace *as)
> +{
> +    assert(qemu_mutex_iothread_locked());
> +    return as->current_map;
> +}
> +
>  static AddrRange addrrange_make(Int128 start, Int128 size)
>  {
>      return (AddrRange) { start, size };
> @@ -155,7 +162,7 @@ enum ListenerDirection { Forward, Reverse };
>  #define MEMORY_LISTENER_UPDATE_REGION(fr, as, dir, callback, _args...)  \
>      do {                                                                \
>          MemoryRegionSection mrs = section_from_flat_range(fr,           \
> -                address_space_to_flatview(as));                         \
> +                address_space_to_flatview_raw(as));                     \
>          MEMORY_LISTENER_CALL(as, callback, dir, &mrs, ##_args);         \
>      } while(0)
>  
> @@ -753,6 +760,7 @@ static FlatView *generate_memory_topology(MemoryRegion *mr)
>  }
>  
>  static void address_space_add_del_ioeventfds(AddressSpace *as,
> +                                             FlatView *view,
>                                               MemoryRegionIoeventfd *fds_new,
>                                               unsigned fds_new_nb,
>                                               MemoryRegionIoeventfd *fds_old,
> @@ -774,7 +782,7 @@ static void address_space_add_del_ioeventfds(AddressSpace *as,
>                                                    &fds_new[inew]))) {
>              fd = &fds_old[iold];
>              section = (MemoryRegionSection) {
> -                .fv = address_space_to_flatview(as),
> +                .fv = view,
>                  .offset_within_address_space = int128_get64(fd->addr.start),
>                  .size = fd->addr.size,
>              };
> @@ -787,7 +795,7 @@ static void address_space_add_del_ioeventfds(AddressSpace *as,
>                                                           &fds_old[iold]))) {
>              fd = &fds_new[inew];
>              section = (MemoryRegionSection) {
> -                .fv = address_space_to_flatview(as),
> +                .fv = view,
>                  .offset_within_address_space = int128_get64(fd->addr.start),
>                  .size = fd->addr.size,
>              };
> @@ -833,7 +841,7 @@ static void address_space_update_ioeventfds(AddressSpace *as)
>      ioeventfd_max = QEMU_ALIGN_UP(as->ioeventfd_nb, 4);
>      ioeventfds = g_new(MemoryRegionIoeventfd, ioeventfd_max);
>  
> -    view = address_space_get_flatview(as);
> +    view = address_space_to_flatview_raw(as);
>      FOR_EACH_FLAT_RANGE(fr, view) {
>          for (i = 0; i < fr->mr->ioeventfd_nb; ++i) {
>              tmp = addrrange_shift(fr->mr->ioeventfds[i].addr,
> @@ -852,13 +860,12 @@ static void address_space_update_ioeventfds(AddressSpace *as)
>          }
>      }
>  
> -    address_space_add_del_ioeventfds(as, ioeventfds, ioeventfd_nb,
> +    address_space_add_del_ioeventfds(as, view, ioeventfds, ioeventfd_nb,
>                                       as->ioeventfds, as->ioeventfd_nb);
>  
>      g_free(as->ioeventfds);
>      as->ioeventfds = ioeventfds;
>      as->ioeventfd_nb = ioeventfd_nb;
> -    flatview_unref(view);
>  }
>  
>  /*
> @@ -1026,7 +1033,7 @@ static void flatviews_reset(void)
>  
>  static void address_space_set_flatview(AddressSpace *as)
>  {
> -    FlatView *old_view = address_space_to_flatview(as);
> +    FlatView *old_view = address_space_to_flatview_raw(as);
>      MemoryRegion *physmr = memory_region_get_flatview_root(as->root);
>      FlatView *new_view = g_hash_table_lookup(flat_views, physmr);
>  
> @@ -2979,8 +2986,7 @@ static void listener_add_address_space(MemoryListener *listener,
>              listener->log_global_start(listener);
>          }
>      }
> -
> -    view = address_space_get_flatview(as);
> +    view = address_space_to_flatview_raw(as);
>      FOR_EACH_FLAT_RANGE(fr, view) {
>          MemoryRegionSection section = section_from_flat_range(fr, view);
>  
> @@ -2994,7 +3000,6 @@ static void listener_add_address_space(MemoryListener *listener,
>      if (listener->commit) {
>          listener->commit(listener);
>      }
> -    flatview_unref(view);
>  }
>  
>  static void listener_del_address_space(MemoryListener *listener,
> @@ -3006,7 +3011,7 @@ static void listener_del_address_space(MemoryListener *listener,
>      if (listener->begin) {
>          listener->begin(listener);
>      }
> -    view = address_space_get_flatview(as);
> +    view = address_space_to_flatview_raw(as);
>      FOR_EACH_FLAT_RANGE(fr, view) {
>          MemoryRegionSection section = section_from_flat_range(fr, view);
>  
> @@ -3020,7 +3025,6 @@ static void listener_del_address_space(MemoryListener *listener,
>      if (listener->commit) {
>          listener->commit(listener);
>      }
> -    flatview_unref(view);
>  }
>  
>  void memory_listener_register(MemoryListener *listener, AddressSpace *as)
> -- 
> 2.20.1
diff mbox series

Patch

diff --git a/softmmu/memory.c b/softmmu/memory.c
index 4699ba55ec..a992a365d9 100644
--- a/softmmu/memory.c
+++ b/softmmu/memory.c
@@ -61,6 +61,13 @@  struct AddrRange {
     Int128 size;
 };
 
+/* Called with BQL held */
+static inline FlatView *address_space_to_flatview_raw(AddressSpace *as)
+{
+    assert(qemu_mutex_iothread_locked());
+    return as->current_map;
+}
+
 static AddrRange addrrange_make(Int128 start, Int128 size)
 {
     return (AddrRange) { start, size };
@@ -155,7 +162,7 @@  enum ListenerDirection { Forward, Reverse };
 #define MEMORY_LISTENER_UPDATE_REGION(fr, as, dir, callback, _args...)  \
     do {                                                                \
         MemoryRegionSection mrs = section_from_flat_range(fr,           \
-                address_space_to_flatview(as));                         \
+                address_space_to_flatview_raw(as));                     \
         MEMORY_LISTENER_CALL(as, callback, dir, &mrs, ##_args);         \
     } while(0)
 
@@ -753,6 +760,7 @@  static FlatView *generate_memory_topology(MemoryRegion *mr)
 }
 
 static void address_space_add_del_ioeventfds(AddressSpace *as,
+                                             FlatView *view,
                                              MemoryRegionIoeventfd *fds_new,
                                              unsigned fds_new_nb,
                                              MemoryRegionIoeventfd *fds_old,
@@ -774,7 +782,7 @@  static void address_space_add_del_ioeventfds(AddressSpace *as,
                                                   &fds_new[inew]))) {
             fd = &fds_old[iold];
             section = (MemoryRegionSection) {
-                .fv = address_space_to_flatview(as),
+                .fv = view,
                 .offset_within_address_space = int128_get64(fd->addr.start),
                 .size = fd->addr.size,
             };
@@ -787,7 +795,7 @@  static void address_space_add_del_ioeventfds(AddressSpace *as,
                                                          &fds_old[iold]))) {
             fd = &fds_new[inew];
             section = (MemoryRegionSection) {
-                .fv = address_space_to_flatview(as),
+                .fv = view,
                 .offset_within_address_space = int128_get64(fd->addr.start),
                 .size = fd->addr.size,
             };
@@ -833,7 +841,7 @@  static void address_space_update_ioeventfds(AddressSpace *as)
     ioeventfd_max = QEMU_ALIGN_UP(as->ioeventfd_nb, 4);
     ioeventfds = g_new(MemoryRegionIoeventfd, ioeventfd_max);
 
-    view = address_space_get_flatview(as);
+    view = address_space_to_flatview_raw(as);
     FOR_EACH_FLAT_RANGE(fr, view) {
         for (i = 0; i < fr->mr->ioeventfd_nb; ++i) {
             tmp = addrrange_shift(fr->mr->ioeventfds[i].addr,
@@ -852,13 +860,12 @@  static void address_space_update_ioeventfds(AddressSpace *as)
         }
     }
 
-    address_space_add_del_ioeventfds(as, ioeventfds, ioeventfd_nb,
+    address_space_add_del_ioeventfds(as, view, ioeventfds, ioeventfd_nb,
                                      as->ioeventfds, as->ioeventfd_nb);
 
     g_free(as->ioeventfds);
     as->ioeventfds = ioeventfds;
     as->ioeventfd_nb = ioeventfd_nb;
-    flatview_unref(view);
 }
 
 /*
@@ -1026,7 +1033,7 @@  static void flatviews_reset(void)
 
 static void address_space_set_flatview(AddressSpace *as)
 {
-    FlatView *old_view = address_space_to_flatview(as);
+    FlatView *old_view = address_space_to_flatview_raw(as);
     MemoryRegion *physmr = memory_region_get_flatview_root(as->root);
     FlatView *new_view = g_hash_table_lookup(flat_views, physmr);
 
@@ -2979,8 +2986,7 @@  static void listener_add_address_space(MemoryListener *listener,
             listener->log_global_start(listener);
         }
     }
-
-    view = address_space_get_flatview(as);
+    view = address_space_to_flatview_raw(as);
     FOR_EACH_FLAT_RANGE(fr, view) {
         MemoryRegionSection section = section_from_flat_range(fr, view);
 
@@ -2994,7 +3000,6 @@  static void listener_add_address_space(MemoryListener *listener,
     if (listener->commit) {
         listener->commit(listener);
     }
-    flatview_unref(view);
 }
 
 static void listener_del_address_space(MemoryListener *listener,
@@ -3006,7 +3011,7 @@  static void listener_del_address_space(MemoryListener *listener,
     if (listener->begin) {
         listener->begin(listener);
     }
-    view = address_space_get_flatview(as);
+    view = address_space_to_flatview_raw(as);
     FOR_EACH_FLAT_RANGE(fr, view) {
         MemoryRegionSection section = section_from_flat_range(fr, view);
 
@@ -3020,7 +3025,6 @@  static void listener_del_address_space(MemoryListener *listener,
     if (listener->commit) {
         listener->commit(listener);
     }
-    flatview_unref(view);
 }
 
 void memory_listener_register(MemoryListener *listener, AddressSpace *as)