On Tue, Feb 28, 2023 at 10:25 PM Longpeng(Mike) <longpe...@huawei.com> wrote:
>
> From: Longpeng <longpe...@huawei.com>
>
> When updating ioeventfds, we need to iterate all address spaces and
> iterate all flat ranges of each address space. There is so much
> redundant process that a FlatView would be iterated for so many times
> during one commit (memory_region_transaction_commit).
>
> We can mark a FlatView as UPDATED and then skip it in the next iteration
> and clear the UPDATED flag at the end of the commit. The overhead can
> be significantly reduced.
>
> For example, a VM with 16 vdpa net devices and each one has 65 vectors,
> can reduce the time spent on memory_region_transaction_commit by 95%.
>
> Signed-off-by: Longpeng <longpe...@huawei.com>
> ---
> include/exec/memory.h | 2 ++
> softmmu/memory.c | 28 +++++++++++++++++++++++++++-
> 2 files changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 2e602a2fad..974eabf765 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -1093,6 +1093,8 @@ struct FlatView {
> unsigned nr_allocated;
> struct AddressSpaceDispatch *dispatch;
> MemoryRegion *root;
> +#define FLATVIEW_FLAG_IOEVENTFD_UPDATED (1 << 0)
> + unsigned flags;
> };
>
> static inline FlatView *address_space_to_flatview(AddressSpace *as)
> diff --git a/softmmu/memory.c b/softmmu/memory.c
> index 9d64efca26..71ff996712 100644
> --- a/softmmu/memory.c
> +++ b/softmmu/memory.c
> @@ -815,6 +815,15 @@ FlatView *address_space_get_flatview(AddressSpace *as)
> return view;
> }
>
> +static void address_space_reset_view_flags(AddressSpace *as, unsigned mask)
> +{
> + FlatView *view = address_space_get_flatview(as);
> +
> + if (view->flags & mask) {
> + view->flags &= ~mask;
> + }
> +}
> +
> static void address_space_update_ioeventfds(AddressSpace *as)
> {
> FlatView *view;
> @@ -825,6 +834,12 @@ static void address_space_update_ioeventfds(AddressSpace
> *as)
> AddrRange tmp;
> unsigned i;
>
> + view = address_space_get_flatview(as);
> + if (view->flags & FLATVIEW_FLAG_IOEVENTFD_UPDATED) {
> + return;
> + }
> + view->flags |= FLATVIEW_FLAG_IOEVENTFD_UPDATED;
> +
Won't we lose the listener calls if multiple address spaces have the
same flatview?
Thanks
> /*
> * It is likely that the number of ioeventfds hasn't changed much, so use
> * the previous size as the starting value, with some headroom to avoid
> @@ -833,7 +848,6 @@ 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);
> FOR_EACH_FLAT_RANGE(fr, view) {
> for (i = 0; i < fr->mr->ioeventfd_nb; ++i) {
> tmp = addrrange_shift(fr->mr->ioeventfds[i].addr,
> @@ -1086,6 +1100,15 @@ void memory_region_transaction_begin(void)
> ++memory_region_transaction_depth;
> }
>
> +static inline void address_space_update_ioeventfds_finish(void)
> +{
> + AddressSpace *as;
> +
> + QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
> + address_space_reset_view_flags(as, FLATVIEW_FLAG_IOEVENTFD_UPDATED);
> + }
> +}
> +
> void memory_region_transaction_commit(void)
> {
> AddressSpace *as;
> @@ -1106,12 +1129,14 @@ void memory_region_transaction_commit(void)
> }
> memory_region_update_pending = false;
> ioeventfd_update_pending = false;
> + address_space_update_ioeventfds_finish();
> MEMORY_LISTENER_CALL_GLOBAL(commit, Forward);
> } else if (ioeventfd_update_pending) {
> QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
> address_space_update_ioeventfds(as);
> }
> ioeventfd_update_pending = false;
> + address_space_update_ioeventfds_finish();
> }
> }
> }
> @@ -3076,6 +3101,7 @@ void address_space_init(AddressSpace *as, MemoryRegion
> *root, const char *name)
> as->name = g_strdup(name ? name : "anonymous");
> address_space_update_topology(as);
> address_space_update_ioeventfds(as);
> + address_space_reset_view_flags(as, FLATVIEW_FLAG_IOEVENTFD_UPDATED);
> }
>
> static void do_address_space_destroy(AddressSpace *as)
> --
> 2.23.0
>
