| Message ID | 20230725112037.1762608-1-hongmianquan@bytedance.com |
|---|---|
| State | New |
| Headers | show |
| Series | memory: avoid updating ioeventfds for some address_space | expand |
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.
在 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?
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,
在 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!
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,
在 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!
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);
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(+)