Topology update could be wrongly triggered in memory region finalize() if there's bug somewhere else. It'll be a very confusing stack when it happens (e.g., sending KVM ioctl within the RCU thread, and we'll observe it only until it fails!).
Instead of that, we use the push()/pop() helper to avoid memory transaction commit, at the same time we use assertions to make sure there's no pending updates or it's a nested transaction, so it could fail even earlier and in a more explicit way. Suggested-by: Paolo Bonzini <pbonz...@redhat.com> Acked-by: David Hildenbrand <da...@redhat.com> Signed-off-by: Peter Xu <pet...@redhat.com> --- softmmu/memory.c | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/softmmu/memory.c b/softmmu/memory.c index 725d57ec17..35b2568fc2 100644 --- a/softmmu/memory.c +++ b/softmmu/memory.c @@ -170,6 +170,12 @@ struct MemoryRegionIoeventfd { EventNotifier *e; }; +/* Returns whether there's any pending memory updates */ +static bool memory_region_has_pending_update(void) +{ + return memory_region_update_pending || ioeventfd_update_pending; +} + static bool memory_region_ioeventfd_before(MemoryRegionIoeventfd *a, MemoryRegionIoeventfd *b) { @@ -1756,12 +1762,25 @@ static void memory_region_finalize(Object *obj) * and cause an infinite loop. */ mr->enabled = false; - memory_region_transaction_begin(); + + /* + * Use depth_inc()/depth_dec() instead of begin()/commit() to make sure + * below block won't trigger any topology update (which should never + * happen, but it's still a safety belt). + */ + memory_region_transaction_depth_inc(); while (!QTAILQ_EMPTY(&mr->subregions)) { MemoryRegion *subregion = QTAILQ_FIRST(&mr->subregions); memory_region_del_subregion(mr, subregion); } - memory_region_transaction_commit(); + memory_region_transaction_depth_dec(); + + /* + * Make sure we're either in a nested transaction or there must have no + * pending updates due to memory_region_del_subregion() above. + */ + assert(memory_region_transaction_depth || + !memory_region_has_pending_update()); mr->destructor(mr); memory_region_clear_coalescing(mr); -- 2.31.1