[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] [PATCH] save/restore race



On Wed, Jan 24, 2007 at 02:19:42AM -0000, Ian Pratt wrote:

> 30 seconds is quite a time to wait... Wouldn't a second be more
> appropriate?
> 
> While you're at it, I'd be grateful if you could move the shared_info
> assignment in linux's post_suspend() and setup_arch() [i386 and x86_64]
> below the list building code.

Sure. Patch below is for 3.0.4 but should essentially apply to
xen-unstable too.

thanks
john

# HG changeset patch
# User john.levon@xxxxxxx
# Date 1169607991 28800
# Node ID b5ac8fda95bc5f9b789df80095d59e23e7f40205
# Parent  a75413c0072fa5b892bd8b6a05c1f1d3435bb093
Close save-after-restore race.

Make xc_linux_save() wait for the frame_list_list MFN to be updated by the
domain before trying to use it. Make Linux set the top-level MFN /after/
updating the other MFN lists.

Signed-off-by: John Levon <john.levon@xxxxxxx>

diff --git a/linux-2.6-xen-sparse/arch/i386/kernel/setup-xen.c 
b/linux-2.6-xen-sparse/arch/i386/kernel/setup-xen.c
--- a/linux-2.6-xen-sparse/arch/i386/kernel/setup-xen.c
+++ b/linux-2.6-xen-sparse/arch/i386/kernel/setup-xen.c
@@ -1777,8 +1777,6 @@ void __init setup_arch(char **cmdline_p)
                 * frames that make up the p2m table. Used by save/restore
                 */
                pfn_to_mfn_frame_list_list = alloc_bootmem_low_pages(PAGE_SIZE);
-               HYPERVISOR_shared_info->arch.pfn_to_mfn_frame_list_list =
-                    virt_to_mfn(pfn_to_mfn_frame_list_list);
 
                fpp = PAGE_SIZE/sizeof(unsigned long);
                for (i=0, j=0, k=-1; i< max_pfn; i+=fpp, j++) {
@@ -1795,6 +1793,8 @@ void __init setup_arch(char **cmdline_p)
                                virt_to_mfn(&phys_to_machine_mapping[i]);
                }
                HYPERVISOR_shared_info->arch.max_pfn = max_pfn;
+               HYPERVISOR_shared_info->arch.pfn_to_mfn_frame_list_list =
+                    virt_to_mfn(pfn_to_mfn_frame_list_list);
        }
 
        /*
diff --git a/linux-2.6-xen-sparse/arch/x86_64/kernel/setup-xen.c 
b/linux-2.6-xen-sparse/arch/x86_64/kernel/setup-xen.c
--- a/linux-2.6-xen-sparse/arch/x86_64/kernel/setup-xen.c
+++ b/linux-2.6-xen-sparse/arch/x86_64/kernel/setup-xen.c
@@ -862,8 +862,6 @@ void __init setup_arch(char **cmdline_p)
                          * save/restore.
                         */
                        pfn_to_mfn_frame_list_list = 
alloc_bootmem_pages(PAGE_SIZE);
-                       HYPERVISOR_shared_info->arch.pfn_to_mfn_frame_list_list 
=
-                               virt_to_mfn(pfn_to_mfn_frame_list_list);
 
                        fpp = PAGE_SIZE/sizeof(unsigned long);
                        for (i=0, j=0, k=-1; i< end_pfn; i+=fpp, j++) {
@@ -880,6 +878,8 @@ void __init setup_arch(char **cmdline_p)
                                        
virt_to_mfn(&phys_to_machine_mapping[i]);
                        }
                        HYPERVISOR_shared_info->arch.max_pfn = end_pfn;
+                       HYPERVISOR_shared_info->arch.pfn_to_mfn_frame_list_list 
=
+                               virt_to_mfn(pfn_to_mfn_frame_list_list);
                }
 
        }
diff --git a/linux-2.6-xen-sparse/drivers/xen/core/machine_reboot.c 
b/linux-2.6-xen-sparse/drivers/xen/core/machine_reboot.c
--- a/linux-2.6-xen-sparse/drivers/xen/core/machine_reboot.c
+++ b/linux-2.6-xen-sparse/drivers/xen/core/machine_reboot.c
@@ -98,9 +98,6 @@ static void post_suspend(void)
 
        memset(empty_zero_page, 0, PAGE_SIZE);
 
-       HYPERVISOR_shared_info->arch.pfn_to_mfn_frame_list_list =
-               virt_to_mfn(pfn_to_mfn_frame_list_list);
-
        fpp = PAGE_SIZE/sizeof(unsigned long);
        for (i = 0, j = 0, k = -1; i < max_pfn; i += fpp, j++) {
                if ((j % fpp) == 0) {
@@ -113,6 +110,8 @@ static void post_suspend(void)
                        virt_to_mfn(&phys_to_machine_mapping[i]);
        }
        HYPERVISOR_shared_info->arch.max_pfn = max_pfn;
+       HYPERVISOR_shared_info->arch.pfn_to_mfn_frame_list_list =
+               virt_to_mfn(pfn_to_mfn_frame_list_list);
 }
 
 #else /* !(defined(__i386__) || defined(__x86_64__)) */
diff --git a/tools/libxc/xc_linux_save.c b/tools/libxc/xc_linux_save.c
--- a/tools/libxc/xc_linux_save.c
+++ b/tools/libxc/xc_linux_save.c
@@ -403,6 +403,33 @@ static int suspend_and_state(int (*suspe
     return -1;
 }
 
+/*
+** Map the top-level page of MFNs from the guest. The guest might not have
+** finished resuming from a previous restore operation, so we wait a while for
+** it to update the MFN to a reasonable value.
+*/
+static void *map_frame_list_list(int xc_handle, uint32_t dom,
+                                 shared_info_t *shinfo)
+{
+    int count = 100;
+    void *p;
+
+    while (count-- && shinfo->arch.pfn_to_mfn_frame_list_list == 0)
+        usleep(10000);
+
+    if (shinfo->arch.pfn_to_mfn_frame_list_list == 0) {
+        ERROR("Timed out waiting for frame list updated.");
+        return NULL;
+    }
+
+    p = xc_map_foreign_range(xc_handle, dom, PAGE_SIZE, PROT_READ,
+                             shinfo->arch.pfn_to_mfn_frame_list_list);
+
+    if (p == NULL)
+        ERROR("Couldn't map p2m_frame_list_list (errno %d)", errno);
+
+    return p;
+}
 
 /*
 ** During transfer (or in the state file), all page-table pages must be
@@ -663,14 +690,11 @@ int xc_linux_save(int xc_handle, int io_
 
     max_pfn = live_shinfo->arch.max_pfn;
 
-    live_p2m_frame_list_list =
-        xc_map_foreign_range(xc_handle, dom, PAGE_SIZE, PROT_READ,
-                             live_shinfo->arch.pfn_to_mfn_frame_list_list);
-
-    if (!live_p2m_frame_list_list) {
-        ERROR("Couldn't map p2m_frame_list_list (errno %d)", errno);
-        goto out;
-    }
+    live_p2m_frame_list_list = map_frame_list_list(xc_handle, dom,
+                                                   live_shinfo);
+
+    if (!live_p2m_frame_list_list)
+        goto out;
 
     live_p2m_frame_list =
         xc_map_foreign_batch(xc_handle, dom, PROT_READ,
@@ -1169,8 +1193,14 @@ int xc_linux_save(int xc_handle, int io_
     ctxt.ctrlreg[3] = 
         xen_pfn_to_cr3(mfn_to_pfn(xen_cr3_to_pfn(ctxt.ctrlreg[3])));
 
+    /*
+     * Reset the MFN to be a known-invalid value. See map_frame_list_list().
+     */
+    memcpy(page, live_shinfo, PAGE_SIZE);
+    ((shared_info_t *)page)->arch.pfn_to_mfn_frame_list_list = 0;
+
     if (!write_exact(io_fd, &ctxt, sizeof(ctxt)) ||
-        !write_exact(io_fd, live_shinfo, PAGE_SIZE)) {
+        !write_exact(io_fd, page, PAGE_SIZE)) {
         ERROR("Error when writing to state file (1) (errno %d)", errno);
         goto out;
     }

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.