* [Qemu-devel] [PATCH] memory: Don't update all memory region when ioeventfd changed
@ 2014-05-08 3:47 arei.gonglei
2014-05-08 12:48 ` Paolo Bonzini
0 siblings, 1 reply; 3+ messages in thread
From: arei.gonglei @ 2014-05-08 3:47 UTC (permalink / raw)
To: qemu-devel
Cc: peter.maydell, weidong.huang, mst, Gonglei, chris.friesen,
pbonzini, Herongguang, rth
From: Gonglei <arei.gonglei@huawei.com>
memory mappings don't rely on ioeventfds, there is no need
to destroy and rebuild them when manipulating ioeventfds,
otherwise it scarifies performance.
according to testing result, each ioeventfd deleing needs
about 5ms, within which memory mapping rebuilding needs
about 4ms. With many Nics and vmchannel in a VM doing migrating,
there can be many ioeventfds deleting which increasing
downtime remarkably.
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Signed-off-by: Herongguang <herongguang.he@huawei.com>
---
memory.c | 33 +++++++++++++++++++++++----------
1 file changed, 23 insertions(+), 10 deletions(-)
diff --git a/memory.c b/memory.c
index 3f1df23..2c783f6 100644
--- a/memory.c
+++ b/memory.c
@@ -28,6 +28,7 @@
static unsigned memory_region_transaction_depth;
static bool memory_region_update_pending;
+static bool ioeventfd_update_pending;
static bool global_dirty_log = false;
/* flat_view_mutex is taken around reading as->current_map; the critical
@@ -786,22 +787,34 @@ void memory_region_transaction_begin(void)
++memory_region_transaction_depth;
}
+static void memory_region_clear_pending(void)
+{
+ memory_region_update_pending = false;
+ ioeventfd_update_pending = false;
+}
+
void memory_region_transaction_commit(void)
{
AddressSpace *as;
assert(memory_region_transaction_depth);
--memory_region_transaction_depth;
- if (!memory_region_transaction_depth && memory_region_update_pending) {
- memory_region_update_pending = false;
- MEMORY_LISTENER_CALL_GLOBAL(begin, Forward);
+ if (!memory_region_transaction_depth) {
+ if (memory_region_update_pending) {
+ MEMORY_LISTENER_CALL_GLOBAL(begin, Forward);
- QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
- address_space_update_topology(as);
- }
+ QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
+ address_space_update_topology(as);
+ }
- MEMORY_LISTENER_CALL_GLOBAL(commit, Forward);
- }
+ MEMORY_LISTENER_CALL_GLOBAL(commit, Forward);
+ } else if (ioeventfd_update_pending) {
+ QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
+ address_space_update_ioeventfds(as);
+ }
+ }
+ memory_region_clear_pending();
+ }
}
static void memory_region_destructor_none(MemoryRegion *mr)
@@ -1373,7 +1386,7 @@ void memory_region_add_eventfd(MemoryRegion *mr,
memmove(&mr->ioeventfds[i+1], &mr->ioeventfds[i],
sizeof(*mr->ioeventfds) * (mr->ioeventfd_nb-1 - i));
mr->ioeventfds[i] = mrfd;
- memory_region_update_pending |= mr->enabled;
+ ioeventfd_update_pending |= mr->enabled;
memory_region_transaction_commit();
}
@@ -1406,7 +1419,7 @@ void memory_region_del_eventfd(MemoryRegion *mr,
--mr->ioeventfd_nb;
mr->ioeventfds = g_realloc(mr->ioeventfds,
sizeof(*mr->ioeventfds)*mr->ioeventfd_nb + 1);
- memory_region_update_pending |= mr->enabled;
+ ioeventfd_update_pending |= mr->enabled;
memory_region_transaction_commit();
}
--
1.7.12.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] memory: Don't update all memory region when ioeventfd changed
2014-05-08 3:47 [Qemu-devel] [PATCH] memory: Don't update all memory region when ioeventfd changed arei.gonglei
@ 2014-05-08 12:48 ` Paolo Bonzini
2014-05-08 13:22 ` Gonglei (Arei)
0 siblings, 1 reply; 3+ messages in thread
From: Paolo Bonzini @ 2014-05-08 12:48 UTC (permalink / raw)
To: arei.gonglei, qemu-devel
Cc: peter.maydell, weidong.huang, mst, chris.friesen, Herongguang,
rth
Il 08/05/2014 05:47, arei.gonglei@huawei.com ha scritto:
> From: Gonglei <arei.gonglei@huawei.com>
>
> memory mappings don't rely on ioeventfds, there is no need
> to destroy and rebuild them when manipulating ioeventfds,
> otherwise it scarifies performance.
>
> according to testing result, each ioeventfd deleing needs
> about 5ms, within which memory mapping rebuilding needs
> about 4ms. With many Nics and vmchannel in a VM doing migrating,
> there can be many ioeventfds deleting which increasing
> downtime remarkably.
>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> Signed-off-by: Herongguang <herongguang.he@huawei.com>
> ---
> memory.c | 33 +++++++++++++++++++++++----------
> 1 file changed, 23 insertions(+), 10 deletions(-)
>
> diff --git a/memory.c b/memory.c
> index 3f1df23..2c783f6 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -28,6 +28,7 @@
>
> static unsigned memory_region_transaction_depth;
> static bool memory_region_update_pending;
> +static bool ioeventfd_update_pending;
> static bool global_dirty_log = false;
>
> /* flat_view_mutex is taken around reading as->current_map; the critical
> @@ -786,22 +787,34 @@ void memory_region_transaction_begin(void)
> ++memory_region_transaction_depth;
> }
>
> +static void memory_region_clear_pending(void)
> +{
> + memory_region_update_pending = false;
> + ioeventfd_update_pending = false;
> +}
> +
> void memory_region_transaction_commit(void)
> {
> AddressSpace *as;
>
> assert(memory_region_transaction_depth);
> --memory_region_transaction_depth;
> - if (!memory_region_transaction_depth && memory_region_update_pending) {
> - memory_region_update_pending = false;
> - MEMORY_LISTENER_CALL_GLOBAL(begin, Forward);
> + if (!memory_region_transaction_depth) {
> + if (memory_region_update_pending) {
> + MEMORY_LISTENER_CALL_GLOBAL(begin, Forward);
>
> - QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
> - address_space_update_topology(as);
> - }
> + QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
> + address_space_update_topology(as);
> + }
>
> - MEMORY_LISTENER_CALL_GLOBAL(commit, Forward);
> - }
> + MEMORY_LISTENER_CALL_GLOBAL(commit, Forward);
> + } else if (ioeventfd_update_pending) {
> + QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
> + address_space_update_ioeventfds(as);
> + }
> + }
> + memory_region_clear_pending();
> + }
> }
>
> static void memory_region_destructor_none(MemoryRegion *mr)
> @@ -1373,7 +1386,7 @@ void memory_region_add_eventfd(MemoryRegion *mr,
> memmove(&mr->ioeventfds[i+1], &mr->ioeventfds[i],
> sizeof(*mr->ioeventfds) * (mr->ioeventfd_nb-1 - i));
> mr->ioeventfds[i] = mrfd;
> - memory_region_update_pending |= mr->enabled;
> + ioeventfd_update_pending |= mr->enabled;
> memory_region_transaction_commit();
> }
>
> @@ -1406,7 +1419,7 @@ void memory_region_del_eventfd(MemoryRegion *mr,
> --mr->ioeventfd_nb;
> mr->ioeventfds = g_realloc(mr->ioeventfds,
> sizeof(*mr->ioeventfds)*mr->ioeventfd_nb + 1);
> - memory_region_update_pending |= mr->enabled;
> + ioeventfd_update_pending |= mr->enabled;
> memory_region_transaction_commit();
> }
>
>
Applied for 2.1, thanks -- but IIUC Chris's patch (if it worked) would
improve performance even more. Is this correct?
Paolo
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] memory: Don't update all memory region when ioeventfd changed
2014-05-08 12:48 ` Paolo Bonzini
@ 2014-05-08 13:22 ` Gonglei (Arei)
0 siblings, 0 replies; 3+ messages in thread
From: Gonglei (Arei) @ 2014-05-08 13:22 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel@nongnu.org
Cc: peter.maydell@linaro.org, Huangweidong (C), mst@redhat.com,
chris.friesen@windriver.com, Herongguang (Stephen),
rth@twiddle.net
Hi,
> -----Original Message-----
> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
> Sent: Thursday, May 08, 2014 8:49 PM
> To: Gonglei (Arei); qemu-devel@nongnu.org
> Cc: rth@twiddle.net; peter.maydell@linaro.org; mst@redhat.com;
> chris.friesen@windriver.com; Huangweidong (C); Herongguang (Stephen)
> Subject: Re: [PATCH] memory: Don't update all memory region when ioeventfd
> changed
>
> Il 08/05/2014 05:47, arei.gonglei@huawei.com ha scritto:
> > From: Gonglei <arei.gonglei@huawei.com>
> >
> > memory mappings don't rely on ioeventfds, there is no need
> > to destroy and rebuild them when manipulating ioeventfds,
> > otherwise it scarifies performance.
> >
> > according to testing result, each ioeventfd deleing needs
> > about 5ms, within which memory mapping rebuilding needs
> > about 4ms. With many Nics and vmchannel in a VM doing migrating,
> > there can be many ioeventfds deleting which increasing
> > downtime remarkably.
> >
> > Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> > Signed-off-by: Herongguang <herongguang.he@huawei.com>
> > ---
> > memory.c | 33 +++++++++++++++++++++++----------
> > 1 file changed, 23 insertions(+), 10 deletions(-)
> >
> > diff --git a/memory.c b/memory.c
> > index 3f1df23..2c783f6 100644
> > --- a/memory.c
> > +++ b/memory.c
> > @@ -28,6 +28,7 @@
> >
> > static unsigned memory_region_transaction_depth;
> > static bool memory_region_update_pending;
> > +static bool ioeventfd_update_pending;
> > static bool global_dirty_log = false;
> >
> > /* flat_view_mutex is taken around reading as->current_map; the critical
> > @@ -786,22 +787,34 @@ void memory_region_transaction_begin(void)
> > ++memory_region_transaction_depth;
> > }
> >
> > +static void memory_region_clear_pending(void)
> > +{
> > + memory_region_update_pending = false;
> > + ioeventfd_update_pending = false;
> > +}
> > +
> > void memory_region_transaction_commit(void)
> > {
> > AddressSpace *as;
> >
> > assert(memory_region_transaction_depth);
> > --memory_region_transaction_depth;
> > - if (!memory_region_transaction_depth &&
> memory_region_update_pending) {
> > - memory_region_update_pending = false;
> > - MEMORY_LISTENER_CALL_GLOBAL(begin, Forward);
> > + if (!memory_region_transaction_depth) {
> > + if (memory_region_update_pending) {
> > + MEMORY_LISTENER_CALL_GLOBAL(begin, Forward);
> >
> > - QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
> > - address_space_update_topology(as);
> > - }
> > + QTAILQ_FOREACH(as, &address_spaces,
> address_spaces_link) {
> > + address_space_update_topology(as);
> > + }
> >
> > - MEMORY_LISTENER_CALL_GLOBAL(commit, Forward);
> > - }
> > + MEMORY_LISTENER_CALL_GLOBAL(commit, Forward);
> > + } else if (ioeventfd_update_pending) {
> > + QTAILQ_FOREACH(as, &address_spaces,
> address_spaces_link) {
> > + address_space_update_ioeventfds(as);
> > + }
> > + }
> > + memory_region_clear_pending();
> > + }
> > }
> >
> > static void memory_region_destructor_none(MemoryRegion *mr)
> > @@ -1373,7 +1386,7 @@ void memory_region_add_eventfd(MemoryRegion
> *mr,
> > memmove(&mr->ioeventfds[i+1], &mr->ioeventfds[i],
> > sizeof(*mr->ioeventfds) * (mr->ioeventfd_nb-1 - i));
> > mr->ioeventfds[i] = mrfd;
> > - memory_region_update_pending |= mr->enabled;
> > + ioeventfd_update_pending |= mr->enabled;
> > memory_region_transaction_commit();
> > }
> >
> > @@ -1406,7 +1419,7 @@ void memory_region_del_eventfd(MemoryRegion
> *mr,
> > --mr->ioeventfd_nb;
> > mr->ioeventfds = g_realloc(mr->ioeventfds,
> >
> sizeof(*mr->ioeventfds)*mr->ioeventfd_nb + 1);
> > - memory_region_update_pending |= mr->enabled;
> > + ioeventfd_update_pending |= mr->enabled;
> > memory_region_transaction_commit();
> > }
> >
> >
>
> Applied for 2.1, thanks -- but IIUC Chris's patch (if it worked) would
> improve performance even more. Is this correct?
>
Yep. In theory, Chri's patch make all the eventfds do a single
commit at last. It will reduce the time of update ioeventfds.
Best regards,
-Gonglei
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-05-08 13:23 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-08 3:47 [Qemu-devel] [PATCH] memory: Don't update all memory region when ioeventfd changed arei.gonglei
2014-05-08 12:48 ` Paolo Bonzini
2014-05-08 13:22 ` Gonglei (Arei)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).