Re: [Qemu-devel] [PATCH 12/23] memory: separate building the final memory map into two steps

[Anthony Liguori]

On 07/25/2011 09:02 AM, Avi Kivity wrote:

Instead of adding and deleting regions in one pass, do a delete
pass followed by an add pass.  This fixes the following case:

from:
   0x0000-0x0fff ram  (a1)
   0x1000-0x1fff mmio (a2)
   0x2000-0x2fff ram  (a3)

to:
   0x0000-0x2fff ram  (b1)

The single pass algorithm removed a1, added b2, then removed a2 and a2,

You mean a2 and a3 I suppose?

Regards,

Anthony Liguori

which caused the wrong memory map to be built.  The two pass algorithm
removes a1, a2, and a3, then adds b1.

Signed-off-by: Avi Kivity<a...@redhat.com>
---
  memory.c |   38 +++++++++++++++++++++++++++++---------
  1 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/memory.c b/memory.c
index a5cde0c..009ad33 100644
--- a/memory.c
+++ b/memory.c
@@ -546,10 +546,11 @@ static void address_space_update_ioeventfds(AddressSpace 
*as)
      as->ioeventfd_nb = ioeventfd_nb;
  }

-static void address_space_update_topology(AddressSpace *as)
+static void address_space_update_topology_pass(AddressSpace *as,
+                                               FlatView old_view,
+                                               FlatView new_view,
+                                               bool adding)
  {
-    FlatView old_view = as->current_map;
-    FlatView new_view = generate_memory_topology(as->root);
      unsigned iold, inew;
      FlatRange *frold, *frnew;

@@ -576,15 +577,20 @@ static void address_space_update_topology(AddressSpace 
*as)
                      &&  !flatrange_equal(frold, frnew)))) {
              /* In old, but (not in new, or in new but attributes changed). */

-            as->ops->range_del(as, frold);
+            if (!adding) {
+                as->ops->range_del(as, frold);
+            }
+
              ++iold;
          } else if (frold&&  frnew&&  flatrange_equal(frold, frnew)) {
              /* In both (logging may have changed) */

-            if (frold->dirty_log_mask&&  !frnew->dirty_log_mask) {
-                as->ops->log_stop(as, frnew);
-            } else if (frnew->dirty_log_mask&&  !frold->dirty_log_mask) {
-                as->ops->log_start(as, frnew);
+            if (adding) {
+                if (frold->dirty_log_mask&&  !frnew->dirty_log_mask) {
+                    as->ops->log_stop(as, frnew);
+                } else if (frnew->dirty_log_mask&&  !frold->dirty_log_mask) {
+                    as->ops->log_start(as, frnew);
+                }
              }

              ++iold;
@@ -592,10 +598,24 @@ static void address_space_update_topology(AddressSpace 
*as)
          } else {
              /* In new */

-            as->ops->range_add(as, frnew);
+            if (adding) {
+                as->ops->range_add(as, frnew);
+            }
+
              ++inew;
          }
      }
+}
+
+
+static void address_space_update_topology(AddressSpace *as)
+{
+    FlatView old_view = as->current_map;
+    FlatView new_view = generate_memory_topology(as->root);
+
+    address_space_update_topology_pass(as, old_view, new_view, false);
+    address_space_update_topology_pass(as, old_view, new_view, true);
+
      as->current_map = new_view;
      flatview_destroy(&old_view);
      address_space_update_ioeventfds(as);