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(-)
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.1Chuang 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
--
Leaves are falling all around, it's time I was on my way.
© 2016 - 2026 Red Hat, Inc.