[Xen-changelog] Bug fix to stop leaking out-of-sync entries, and to stop trashing

[BitKeeper Bot <riel@xxxxxxxxxxx>]

ChangeSet 1.1236.32.7, 2005/03/16 01:12:53+00:00, mafetter@xxxxxxxxxxxxxxxx

        Bug fix to stop leaking out-of-sync entries, and to stop trashing
        the list of out-of-sync entries by manipulating it in one routine
        while walking it in another.
        
        Signed-off-by: michael.fetterman@xxxxxxxxxxxx



 arch/x86/shadow.c        |   60 +++++++++++++++++++++++++++++++++++------------
 include/asm-x86/shadow.h |    9 +++++--
 2 files changed, 52 insertions(+), 17 deletions(-)


diff -Nru a/xen/arch/x86/shadow.c b/xen/arch/x86/shadow.c
--- a/xen/arch/x86/shadow.c     2005-04-05 12:09:34 -04:00
+++ b/xen/arch/x86/shadow.c     2005-04-05 12:09:34 -04:00
@@ -70,7 +70,7 @@
         min_type = PGT_l1_shadow;
         max_type = PGT_l1_shadow;
     }
-    FSH_LOG("shadow_promote gpfn=%p gmfn=%p nt=%p min=%p max=%p\n",
+    FSH_LOG("shadow_promote gpfn=%p gmfn=%p nt=%p min=%p max=%p",
             gmfn, gmfn, new_type, min_type, max_type);
 
     if ( min_type <= max_type )
@@ -388,7 +388,12 @@
     // Only use entries that have low bits clear...
     //
     if ( !(entry->writable_pl1e & (sizeof(l1_pgentry_t)-1)) )
+    {
         put_shadow_ref(entry->writable_pl1e >> PAGE_SHIFT);
+        entry->writable_pl1e = -2;
+    }
+    else
+        ASSERT( entry->writable_pl1e == -1 );
 
     // Free the snapshot
     //
@@ -399,38 +404,63 @@
 {
     struct out_of_sync_entry *entry = d->arch.out_of_sync;
     struct out_of_sync_entry **prev = &d->arch.out_of_sync;
+    struct out_of_sync_entry *found = NULL;
 
+    // NB: Be careful not to call something that manipulates this list
+    //     while walking it.  Collect the results into a separate list
+    //     first, then walk that list.
+    //
     while ( entry )
     {
         if ( entry->gmfn == gmfn )
         {
-            release_out_of_sync_entry(d, entry);
-            *prev = entry = entry->next;
+            // remove from out of sync list
+            *prev = entry->next;
+
+            // add to found list
+            entry->next = found;
+            found = entry;
+
+            entry = *prev;
             continue;
         }
         prev = &entry->next;
         entry = entry->next;
     }
+
+    prev = NULL;
+    entry = found;
+    while ( entry )
+    {
+        release_out_of_sync_entry(d, entry);
+
+        prev = &entry->next;
+        entry = entry->next;
+    }
+
+    // Add found list to free list
+    if ( prev )
+    {
+        *prev = d->arch.out_of_sync_free;
+        d->arch.out_of_sync_free = found;
+    }
 }
 
 static void free_out_of_sync_state(struct domain *d)
 {
     struct out_of_sync_entry *entry;
-    struct out_of_sync_entry **tail = NULL;
 
-    // Add the list of out-of-sync entries to the free list of entries.
-    // Not the smartest code.  But it works.
+    // NB: Be careful not to call something that manipulates this list
+    //     while walking it.  Remove one item at a time, and always
+    //     restart from start of list.
     //
-    for ( entry = d->arch.out_of_sync; entry; entry = entry->next)
+    while ( (entry = d->arch.out_of_sync) )
     {
+        d->arch.out_of_sync = entry->next;
         release_out_of_sync_entry(d, entry);
-        tail = &entry->next;
-    }
-    if ( tail )
-    {
-        *tail = d->arch.out_of_sync_free;
-        d->arch.out_of_sync_free = d->arch.out_of_sync;
-        d->arch.out_of_sync = NULL;
+
+        entry->next = d->arch.out_of_sync_free;
+        d->arch.out_of_sync_free = entry;
     }
 }
 
@@ -1515,7 +1545,7 @@
                 put_page_from_l1e(mk_l1_pgentry(old), d);
 
             FSH_LOG("removed write access to mfn=%p in smfn=%p entry %x "
-                    "is_l1_shadow=%d\n",
+                    "is_l1_shadow=%d",
                     readonly_mfn, pt_mfn, i, is_l1_shadow);
         }
     }
diff -Nru a/xen/include/asm-x86/shadow.h b/xen/include/asm-x86/shadow.h
--- a/xen/include/asm-x86/shadow.h      2005-04-05 12:09:34 -04:00
+++ b/xen/include/asm-x86/shadow.h      2005-04-05 12:09:34 -04:00
@@ -375,8 +375,9 @@
 
     if ( unlikely(x == 0) )
     {
-        printk("put_shadow_ref underflow, gmfn=%p smfn=%p\n",
-               frame_table[smfn].u.inuse.type_info & PGT_mfn_mask, smfn);
+        printk("put_shadow_ref underflow, oc=%p t=%p\n",
+               frame_table[smfn].count_info,
+               frame_table[smfn].u.inuse.type_info);
         BUG();
     }
 
@@ -745,6 +746,9 @@
         BUG();
     }
 #endif
+
+    // XXX ought to add some code to audit the out-of-sync entries, too.
+    //
 }
 #else
 #define shadow_audit(p, print) ((void)0)
@@ -1041,6 +1045,7 @@
     {
         if ( x->gpfn_and_flags == key )
         {
+            BUG();
             x->smfn = smfn;
             goto done;
         }

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog