[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH v3 4/8] memory: Don't do topology update in memory finalize()
From: |
Peter Xu |
Subject: |
[PATCH v3 4/8] memory: Don't do topology update in memory finalize() |
Date: |
Wed, 28 Jul 2021 14:31:47 -0400 |
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 <pbonzini@redhat.com>
Acked-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Peter Xu <peterx@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
- [PATCH v3 0/8] memory: Sanity checks memory transaction when releasing BQL, Peter Xu, 2021/07/28
- [PATCH v3 1/8] cpus: Export queue work related fields to cpu.h, Peter Xu, 2021/07/28
- [PATCH v3 3/8] memory: Introduce memory_region_transaction_depth_{inc|dec}(), Peter Xu, 2021/07/28
- [PATCH v3 2/8] cpus: Move do_run_on_cpu into softmmu/cpus.c, Peter Xu, 2021/07/28
- [PATCH v3 4/8] memory: Don't do topology update in memory finalize(),
Peter Xu <=
- [PATCH v3 5/8] cpus: Use qemu_cond_wait_iothread() where proper, Peter Xu, 2021/07/28
- [PATCH v3 6/8] cpus: Remove the mutex parameter from do_run_on_cpu(), Peter Xu, 2021/07/28
- [PATCH v3 8/8] memory: Delay the transaction pop() until commit completed, Peter Xu, 2021/07/28
- [PATCH v3 7/8] memory: Assert on no ongoing memory transaction before release BQL, Peter Xu, 2021/07/28