Hi, > -----Original Message----- > From: Paolo Bonzini [mailto:[email protected]] > Sent: Thursday, May 08, 2014 8:49 PM > To: Gonglei (Arei); [email protected] > Cc: [email protected]; [email protected]; [email protected]; > [email protected]; Huangweidong (C); Herongguang (Stephen) > Subject: Re: [PATCH] memory: Don't update all memory region when ioeventfd > changed > > Il 08/05/2014 05:47, [email protected] ha scritto: > > From: Gonglei <[email protected]> > > > > 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 <[email protected]> > > Signed-off-by: Herongguang <[email protected]> > > --- > > 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
