Walking the page lists needs the page_alloc lock

There are a few places in Xen where we walk a domain's page lists
without holding the page_alloc lock.  They race with updates to the
page lists, which are normally rare but can be quite common under PoD
when the domain is close to its memory limit and the PoD reclaimer is
busy.  This patch protects those places by taking the page_alloc lock.

I think this is OK for the two debug-key printouts - they don't run
from irq context and look deadlock-free.  The tboot change seems safe
too unless tboot shutdown functions are called from irq context or
with the page_alloc lock held.  The p2m one is the scariest but there
are already code paths in PoD that take the page_alloc lock with the
p2m lock held so it's no worse than existing code.

Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com>
xen-unstable changeset:   21881:57de3a3118bb
xen-unstable date:        Wed Jul 28 07:54:12 2010 +0100

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index d4a0c44caaaf20edaf6fed3334e64cb847f3e6ec..48db6363cf81d9e3cfb9a9920c12d8653a81127a 100644 (file)
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -150,12 +150,14 @@ void dump_pageframe_info(struct domain *d)
     }
     else
     {
+        spin_lock(&d->page_alloc_lock);
         page_list_for_each ( page, &d->page_list )
         {
             printk("    DomPage %p: caf=%08lx, taf=%" PRtype_info "\n",
                    _p(page_to_mfn(page)),
                    page->count_info, page->u.inuse.type_info);
         }
+        spin_unlock(&d->page_alloc_lock);
     }
 
     if ( is_hvm_domain(d) )
@@ -163,12 +165,14 @@ void dump_pageframe_info(struct domain *d)
         p2m_pod_dump_data(d);
     }
 
+    spin_lock(&d->page_alloc_lock);
     page_list_for_each ( page, &d->xenpage_list )
     {
         printk("    XenPage %p: caf=%08lx, taf=%" PRtype_info "\n",
                _p(page_to_mfn(page)),
                page->count_info, page->u.inuse.type_info);
     }
+    spin_unlock(&d->page_alloc_lock);
 }
 
 struct domain *alloc_domain_struct(void)

diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index c90c6743ba1b83f93c2b050b87cfb893c741591d..740e9af628a52ffb8e721a301873c989f02d991e 100644 (file)
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1674,6 +1674,7 @@ int p2m_alloc_table(struct domain *d,
         goto error;
 
     /* Copy all existing mappings from the page list and m2p */
+    spin_lock(&d->page_alloc_lock);
     page_list_for_each(page, &d->page_list)
     {
         mfn = page_to_mfn(page);
@@ -1689,13 +1690,16 @@ int p2m_alloc_table(struct domain *d,
 #endif
              && gfn != INVALID_M2P_ENTRY
             && !set_p2m_entry(d, gfn, mfn, 0, p2m_ram_rw) )
-            goto error;
+            goto error_unlock;
     }
+    spin_unlock(&d->page_alloc_lock);
 
     P2M_PRINTK("p2m table initialised (%u pages)\n", page_count);
     p2m_unlock(p2m);
     return 0;
 
+error_unlock:
+    spin_unlock(&d->page_alloc_lock);
  error:
     P2M_PRINTK("failed to initialize p2m table, gfn=%05lx, mfn=%"
                PRI_mfn "\n", gfn, mfn_x(mfn));

diff --git a/xen/arch/x86/numa.c b/xen/arch/x86/numa.c
index 75d4be1d202035f64ef71bfb9aa1eb4bedc7cb5a..3101981f7cc113a4e72d3d1e40b4b39bbbb73ee4 100644 (file)
--- a/xen/arch/x86/numa.c
+++ b/xen/arch/x86/numa.c
@@ -380,11 +380,13 @@ static void dump_numa(unsigned char key)
                for_each_online_node(i)
                        page_num_node[i] = 0;
 
+               spin_lock(&d->page_alloc_lock);
                page_list_for_each(page, &d->page_list)
                {
                        i = phys_to_nid((paddr_t)page_to_mfn(page) << PAGE_SHIFT);
                        page_num_node[i]++;
                }
+               spin_unlock(&d->page_alloc_lock);
 
                for_each_online_node(i)
                        printk("    Node %u: %u\n", i, page_num_node[i]);

diff --git a/xen/arch/x86/tboot.c b/xen/arch/x86/tboot.c
index 9e0205b7387c4cf1203af8d1ed82639311c1fd3b..f5930401a2d5ed65c4832077a2ce4a3187d8d70e 100644 (file)
--- a/xen/arch/x86/tboot.c
+++ b/xen/arch/x86/tboot.c
@@ -211,12 +211,14 @@ static void tboot_gen_domain_integrity(const uint8_t key[TB_KEY_SIZE],
             continue;
         printk("MACing Domain %u\n", d->domain_id);
 
+        spin_lock(&d->page_alloc_lock);
         page_list_for_each(page, &d->page_list)
         {
             void *pg = __map_domain_page(page);
             vmac_update(pg, PAGE_SIZE, &ctx);
             unmap_domain_page(pg);
         }
+        spin_unlock(&d->page_alloc_lock);
 
         if ( !is_idle_domain(d) )
         {