[PATCH] memory: avoid updating ioeventfds for some address_space

[PATCH] memory: avoid updating ioeventfds for some address_space

[hongmianquan]

When updating ioeventfds, we need to iterate all address spaces,
but some address spaces do not register eventfd_add|del call when
memory_listener_register() and they do nothing when updating ioeventfds.
So we can skip these AS in address_space_update_ioeventfds().

The overhead of memory_region_transaction_commit() can be significantly
reduced. For example, a VM with 8 vhost net devices and each one has
64 vectors, can reduce the time spent on memory_region_transaction_commit by 20%.

Signed-off-by: hongmianquan <hongmianquan@bytedance.com>
---
 include/exec/memory.h |  1 +
 softmmu/memory.c      | 12 ++++++++++++
 2 files changed, 13 insertions(+)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 7f5c11a0cc..556f4f1871 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -1089,6 +1089,7 @@ struct AddressSpace {
     struct FlatView *current_map;
 
     int ioeventfd_nb;
+    int ioeventfd_notifiers;
     struct MemoryRegionIoeventfd *ioeventfds;
     QTAILQ_HEAD(, MemoryListener) listeners;
     QTAILQ_ENTRY(AddressSpace) address_spaces_link;
diff --git a/softmmu/memory.c b/softmmu/memory.c
index 7d9494ce70..77042d3f93 100644
--- a/softmmu/memory.c
+++ b/softmmu/memory.c
@@ -842,6 +842,10 @@ static void address_space_update_ioeventfds(AddressSpace *as)
     AddrRange tmp;
     unsigned i;
 
+    if (!as->ioeventfd_notifiers) {
+        return;
+    }
+
     /*
      * 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
@@ -3075,6 +3079,10 @@ void memory_listener_register(MemoryListener *listener, AddressSpace *as)
     }
 
     listener_add_address_space(listener, as);
+
+    if (listener->eventfd_add || listener->eventfd_del) {
+        as->ioeventfd_notifiers++;
+    }
 }
 
 void memory_listener_unregister(MemoryListener *listener)
@@ -3083,6 +3091,10 @@ void memory_listener_unregister(MemoryListener *listener)
         return;
     }
 
+    if (listener->eventfd_add || listener->eventfd_del) {
+        as->ioeventfd_notifiers--;
+    }
+
     listener_del_address_space(listener, listener->address_space);
     QTAILQ_REMOVE(&memory_listeners, listener, link);
     QTAILQ_REMOVE(&listener->address_space->listeners, listener, link_as);
-- 
2.11.0

[PATCH] memory: avoid updating ioeventfds for some address_space

[hongmianquan]

When updating ioeventfds, we need to iterate all address spaces,
but some address spaces do not register eventfd_add|del call when
memory_listener_register() and they do nothing when updating ioeventfds.
So we can skip these AS in address_space_update_ioeventfds().

The overhead of memory_region_transaction_commit() can be significantly
reduced. For example, a VM with 8 vhost net devices and each one has
64 vectors, can reduce the time spent on memory_region_transaction_commit by 20%.

Signed-off-by: hongmianquan <hongmianquan@bytedance.com>
---
 include/exec/memory.h |  1 +
 softmmu/memory.c      | 12 ++++++++++++
 2 files changed, 13 insertions(+)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 7f5c11a0cc..556f4f1871 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -1089,6 +1089,7 @@ struct AddressSpace {
     struct FlatView *current_map;
 
     int ioeventfd_nb;
+    int ioeventfd_notifiers;
     struct MemoryRegionIoeventfd *ioeventfds;
     QTAILQ_HEAD(, MemoryListener) listeners;
     QTAILQ_ENTRY(AddressSpace) address_spaces_link;
diff --git a/softmmu/memory.c b/softmmu/memory.c
index 7d9494ce70..178816c845 100644
--- a/softmmu/memory.c
+++ b/softmmu/memory.c
@@ -842,6 +842,10 @@ static void address_space_update_ioeventfds(AddressSpace *as)
     AddrRange tmp;
     unsigned i;
 
+    if (!as->ioeventfd_notifiers) {
+        return;
+    }
+
     /*
      * 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
@@ -3075,6 +3079,10 @@ void memory_listener_register(MemoryListener *listener, AddressSpace *as)
     }
 
     listener_add_address_space(listener, as);
+
+    if (listener->eventfd_add || listener->eventfd_del) {
+        as->ioeventfd_notifiers++;
+    }
 }
 
 void memory_listener_unregister(MemoryListener *listener)
@@ -3083,6 +3091,10 @@ void memory_listener_unregister(MemoryListener *listener)
         return;
     }
 
+    if (listener->eventfd_add || listener->eventfd_del) {
+        listener->address_space->ioeventfd_notifiers--;
+    }
+
     listener_del_address_space(listener, listener->address_space);
     QTAILQ_REMOVE(&memory_listeners, listener, link);
     QTAILQ_REMOVE(&listener->address_space->listeners, listener, link_as);
-- 
2.11.0

Re: [PATCH] memory: avoid updating ioeventfds for some address_space

[Peter Xu]

On Tue, Jul 25, 2023 at 07:20:37PM +0800, hongmianquan wrote:
> When updating ioeventfds, we need to iterate all address spaces,
> but some address spaces do not register eventfd_add|del call when
> memory_listener_register() and they do nothing when updating ioeventfds.
> So we can skip these AS in address_space_update_ioeventfds().
> 
> The overhead of memory_region_transaction_commit() can be significantly
> reduced. For example, a VM with 8 vhost net devices and each one has
> 64 vectors, can reduce the time spent on memory_region_transaction_commit by 20%.
> 
> Signed-off-by: hongmianquan <hongmianquan@bytedance.com>

Reviewed-by: Peter Xu <peterx@redhat.com>

Should be for 8.2, though.  Please always copy Paolo for memory related
patches.  I hope Paolo can see this.

-- 
Peter Xu

Re: [External] Re: [PATCH] memory: avoid updating ioeventfds for some address_space

[hongmainquan]

在 2023/7/27 1:45 上午, Peter Xu 写道:
> On Tue, Jul 25, 2023 at 07:20:37PM +0800, hongmianquan wrote:
>> When updating ioeventfds, we need to iterate all address spaces,
>> but some address spaces do not register eventfd_add|del call when
>> memory_listener_register() and they do nothing when updating ioeventfds.
>> So we can skip these AS in address_space_update_ioeventfds().
>>
>> The overhead of memory_region_transaction_commit() can be significantly
>> reduced. For example, a VM with 8 vhost net devices and each one has
>> 64 vectors, can reduce the time spent on memory_region_transaction_commit by 20%.
>>
>> Signed-off-by: hongmianquan <hongmianquan@bytedance.com>
> 
> Reviewed-by: Peter Xu <peterx@redhat.com>
> 
> Should be for 8.2, though.  Please always copy Paolo for memory related
> patches.  I hope Paolo can see this.
> 
Thanks, I hope so. Also, I'm not quite sure what 'Should be for 8.2' 
means. Does it imply that there will be changes to this logic after 
version 8.2?

Re: [External] Re: [PATCH] memory: avoid updating ioeventfds for some address_space

[Peter Xu]

On Thu, Jul 27, 2023 at 11:59:43AM +0800, hongmainquan wrote:
> 
> 
> 在 2023/7/27 1:45 上午, Peter Xu 写道:
> > On Tue, Jul 25, 2023 at 07:20:37PM +0800, hongmianquan wrote:
> > > When updating ioeventfds, we need to iterate all address spaces,
> > > but some address spaces do not register eventfd_add|del call when
> > > memory_listener_register() and they do nothing when updating ioeventfds.
> > > So we can skip these AS in address_space_update_ioeventfds().
> > > 
> > > The overhead of memory_region_transaction_commit() can be significantly
> > > reduced. For example, a VM with 8 vhost net devices and each one has
> > > 64 vectors, can reduce the time spent on memory_region_transaction_commit by 20%.
> > > 
> > > Signed-off-by: hongmianquan <hongmianquan@bytedance.com>
> > 
> > Reviewed-by: Peter Xu <peterx@redhat.com>
> > 
> > Should be for 8.2, though.  Please always copy Paolo for memory related
> > patches.  I hope Paolo can see this.
> > 
> Thanks, I hope so. Also, I'm not quite sure what 'Should be for 8.2' means.
> Does it imply that there will be changes to this logic after version 8.2?

See:

https://wiki.qemu.org/Planning/8.1

We're already right before 8.1-rc2 release, perf patch isn't normally the
target of this phase.

Thanks,

-- 
Peter Xu

Re: [External] Re: [PATCH] memory: avoid updating ioeventfds for some address_space

[hongmainquan]

在 2023/7/27 8:53 下午, Peter Xu 写道:
> On Thu, Jul 27, 2023 at 11:59:43AM +0800, hongmainquan wrote:
>>
>>
>> 在 2023/7/27 1:45 上午, Peter Xu 写道:
>>> On Tue, Jul 25, 2023 at 07:20:37PM +0800, hongmianquan wrote:
>>>> When updating ioeventfds, we need to iterate all address spaces,
>>>> but some address spaces do not register eventfd_add|del call when
>>>> memory_listener_register() and they do nothing when updating ioeventfds.
>>>> So we can skip these AS in address_space_update_ioeventfds().
>>>>
>>>> The overhead of memory_region_transaction_commit() can be significantly
>>>> reduced. For example, a VM with 8 vhost net devices and each one has
>>>> 64 vectors, can reduce the time spent on memory_region_transaction_commit by 20%.
>>>>
>>>> Signed-off-by: hongmianquan <hongmianquan@bytedance.com>
>>>
>>> Reviewed-by: Peter Xu <peterx@redhat.com>
>>>
>>> Should be for 8.2, though.  Please always copy Paolo for memory related
>>> patches.  I hope Paolo can see this.
>>>
>> Thanks, I hope so. Also, I'm not quite sure what 'Should be for 8.2' means.
>> Does it imply that there will be changes to this logic after version 8.2?
> 
> See:
> 
> https://wiki.qemu.org/Planning/8.1
> 
> We're already right before 8.1-rc2 release, perf patch isn't normally the
> target of this phase.
> 
> Thanks,
> 
Understood. Hope for some suggestions from you.

Thanks!

Re: [External] Re: [PATCH] memory: avoid updating ioeventfds for some address_space

[Peter Xu]

On Thu, Jul 27, 2023 at 09:23:34PM +0800, hongmainquan wrote:
> 
> 
> 在 2023/7/27 8:53 下午, Peter Xu 写道:
> > On Thu, Jul 27, 2023 at 11:59:43AM +0800, hongmainquan wrote:
> > > 
> > > 
> > > 在 2023/7/27 1:45 上午, Peter Xu 写道:
> > > > On Tue, Jul 25, 2023 at 07:20:37PM +0800, hongmianquan wrote:
> > > > > When updating ioeventfds, we need to iterate all address spaces,
> > > > > but some address spaces do not register eventfd_add|del call when
> > > > > memory_listener_register() and they do nothing when updating ioeventfds.
> > > > > So we can skip these AS in address_space_update_ioeventfds().
> > > > > 
> > > > > The overhead of memory_region_transaction_commit() can be significantly
> > > > > reduced. For example, a VM with 8 vhost net devices and each one has
> > > > > 64 vectors, can reduce the time spent on memory_region_transaction_commit by 20%.
> > > > > 
> > > > > Signed-off-by: hongmianquan <hongmianquan@bytedance.com>
> > > > 
> > > > Reviewed-by: Peter Xu <peterx@redhat.com>
> > > > 
> > > > Should be for 8.2, though.  Please always copy Paolo for memory related
> > > > patches.  I hope Paolo can see this.
> > > > 
> > > Thanks, I hope so. Also, I'm not quite sure what 'Should be for 8.2' means.
> > > Does it imply that there will be changes to this logic after version 8.2?
> > 
> > See:
> > 
> > https://wiki.qemu.org/Planning/8.1
> > 
> > We're already right before 8.1-rc2 release, perf patch isn't normally the
> > target of this phase.
> > 
> > Thanks,
> > 
> Understood. Hope for some suggestions from you.

No further suggestion from my side. You can just keep an eye on this patch
after the 8.1 release - it probably just won't get merged before that.

Some maintainers prefer a resend after the release, but many don't.  It's
optional in this case I think.

Thanks,

-- 
Peter Xu

Re: [External] Re: [PATCH] memory: avoid updating ioeventfds for some address_space

[hongmainquan]

在 2023/7/27 10:36 下午, Peter Xu 写道:
> On Thu, Jul 27, 2023 at 09:23:34PM +0800, hongmainquan wrote:
>>
>>
>> 在 2023/7/27 8:53 下午, Peter Xu 写道:
>>> On Thu, Jul 27, 2023 at 11:59:43AM +0800, hongmainquan wrote:
>>>>
>>>>
>>>> 在 2023/7/27 1:45 上午, Peter Xu 写道:
>>>>> On Tue, Jul 25, 2023 at 07:20:37PM +0800, hongmianquan wrote:
>>>>>> When updating ioeventfds, we need to iterate all address spaces,
>>>>>> but some address spaces do not register eventfd_add|del call when
>>>>>> memory_listener_register() and they do nothing when updating ioeventfds.
>>>>>> So we can skip these AS in address_space_update_ioeventfds().
>>>>>>
>>>>>> The overhead of memory_region_transaction_commit() can be significantly
>>>>>> reduced. For example, a VM with 8 vhost net devices and each one has
>>>>>> 64 vectors, can reduce the time spent on memory_region_transaction_commit by 20%.
>>>>>>
>>>>>> Signed-off-by: hongmianquan <hongmianquan@bytedance.com>
>>>>>
>>>>> Reviewed-by: Peter Xu <peterx@redhat.com>
>>>>>
>>>>> Should be for 8.2, though.  Please always copy Paolo for memory related
>>>>> patches.  I hope Paolo can see this.
>>>>>
>>>> Thanks, I hope so. Also, I'm not quite sure what 'Should be for 8.2' means.
>>>> Does it imply that there will be changes to this logic after version 8.2?
>>>
>>> See:
>>>
>>> https://wiki.qemu.org/Planning/8.1
>>>
>>> We're already right before 8.1-rc2 release, perf patch isn't normally the
>>> target of this phase.
>>>
>>> Thanks,
>>>
>> Understood. Hope for some suggestions from you.
> 
> No further suggestion from my side. You can just keep an eye on this patch
> after the 8.1 release - it probably just won't get merged before that.
> 
> Some maintainers prefer a resend after the release, but many don't.  It's
> optional in this case I think.
> 
> Thanks,
> 
Got it, thank you very much!