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

["Srikanth SM" <srikanth.sm@xxxxxxxxx>]

Hi,

I Came across this thread trying to understand live migration better.
I did some experiments and atleast on linux, I could do away with the
waiting for the arch.pfn_to_mfn_frame_list_list to get initialized
after a restore.  I believe it will work with other OSes as
well. I request your comments regarding this.

Please see if the below patch (against xen-unstable) can be used.

What I did was, simple-minded though: Instead of the guest building
p2m frame-list-list and the frame-list _after_ a restore, we build it from
dom0 _during_ restore.
- In save, canonicalise the pfn_to_mfn_frame_list_list mfn along
 with the entries in that page and store them in state file. (Note
 that we are anyway saving canonicalised p2m_frame_list pages)

- In restore, uncanonicalise pfn_to_mfn_frame_list_list pfn, and
 the entries of the page and copy it to appropriate place in guest memory.
 Also copy p2m_frame_list entries to appropriate place in guest memory.

However, what I do not know is the exact reason why this
initialization of pfn_to_mfn_frame_list_list and pfn_to_mfn_frame_list
is being done by the guest domain after unpausing
currently. Could you please clarify this?

Thanks and regards
Srikanth
Patch for inline viewing:


# HG changeset patch
# User srao@srikanth
# Node ID 6443c0465741c05ffef24f5ca7c7fd3907089d04
# Parent  8bc64a3a505413e4d3bd7def06539ef088f2ca0c
Avoid waiting for pfn-to-mfn-frame-list-list etc., to get initialized
when doing a save after restore.

Signed-off-by: Srikanth S. M. <srikanth_sm@xxxxxxxxxxxx>

diff -r 8bc64a3a5054 -r 6443c0465741
linux-2.6-xen-sparse/drivers/xen/core/machine_reboot.c
--- a/linux-2.6-xen-sparse/drivers/xen/core/machine_reboot.c    Mon Feb
5 16:40:19 2007
+++ b/linux-2.6-xen-sparse/drivers/xen/core/machine_reboot.c    Tue Feb
6 14:42:34 2007
@@ -87,10 +87,7 @@

static void post_suspend(int suspend_cancelled)
{
-       int i, j, k, fpp;
        extern unsigned long max_pfn;
-       extern unsigned long *pfn_to_mfn_frame_list_list;
-       extern unsigned long *pfn_to_mfn_frame_list[];

        if (suspend_cancelled) {
                xen_start_info->store_mfn =
@@ -105,20 +102,7 @@

        memset(empty_zero_page, 0, PAGE_SIZE);

-       fpp = PAGE_SIZE/sizeof(unsigned long);
-       for (i = 0, j = 0, k = -1; i < max_pfn; i += fpp, j++) {
-               if ((j % fpp) == 0) {
-                       k++;
-                       pfn_to_mfn_frame_list_list[k] =
-                               virt_to_mfn(pfn_to_mfn_frame_list[k]);
-                       j = 0;
-               }
-               pfn_to_mfn_frame_list[k][j] =
-                       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 -r 8bc64a3a5054 -r 6443c0465741 tools/libxc/xc_linux_restore.c
--- a/tools/libxc/xc_linux_restore.c    Mon Feb  5 16:40:19 2007
+++ b/tools/libxc/xc_linux_restore.c    Tue Feb  6 14:42:34 2007
@@ -172,6 +172,9 @@
    /* A copy of the pfn-to-mfn table frame list. */
    xen_pfn_t *p2m_frame_list = NULL;

+    /* A temporary mapping of pfn-to-mfn table frame list */
+    xen_pfn_t *live_p2m_fl = NULL;
+
    /* A temporary mapping of the guest's start_info page. */
    start_info_t *start_info;

@@ -816,11 +819,41 @@
    for ( i = 0; i < MAX_VIRT_CPUS; i++ )
        shared_info->vcpu_info[i].evtchn_pending_sel = 0;

-    /* Copy saved contents of shared-info page. No checking needed. */
+    /*
+     * Uncanonicalise fll pfn. Copy saved contents of shared-info page.
+     * No checking needed.
+     */
+    shared_info->arch.pfn_to_mfn_frame_list_list =
+       p2m[shared_info->arch.pfn_to_mfn_frame_list_list];
    page = xc_map_foreign_range(
        xc_handle, dom, PAGE_SIZE, PROT_WRITE, shared_info_frame);
    memcpy(page, shared_info, PAGE_SIZE);
    munmap(page, PAGE_SIZE);
+
+    /* Update fll page. Uncanonicalise entries in fll */
+    page = xc_map_foreign_range(xc_handle, dom, PAGE_SIZE, PROT_WRITE,
+                               shared_info->arch.pfn_to_mfn_frame_list_list);
+    if (page == NULL) {
+       ERROR("Cant map fll page");
+       goto out;
+    }
+    if (!read_exact(io_fd, page, PAGE_SIZE)) {
+       ERROR("Cant read fll page");
+       goto out;
+    }
+    for (i = 0; i < P2M_FLL_ENTRIES; i++) {
+       xen_pfn_t *p = page;
+       p[i] = p2m[p[i]];
+    }
+
+    /* Map pfn-to-mfn frame list pages */
+    live_p2m_fl = xc_map_foreign_batch(xc_handle, dom, PROT_WRITE,
+                                     page, P2M_FLL_ENTRIES);
+    munmap(page, PAGE_SIZE);
+    if (live_p2m_fl == NULL) {
+       ERROR("Cant map p2m frame-list pages");
+       goto out;
+    }

    /* Uncanonicalise the pfn-to-mfn table frame-number list. */
    for (i = 0; i < P2M_FL_ENTRIES; i++) {
@@ -832,6 +865,13 @@

        p2m_frame_list[i] = p2m[pfn];
    }
+
+    /*
+     * Copy the p2m_frame_list (local copy)
+     * to the live-p2m-frame-list
+     */
+    memcpy(live_p2m_fl, p2m_frame_list, P2M_FL_ENTRIES * sizeof (xen_pfn_t));
+    munmap(live_p2m_fl, P2M_FLL_ENTRIES * PAGE_SIZE);

    /* Copy the P2M we've constructed to the 'live' P2M */
    if (!(live_p2m = xc_map_foreign_batch(xc_handle, dom, PROT_WRITE,
diff -r 8bc64a3a5054 -r 6443c0465741 tools/libxc/xc_linux_save.c
--- a/tools/libxc/xc_linux_save.c       Mon Feb  5 16:40:19 2007
+++ b/tools/libxc/xc_linux_save.c       Tue Feb  6 14:42:34 2007
@@ -426,34 +426,6 @@
}

/*
-** 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
** converted into a 'canonical' form where references to actual mfns
** are replaced with references to the corresponding pfns.
@@ -616,7 +588,6 @@
}


-
int xc_linux_save(int xc_handle, int io_fd, uint32_t dom, uint32_t max_iters,
                  uint32_t max_factor, uint32_t flags, int (*suspend)(int))
{
@@ -714,12 +685,14 @@
    }

    max_pfn = live_shinfo->arch.max_pfn;
-
-    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_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 =
        xc_map_foreign_batch(xc_handle, dom, PROT_READ,
@@ -1225,14 +1198,32 @@
        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().
+     * Canonicalise fll mfn
     */
    memcpy(page, live_shinfo, PAGE_SIZE);
-    ((shared_info_t *)page)->arch.pfn_to_mfn_frame_list_list = 0;
+    if (!translate_mfn_to_pfn(
+            &((shared_info_t *)page)->arch.pfn_to_mfn_frame_list_list)) {
+        ERROR("Cant canonicalise fll mfn");
+        goto out;
+    }

    if (!write_exact(io_fd, &ctxt, sizeof(ctxt)) ||
        !write_exact(io_fd, page, PAGE_SIZE)) {
        ERROR("Error when writing to state file (1) (errno %d)", errno);
+        goto out;
+    }
+
+    /* canonicalise fll entries and send fll page */
+    memcpy(page, live_p2m_frame_list_list, PAGE_SIZE);
+    for (i = 0; i < P2M_FLL_ENTRIES; i++) {
+        if (!translate_mfn_to_pfn(&((xen_pfn_t *)page)[i])) {
+            ERROR("Cant canonicalise fll entry %d", i);
+            goto out;
+        }
+    }
+
+    if (!write_exact(io_fd, page, PAGE_SIZE)) {
+        ERROR("Cant write fll page to state file");
        goto out;
    }




> > 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

Attachment: save_restore.patch
Description: Text Data

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