xenpaging fixes for xen-unstable

PATCH In review Olaf Hering 4 participants 21 messages Jun 7, 2011 – Jun 10, 2011

Share & tools

Olaf Hering

author

2011-06-07 09:53 UTC #0 lore raw

PATCH 1/1 xenpaging fixes for xen-unstable

The following series of changes for xenpaging fixes a few bugs, does some cleanup and adds two new features to the pager.
It is now possible to run the pager manually and stop it with ctrl-c, without corrupting the guest. A shutdown of a guest is also catched with a xenstore watch.

Please review and apply.

Olaf

 tools/xenpaging/Makefile         |    2 
 tools/xenpaging/pagein.c         |   68 +++++++++++++++++++
 tools/xenpaging/policy_default.c |    4 +
 tools/xenpaging/watch.c          |   79 ++++++++++++++++++++++
 tools/xenpaging/xenpaging.c      |  138 ++++++++++++++++++++++-----------------
 tools/xenpaging/xenpaging.h      |    5 +
 xen/arch/x86/mm/p2m.c            |    1 
 7 files changed, 238 insertions(+), 59 deletions(-)

Olaf Hering

author

2011-06-07 09:53 UTC #1 lore raw

PATCH 1/1 xenpaging: correct dropping of pages to avoid full ring buffer

# HG changeset patch
# User Olaf Hering <[email protected]>
# Date 1307437209 -7200
# Node ID 6b8446bf4e5fbfa93169ec2509364c0fde74beca
# Parent c231a26a29327aa3c737170e04c738289be2d309
xenpaging: correct dropping of pages to avoid full ring buffer

Doing a one-way channel from Xen to xenpaging is not possible with the current ring buffer implementation. xenpaging uses the mem_event ring buffer, which expects request/response pairs to make progress. The previous patch, which tried to establish a one-way communication from Xen to xenpaging, stalled the guest once the buffer was filled up with requests. Correct page-dropping by taking the slow path and let p2m_mem_paging_resume() consume the response from xenpaging. This makes room for yet another request/response pair and avoids hanging guests.

Signed-off-by Olaf Hering <[email protected]>

tools/xenpaging/xenpaging.c +11 ~~-11~~

diff -r c231a26a2932 -r 6b8446bf4e5f tools/xenpaging/xenpaging.c
@@ -653,19 +653,19 @@ int main(int argc, char *argv[])
                         ERROR("Error populating page");
                         goto out;
                     }
+                }
 
-                    /* Prepare the response */
-                    rsp.gfn = req.gfn;
-                    rsp.p2mt = req.p2mt;
-                    rsp.vcpu_id = req.vcpu_id;
-                    rsp.flags = req.flags;
+                /* Prepare the response */
+                rsp.gfn = req.gfn;
+                rsp.p2mt = req.p2mt;
+                rsp.vcpu_id = req.vcpu_id;
+                rsp.flags = req.flags;
 
-                    rc = xenpaging_resume_page(paging, &rsp, 1);
-                    if ( rc != 0 )
-                    {
-                        ERROR("Error resuming page");
-                        goto out;
-                    }
+                rc = xenpaging_resume_page(paging, &rsp, 1);
+                if ( rc != 0 )
+                {
+                    ERROR("Error resuming page");
+                    goto out;
                 }
 
                 /* Evict a new page to replace the one we just paged in */

Olaf Hering

author

2011-06-07 09:53 UTC #2 lore raw

PATCH 1/1 xenpaging: do not bounce p2mt to xenpaging

In reply to Olaf Hering

# HG changeset patch
# User Olaf Hering <[email protected]>
# Date 1307437210 -7200
# Node ID d22489c1b10a7fd17fca635d0a29dc774cf97bf3 # Parent 6b8446bf4e5fbfa93169ec2509364c0fde74beca
xenpaging: do not bounce p2mt to xenpaging

do not bounce p2mt to xenpaging because p2m_mem_paging_populate() and p2m_mem_paging_resume() dont make use of p2mt

Signed-off-by Olaf Hering <[email protected]>

tools/xenpaging/xenpaging.c +0 -4

diff -r 6b8446bf4e5f -r d22489c1b10a tools/xenpaging/xenpaging.c
@@ -657,7 +657,6 @@ int main(int argc, char *argv[])
 
                 /* Prepare the response */
                 rsp.gfn = req.gfn;
-                rsp.p2mt = req.p2mt;
                 rsp.vcpu_id = req.vcpu_id;
                 rsp.flags = req.flags;

@@ -674,10 +673,8 @@ int main(int argc, char *argv[])
             else
             {
                 DPRINTF("page already populated (domain = %d; vcpu = %d;"
-                        " p2mt = %x;"
                         " gfn = %"PRIx64"; paused = %d)\n",
                         paging->mem_event.domain_id, req.vcpu_id,
-                        req.p2mt,
                         req.gfn, req.flags & MEM_EVENT_FLAG_VCPU_PAUSED);
 
                 /* Tell Xen to resume the vcpu */

@@ -686,7 +683,6 @@ int main(int argc, char *argv[])
                 {
                     /* Prepare the response */
                     rsp.gfn = req.gfn;
-                    rsp.p2mt = req.p2mt;
                     rsp.vcpu_id = req.vcpu_id;
                     rsp.flags = req.flags;

xen/arch/x86/mm/p2m.c +0 -1

diff -r 6b8446bf4e5f -r d22489c1b10a xen/arch/x86/mm/p2m.c
@@ -773,7 +773,6 @@ void p2m_mem_paging_populate(struct doma
 
     /* Send request to pager */
     req.gfn = gfn;
-    req.p2mt = p2mt;
     req.vcpu_id = v->vcpu_id;
 
     mem_event_put_request(d, &req);

Olaf Hering

author

2011-06-07 09:53 UTC #3 lore raw

PATCH 1/1 xenpaging: remove srand call

In reply to Olaf Hering

# HG changeset patch
# User Olaf Hering <[email protected]>
# Date 1307437211 -7200
# Node ID e2480aad3a3bd4e801bd79daf1b6ee77efe1ed7c # Parent d22489c1b10a7fd17fca635d0a29dc774cf97bf3
xenpaging: remove srand call

The policy uses now a linear algorithm instead of a random one. Remove the call to srand().

Signed-off-by Olaf Hering <[email protected]>

tools/xenpaging/xenpaging.c +0 -3

diff -r d22489c1b10a -r e2480aad3a3b tools/xenpaging/xenpaging.c
@@ -544,9 +544,6 @@ int main(int argc, char *argv[])
     domain_id = atoi(argv[1]);
     num_pages = atoi(argv[2]);
 
-    /* Seed random-number generator */
-    srand(time(NULL));
-
     /* Initialise domain paging */
     paging = xenpaging_init(domain_id);
     if ( paging == NULL )

Olaf Hering

author

2011-06-07 09:53 UTC #4 lore raw

PATCH 1/1 xenpaging: remove return values from functions that can not fail

In reply to Olaf Hering

# HG changeset patch
# User Olaf Hering <[email protected]>
# Date 1307437212 -7200
# Node ID c31636354b69164c06e86feed58f9713656b8f0c
# Parent e2480aad3a3bd4e801bd79daf1b6ee77efe1ed7c
xenpaging: remove return values from functions that can not fail

get_request() and put_response() can not fail, remove return value and update calling functions.

Signed-off-by Olaf Hering <[email protected]>

tools/xenpaging/xenpaging.c +4 ~~-15~~

diff -r e2480aad3a3b -r c31636354b69 tools/xenpaging/xenpaging.c
@@ -297,7 +297,7 @@ static int xenpaging_teardown(xenpaging_
     return -1;
 }
 
-static int get_request(mem_event_t *mem_event, mem_event_request_t *req)
+static void get_request(mem_event_t *mem_event, mem_event_request_t *req)
 {
     mem_event_back_ring_t *back_ring;
     RING_IDX req_cons;

@@ -316,11 +316,9 @@ static int get_request(mem_event_t *mem_
     back_ring->sring->req_event = req_cons + 1;
 
     mem_event_ring_unlock(mem_event);
-
-    return 0;
 }
 
-static int put_response(mem_event_t *mem_event, mem_event_response_t *rsp)
+static void put_response(mem_event_t *mem_event, mem_event_response_t *rsp)
 {
     mem_event_back_ring_t *back_ring;
     RING_IDX rsp_prod;

@@ -339,8 +337,6 @@ static int put_response(mem_event_t *mem
     RING_PUSH_RESPONSES(back_ring);
 
     mem_event_ring_unlock(mem_event);
-
-    return 0;
 }
 
 static int xenpaging_evict_page(xenpaging_t *paging,

@@ -400,9 +396,7 @@ static int xenpaging_resume_page(xenpagi
     int ret;
 
     /* Put the page info on the ring */
-    ret = put_response(&paging->mem_event, rsp);
-    if ( ret != 0 )
-        goto out;
+    put_response(&paging->mem_event, rsp);
 
     /* Notify policy of page being paged in */
     if ( notify_policy )

@@ -612,12 +606,7 @@ int main(int argc, char *argv[])
 
         while ( RING_HAS_UNCONSUMED_REQUESTS(&paging->mem_event.back_ring) )
         {
-            rc = get_request(&paging->mem_event, &req);
-            if ( rc != 0 )
-            {
-                ERROR("Error getting request");
-                goto out;
-            }
+            get_request(&paging->mem_event, &req);
 
             /* Check if the page has already been paged in */
             if ( test_and_clear_bit(req.gfn, paging->bitmap) )

Olaf Hering

author

2011-06-07 09:53 UTC #5 lore raw

PATCH 1/1 xenpaging: catch xc_mem_paging_resume errors

In reply to Olaf Hering

# HG changeset patch
# User Olaf Hering <[email protected]>
# Date 1307437214 -7200
# Node ID 8eb07742f96854303ad7793f9319cd314dcf7be3 # Parent c31636354b69164c06e86feed58f9713656b8f0c
xenpaging: catch xc_mem_paging_resume errors

In the unlikely event that xc_mem_paging_resume() fails, do not overwrite the error with the return value from xc_evtchn_notify()

Signed-off-by Olaf Hering <[email protected]>

tools/xenpaging/xenpaging.c +3 -2

diff -r c31636354b69 -r 8eb07742f968 tools/xenpaging/xenpaging.c
@@ -405,8 +405,9 @@ static int xenpaging_resume_page(xenpagi
     /* Tell Xen page is ready */
     ret = xc_mem_paging_resume(paging->xc_handle, paging->mem_event.domain_id,
                                rsp->gfn);
-    ret = xc_evtchn_notify(paging->mem_event.xce_handle,
-                           paging->mem_event.port);
+    if ( ret == 0 ) 
+        ret = xc_evtchn_notify(paging->mem_event.xce_handle,
+                               paging->mem_event.port);
 
  out:
     return ret;

Olaf Hering

author

2011-06-07 09:53 UTC #6 lore raw

PATCH 1/1 xenpaging: remove local domain_id variable

In reply to Olaf Hering

# HG changeset patch
# User Olaf Hering <[email protected]>
# Date 1307437215 -7200
# Node ID 46c9f04365804c08a454b82e76119c43df4d7fa0 # Parent 8eb07742f96854303ad7793f9319cd314dcf7be3
xenpaging: remove local domain_id variable

Remove the local domain_id variable, it is already fetched from paging->mem_event in other places.
Update the sprintf format string to use unsigned argument.

Signed-off-by Olaf Hering <[email protected]>

tools/xenpaging/xenpaging.c +3 -5

diff -r 8eb07742f968 -r 46c9f0436580 tools/xenpaging/xenpaging.c
@@ -515,7 +515,6 @@ static int evict_victim(xenpaging_t *pag
 int main(int argc, char *argv[])
 {
     struct sigaction act;
-    domid_t domain_id;
     int num_pages;
     xenpaging_t *paging;
     xenpaging_victim_t *victims;

@@ -536,11 +535,10 @@ int main(int argc, char *argv[])
         return -1;
     }
 
-    domain_id = atoi(argv[1]);
     num_pages = atoi(argv[2]);
 
     /* Initialise domain paging */
-    paging = xenpaging_init(domain_id);
+    paging = xenpaging_init(atoi(argv[1]));
     if ( paging == NULL )
     {
         fprintf(stderr, "Error initialising paging");

@@ -548,10 +546,10 @@ int main(int argc, char *argv[])
     }
     xch = paging->xc_handle;
 
-    DPRINTF("starting %s %u %d\n", argv[0], domain_id, num_pages);
+    DPRINTF("starting %s %u %d\n", argv[0], paging->mem_event.domain_id, num_pages);
 
     /* Open file */
-    sprintf(filename, "page_cache_%d", domain_id);
+    sprintf(filename, "page_cache_%u", paging->mem_event.domain_id);
     fd = open(filename, open_flags, open_mode);
     if ( fd < 0 )
     {

Olaf Hering

author

2011-06-07 09:53 UTC #7 lore raw

PATCH 1/1 xenpaging: move num_pages into xenpaging struct

In reply to Olaf Hering

# HG changeset patch
# User Olaf Hering <[email protected]>
# Date 1307437216 -7200
# Node ID 5aae4431bfc0970fd48f856fc4ede547ed4860f3 # Parent 46c9f04365804c08a454b82e76119c43df4d7fa0
xenpaging: move num_pages into xenpaging struct

Move num_pages into struct xenpaging.
num_pages will be used by the policy in a subsequent patch. Also remove a memset, the victims array is allocated with calloc.

Signed-off-by Olaf Hering <[email protected]>

tools/xenpaging/xenpaging.c +14 ~~-16~~

diff -r 46c9f0436580 -r 5aae4431bfc0 tools/xenpaging/xenpaging.c
@@ -73,7 +73,7 @@ static void *init_page(void)
     return NULL;
 }
 
-static xenpaging_t *xenpaging_init(domid_t domain_id)
+static xenpaging_t *xenpaging_init(domid_t domain_id, int num_pages)
 {
     xenpaging_t *paging;
     xc_interface *xch;

@@ -215,6 +215,13 @@ static xenpaging_t *xenpaging_init(domid
     }
     DPRINTF("max_pages = %"PRIx64"\n", paging->domain_info->max_pages);
 
+    if ( num_pages < 0 || num_pages > paging->domain_info->max_pages )
+    {
+        num_pages = paging->domain_info->max_pages;
+        DPRINTF("setting num_pages to %d\n", num_pages);
+    }
+    paging->num_pages = num_pages;
+
     /* Initialise policy */
     rc = policy_init(paging);
     if ( rc != 0 )

@@ -515,7 +522,6 @@ static int evict_victim(xenpaging_t *pag
 int main(int argc, char *argv[])
 {
     struct sigaction act;
-    int num_pages;
     xenpaging_t *paging;
     xenpaging_victim_t *victims;
     mem_event_request_t req;

@@ -535,10 +541,8 @@ int main(int argc, char *argv[])
         return -1;
     }
 
-    num_pages = atoi(argv[2]);
-
     /* Initialise domain paging */
-    paging = xenpaging_init(atoi(argv[1]));
+    paging = xenpaging_init(atoi(argv[1]), atoi(argv[2]));
     if ( paging == NULL )
     {
         fprintf(stderr, "Error initialising paging");

@@ -546,7 +550,7 @@ int main(int argc, char *argv[])
     }
     xch = paging->xc_handle;
 
-    DPRINTF("starting %s %u %d\n", argv[0], paging->mem_event.domain_id, num_pages);
+    DPRINTF("starting %s %u %d\n", argv[0], paging->mem_event.domain_id, paging->num_pages);
 
     /* Open file */
     sprintf(filename, "page_cache_%u", paging->mem_event.domain_id);

@@ -557,12 +561,7 @@ int main(int argc, char *argv[])
         return 2;
     }
 
-    if ( num_pages < 0 || num_pages > paging->domain_info->max_pages )
-    {
-        num_pages = paging->domain_info->max_pages;
-        DPRINTF("setting num_pages to %d\n", num_pages);
-    }
-    victims = calloc(num_pages, sizeof(xenpaging_victim_t));
+    victims = calloc(paging->num_pages, sizeof(xenpaging_victim_t));
 
     /* ensure that if we get a signal, we'll do cleanup, then exit */
     act.sa_handler = close_handler;

@@ -574,8 +573,7 @@ int main(int argc, char *argv[])
     sigaction(SIGALRM, &act, NULL);
 
     /* Evict pages */
-    memset(victims, 0, sizeof(xenpaging_victim_t) * num_pages);
-    for ( i = 0; i < num_pages; i++ )
+    for ( i = 0; i < paging->num_pages; i++ )
     {
         rc = evict_victim(paging, &victims[i], fd, i);
         if ( rc == -ENOSPC )

@@ -611,13 +609,13 @@ int main(int argc, char *argv[])
             if ( test_and_clear_bit(req.gfn, paging->bitmap) )
             {
                 /* Find where in the paging file to read from */
-                for ( i = 0; i < num_pages; i++ )
+                for ( i = 0; i < paging->num_pages; i++ )
                 {
                     if ( victims[i].gfn == req.gfn )
                         break;
                 }
     
-                if ( i >= num_pages )
+                if ( i >= paging->num_pages )
                 {
                     DPRINTF("Couldn't find page %"PRIx64"\n", req.gfn);
                     goto out;

tools/xenpaging/xenpaging.h +1 -0

diff -r 46c9f0436580 -r 5aae4431bfc0 tools/xenpaging/xenpaging.h
@@ -45,6 +45,7 @@ typedef struct xenpaging {
     unsigned long *bitmap;
 
     mem_event_t mem_event;
+    int num_pages;
     int policy_mru_size;
 } xenpaging_t;

Olaf Hering

author

2011-06-07 09:53 UTC #8 lore raw

PATCH 1/1 xenpaging: start paging in the middle of gfn range

In reply to Olaf Hering

# HG changeset patch
# User Olaf Hering <[email protected]>
# Date 1307437217 -7200
# Node ID f5203dded3b13a33716ae50ef12d766c394f81f5 # Parent 5aae4431bfc0970fd48f856fc4ede547ed4860f3
xenpaging: start paging in the middle of gfn range

Set the starting gfn to somewhere in the middle of the gfn range to avoid paging during BIOS startup. This can speedup booting of a guest.

Signed-off-by Olaf Hering <[email protected]>

tools/xenpaging/policy_default.c +4 -0

diff -r 5aae4431bfc0 -r f5203dded3b1 tools/xenpaging/policy_default.c
@@ -76,6 +76,10 @@ int policy_init(xenpaging_t *paging)
     /* Don't page out page 0 */
     set_bit(0, bitmap);
 
+    /* Start in the middle to avoid paging during BIOS startup */
+    current_gfn = max_pages / 2;
+    current_gfn -= paging->num_pages / 2;
+
  out:
     return rc;
 }

Olaf Hering

author

2011-06-07 09:53 UTC #9 lore raw

PATCH 1/1 xenpaging: pass integer to xenpaging_populate_page

In reply to Olaf Hering

# HG changeset patch
# User Olaf Hering <[email protected]>
# Date 1307437218 -7200
# Node ID 48244bc8156ff8eca82ce9ab811516aae3d3fe32 # Parent f5203dded3b13a33716ae50ef12d766c394f81f5
xenpaging: pass integer to xenpaging_populate_page

Pass gfn as integer to xenpaging_populate_page(). xc_map_foreign_pages() takes a pointer to a list of gfns, but its a const pointer. So writing the value back to the caller is not needed.

Signed-off-by Olaf Hering <[email protected]>

tools/xenpaging/xenpaging.c +6 ~~-10~~

diff -r f5203dded3b1 -r 48244bc8156f tools/xenpaging/xenpaging.c
@@ -421,27 +421,24 @@ static int xenpaging_resume_page(xenpagi
 }
 
 static int xenpaging_populate_page(xenpaging_t *paging,
-    uint64_t *gfn, int fd, int i)
+    xen_pfn_t gfn, int fd, int i)
 {
     xc_interface *xch = paging->xc_handle;
-    unsigned long _gfn;
     void *page;
     int ret;
     unsigned char oom = 0;
 
-    _gfn = *gfn;
-    DPRINTF("populate_page < gfn %lx pageslot %d\n", _gfn, i);
+    DPRINTF("populate_page < gfn %"PRI_xen_pfn" pageslot %d\n", gfn, i);
     do
     {
         /* Tell Xen to allocate a page for the domain */
-        ret = xc_mem_paging_prep(xch, paging->mem_event.domain_id,
-                                 _gfn);
+        ret = xc_mem_paging_prep(xch, paging->mem_event.domain_id, gfn);
         if ( ret != 0 )
         {
             if ( errno == ENOMEM )
             {
                 if ( oom++ == 0 )
-                    DPRINTF("ENOMEM while preparing gfn %lx\n", _gfn);
+                    DPRINTF("ENOMEM while preparing gfn %"PRI_xen_pfn"\n", gfn);
                 sleep(1);
                 continue;
             }

@@ -454,8 +451,7 @@ static int xenpaging_populate_page(xenpa
     /* Map page */
     ret = -EFAULT;
     page = xc_map_foreign_pages(xch, paging->mem_event.domain_id,
-                                PROT_READ | PROT_WRITE, &_gfn, 1);
-    *gfn = _gfn;
+                                PROT_READ | PROT_WRITE, &gfn, 1);
     if ( page == NULL )
     {
         ERROR("Error mapping page: page is null");

@@ -630,7 +626,7 @@ int main(int argc, char *argv[])
                 else
                 {
                     /* Populate the page */
-                    rc = xenpaging_populate_page(paging, &req.gfn, fd, i);
+                    rc = xenpaging_populate_page(paging, req.gfn, fd, i);
                     if ( rc != 0 )
                     {
                         ERROR("Error populating page");

Olaf Hering

author

2011-06-07 09:53 UTC #10 lore raw

PATCH 1/1 xenpaging: add helper function for unlinking pagefile

In reply to Olaf Hering

# HG changeset patch
# User Olaf Hering <[email protected]>
# Date 1307437220 -7200
# Node ID fb445bd61233844f4b9d42b6eca172670f75eb98 # Parent 48244bc8156ff8eca82ce9ab811516aae3d3fe32
xenpaging: add helper function for unlinking pagefile

Unlink pagefile in the signal handler and also in the exit path. This does not leave a stale pagefile if an error occoured.

Signed-off-by Olaf Hering <[email protected]>

tools/xenpaging/xenpaging.c +12 -2

diff -r 48244bc8156f -r fb445bd61233 tools/xenpaging/xenpaging.c
@@ -41,11 +41,20 @@
 
 static char filename[80];
 static int interrupted;
+
+static void unlink_pagefile(void)
+{
+    if ( filename[0] )
+    {
+        unlink(filename);
+        filename[0] = '\0';
+    }
+}
+
 static void close_handler(int sig)
 {
     interrupted = sig;
-    if ( filename[0] )
-        unlink(filename);
+    unlink_pagefile();
 }
 
 static void *init_page(void)

@@ -679,6 +688,7 @@ int main(int argc, char *argv[])
 
  out:
     close(fd);
+    unlink_pagefile();
     free(victims);
 
     /* Tear down domain paging */

Olaf Hering

author

2011-06-07 09:53 UTC #11 lore raw

PATCH 1/1 xenpaging: add watch thread to catch guest shutdown

In reply to Olaf Hering

# HG changeset patch
# User Olaf Hering <[email protected]>
# Date 1307437221 -7200
# Node ID 9484d35ec6e802aa3727cb6332e649b41e80a615 # Parent fb445bd61233844f4b9d42b6eca172670f75eb98
xenpaging: add watch thread to catch guest shutdown

If xenpaging is started manually then no event is sent to xenpaging when the guest is shutdown or rebooted. Add a watch on the shutdown node to leave the loop and gracefully shutdown the pager.

Signed-off-by Olaf Hering <[email protected]>

tools/xenpaging/Makefile +1 -0

diff -r fb445bd61233 -r 9484d35ec6e8 tools/xenpaging/Makefile
@@ -8,6 +8,7 @@ POLICY    = default
 
 SRC      :=
 SRCS     += file_ops.c xc.c xenpaging.c policy_$(POLICY).c
+SRCS     += watch.c
 
 CFLAGS   += -Werror
 CFLAGS   += -Wno-unused

tools/xenpaging/watch.c new file +79 -0

79 lines not on this page · raw message

tools/xenpaging/xenpaging.c +8 -0

diff -r fb445bd61233 -r 9484d35ec6e8 tools/xenpaging/xenpaging.c
@@ -57,6 +57,11 @@ static void close_handler(int sig)
     unlink_pagefile();
 }
 
+static void set_interrupted_quit(void)
+{
+    interrupted = SIGQUIT;
+}
+
 static void *init_page(void)
 {
     void *buffer;

@@ -577,6 +582,9 @@ int main(int argc, char *argv[])
     sigaction(SIGINT,  &act, NULL);
     sigaction(SIGALRM, &act, NULL);
 
+    /* watch for shutdown of domain_id */
+    create_watch_thread(paging->mem_event.domain_id, set_interrupted_quit);
+
     /* Evict pages */
     for ( i = 0; i < paging->num_pages; i++ )
     {

tools/xenpaging/xenpaging.h +1 -0

diff -r fb445bd61233 -r 9484d35ec6e8 tools/xenpaging/xenpaging.h
@@ -55,6 +55,7 @@ typedef struct xenpaging_victim {
     unsigned long gfn;
 } xenpaging_victim_t;
 
+extern void create_watch_thread(domid_t domain_id, void (*fn)(void));
 
 #endif // __XEN_PAGING_H__

Ian Campbell

2011-06-07 10:23 UTC #12 lore raw

Re: [PATCH 11 of 12] xenpaging: add watch thread to catch guest shutdown

Quoted 7 lines · Olaf Hering

# HG changeset patch
# User Olaf Hering <[email protected]>
# Date 1307437221 -7200
# Node ID 9484d35ec6e802aa3727cb6332e649b41e80a615 # Parent fb445bd61233844f4b9d42b6eca172670f75eb98
xenpaging: add watch thread to catch guest shutdown

Quoted 5 lines

If xenpaging is started manually then no event is sent to xenpaging when the guest is shutdown or rebooted. Add a watch on the shutdown node to leave the loop and gracefully shutdown the pager.
Signed-off-by Olaf Hering <[email protected]>

There are ways for a domain to shutdown which do not involve the shutdown node.

The correct way to watch for domain shutdown is to watch the special "@releaseDomain" node. IIRC this notifies you when _any_ domain has shutdown so you need to check for the domain you are actually interested in. Doing this also removes the need for the hacky extra xs_read_watch which you have -- which seems very racy to me and is almost certainly incorrect.

Lastly I don't think you need a new thread for this, you can integrate the xs fd (from xs_fileno()) into your existing poll loop from xc_wait_for_event_or_timeout (which is a terrible name for a function which isn't in libxc, this should either be moved into the library or renamed depending on it's actual use cases. Same for the other xc_* in xenpaging...)

Ian.

Quoted 135 lines

diff -r fb445bd61233 -r 9484d35ec6e8 tools/xenpaging/Makefile

tools/xenpaging/Makefile +1 -0

@@ -8,6 +8,7 @@ POLICY    = default
 
 SRC      :=
 SRCS     += file_ops.c xc.c xenpaging.c policy_$(POLICY).c
+SRCS     += watch.c
 
 CFLAGS   += -Werror
 CFLAGS   += -Wno-unused

diff -r fb445bd61233 -r 9484d35ec6e8 tools/xenpaging/watch.c

tools/xenpaging/watch.c new file +79 -0

@@ -0,0 +1,79 @@
+/* watch for guest shutdown in case xenpaging is started manually */
+#include <unistd.h>
+#include <pthread.h>
+#include <string.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <xs.h>
+#include <xen/xen.h>
+
+struct watch_args {
+    domid_t domain_id;
+    void (*fn)(void);
+};
+
+static pthread_t watch_thread;
+static struct watch_args watch_args;
+static const char shutdown[] = "/control/shutdown";
+
+static void *watch_domain(void *arg)
+{
+    struct watch_args *wa = arg;
+    struct xs_handle *xs;
+    char *dom_path, *path, **vec;
+    unsigned int num;
+    size_t malloc_len;
+    bool ret;
+
+    xs = xs_daemon_open_readonly();
+    if ( xs == NULL )
+        goto exit;
+
+    dom_path = xs_get_domain_path(xs, wa->domain_id);
+    if ( dom_path == NULL )
+        goto close_exit;
+
+    malloc_len = strlen(dom_path) + strlen(shutdown) + 1;
+    path = malloc(malloc_len);
+    if ( path == NULL )
+        goto close_exit;
+
+    snprintf(path, malloc_len, "%s%s", dom_path, shutdown);
+
+    ret = xs_watch(xs, path, "");
+    if ( ret == true )
+    {
+        /* first watch fires right away */
+        vec = xs_read_watch(xs, &num);
+        free(vec);
+        /* wait for real event */
+        vec = xs_read_watch(xs, &num);
+        free(vec);
+        xs_unwatch(xs, path, "");
+    
+        /* notify pager */
+        wa->fn();
+    }
+
+    free(path);
+close_exit:
+    xs_daemon_close(xs);
+exit:
+    pthread_exit(NULL);
+}
+
+void create_watch_thread(domid_t domain_id, void (*fn)(void))
+{
+    watch_args.domain_id = domain_id;
+    watch_args.fn = fn;
+    pthread_create(&watch_thread, NULL, watch_domain, &watch_args);
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-set-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End: 
+ */

diff -r fb445bd61233 -r 9484d35ec6e8 tools/xenpaging/xenpaging.c

tools/xenpaging/xenpaging.c +8 -0

@@ -57,6 +57,11 @@ static void close_handler(int sig)
     unlink_pagefile();
 }
 
+static void set_interrupted_quit(void)
+{
+    interrupted = SIGQUIT;
+}
+
 static void *init_page(void)
 {
     void *buffer;

@@ -577,6 +582,9 @@ int main(int argc, char *argv[])
     sigaction(SIGINT,  &act, NULL);
     sigaction(SIGALRM, &act, NULL);
 
+    /* watch for shutdown of domain_id */
+    create_watch_thread(paging->mem_event.domain_id, set_interrupted_quit);
+
     /* Evict pages */
     for ( i = 0; i < paging->num_pages; i++ )
     {

diff -r fb445bd61233 -r 9484d35ec6e8 tools/xenpaging/xenpaging.h

tools/xenpaging/xenpaging.h +1 -0

@@ -55,6 +55,7 @@ typedef struct xenpaging_victim {
     unsigned long gfn;
 } xenpaging_victim_t;
 
+extern void create_watch_thread(domid_t domain_id, void (*fn)(void));
 
 #endif // __XEN_PAGING_H__

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Olaf Hering

author

2011-06-07 19:07 UTC #13 lore raw

Re: [PATCH 11 of 12] xenpaging: add watch thread to catch guest shutdown

Quoted 6 lines · Ian Campbell

The correct way to watch for domain shutdown is to watch the special "@releaseDomain" node. IIRC this notifies you when _any_ domain has shutdown so you need to check for the domain you are actually interested in. Doing this also removes the need for the hacky extra xs_read_watch which you have -- which seems very racy to me and is almost certainly incorrect.

Thanks for the feedback, I have updated my patch to use this interface.

Quoted 6 lines

Lastly I don't think you need a new thread for this, you can integrate the xs fd (from xs_fileno()) into your existing poll loop from xc_wait_for_event_or_timeout (which is a terrible name for a function which isn't in libxc, this should either be moved into the library or renamed depending on it's actual use cases. Same for the other xc_* in xenpaging...)

All of xc.c can be removed. If patches 1-10 are ok, can you apply them?

I will send another series to remove xc.c, which will also contain the changes from patch 11 and 12.

Olaf

Olaf Hering

author

2011-06-07 09:53 UTC #14 lore raw

PATCH 1/1 xenpaging: implement stopping of pager by sending SIGTERM/SIGINT

In reply to Olaf Hering

# HG changeset patch
# User Olaf Hering <[email protected]>
# Date 1307437222 -7200
# Node ID 168eef309cc99a5bebed230a54f337b6692c112d
# Parent 9484d35ec6e802aa3727cb6332e649b41e80a615
xenpaging: implement stopping of pager by sending SIGTERM/SIGINT

Write all paged-out pages back into the guest if the pager is interrupted by ctrl-c or if it receives SIGTERM.

Signed-off-by Olaf Hering <[email protected]>

tools/xenpaging/Makefile +1 -0

diff -r 9484d35ec6e8 -r 168eef309cc9 tools/xenpaging/Makefile
@@ -9,6 +9,7 @@ POLICY    = default
 SRC      :=
 SRCS     += file_ops.c xc.c xenpaging.c policy_$(POLICY).c
 SRCS     += watch.c
+SRCS     += pagein.c
 
 CFLAGS   += -Werror
 CFLAGS   += -Wno-unused

tools/xenpaging/pagein.c new file +68 -0

68 lines not on this page · raw message

tools/xenpaging/xenpaging.c +32 -3

diff -r 9484d35ec6e8 -r 168eef309cc9 tools/xenpaging/xenpaging.c
@@ -585,6 +585,9 @@ int main(int argc, char *argv[])
     /* watch for shutdown of domain_id */
     create_watch_thread(paging->mem_event.domain_id, set_interrupted_quit);
 
+    /* listen for page-in events to stop pager */
+    create_page_in_thread(paging->mem_event.domain_id, xch);
+
     /* Evict pages */
     for ( i = 0; i < paging->num_pages; i++ )
     {

@@ -600,7 +603,7 @@ int main(int argc, char *argv[])
     DPRINTF("%d pages evicted. Done.\n", i);
 
     /* Swap pages in and out */
-    while ( !interrupted )
+    while ( 1 )
     {
         /* Wait for Xen to signal that a page needs paged in */
         rc = xc_wait_for_event_or_timeout(xch, paging->mem_event.xce_handle, 100);

@@ -663,8 +666,12 @@ int main(int argc, char *argv[])
                     goto out;
                 }
 
-                /* Evict a new page to replace the one we just paged in */
-                evict_victim(paging, &victims[i], fd, i);
+                /* Evict a new page to replace the one we just paged in,
+                 * or clear this pagefile slot on exit */
+                if ( interrupted )
+                    victims[i].gfn = INVALID_MFN;
+                else
+                    evict_victim(paging, &victims[i], fd, i);
             }
             else
             {

@@ -691,6 +698,28 @@ int main(int argc, char *argv[])
                 }
             }
         }
+
+        /* Write all pages back into the guest */
+        if ( interrupted == SIGTERM || interrupted == SIGINT )
+        {
+            for ( i = 0; i < paging->domain_info->max_pages; i++ )
+            {
+                if ( test_bit(i, paging->bitmap) )
+                {
+                    page_in_trigger(i);
+                    break;
+                }
+            }
+            /* If no more pages to process, exit loop */
+            if ( i == paging->domain_info->max_pages )
+                break;
+        }
+        else
+        {
+            /* Exit on any other signal */
+            if ( interrupted )
+                break;
+        }
     }
     DPRINTF("xenpaging got signal %d\n", interrupted);

tools/xenpaging/xenpaging.h +3 -0

diff -r 9484d35ec6e8 -r 168eef309cc9 tools/xenpaging/xenpaging.h
@@ -57,6 +57,9 @@ typedef struct xenpaging_victim {
 
 extern void create_watch_thread(domid_t domain_id, void (*fn)(void));
 
+extern void create_page_in_thread(domid_t domain_id, xc_interface *xch);
+extern void page_in_trigger(unsigned long gfn);
+
 #endif // __XEN_PAGING_H__

Keir Fraser

2011-06-07 12:27 UTC #15 lore raw

In reply to Olaf Hering

Quoted 7 lines · Olaf Hering

The following series of changes for xenpaging fixes a few bugs, does some cleanup and adds two new features to the pager.
It is now possible to run the pager manually and stop it with ctrl-c, without corrupting the guest. A shutdown of a guest is also catched with a xenstore watch.
Please review and apply.

I'll leave these patches for a tools maintainer to apply.

Once they're in, what is left to do? Do you need help with the in-Xen waitqueue stuff? Do you have test cases that fail, a private patch queue, etc?

-- Keir

Quoted 17 lines

Olaf

 tools/xenpaging/Makefile         |    2
 tools/xenpaging/pagein.c         |   68 +++++++++++++++++++
 tools/xenpaging/policy_default.c |    4 +
 tools/xenpaging/watch.c          |   79 ++++++++++++++++++++++
 tools/xenpaging/xenpaging.c      |  138
++++++++++++++++++++++-----------------
 tools/xenpaging/xenpaging.h      |    5 +
 xen/arch/x86/mm/p2m.c            |    1
 7 files changed, 238 insertions(+), 59 deletions(-)

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Olaf Hering

author

2011-06-07 13:38 UTC #16 lore raw

Quoted 3 lines · Keir Fraser

Once they're in, what is left to do? Do you need help with the in-Xen waitqueue stuff? Do you have test cases that fail, a private patch queue, etc?

Keir,

I have no new patches, just returned to xenpaging end of last week. A few months ago I collected my TODO list below.

One thing that I would need help with is the machine_to_phys_mapping[] handling, an attempt to fix it was reverted before the 4.1 release. See the thread in the URL below.

My testcase was a SLES11SP1 pv-on-hvm guest, rebooted in an endless loop. That catched all of the issues. With the waitqueue/HVMCOPY_gfn_paged_out issue its best to customize the xenpaging policy to page the range of gfns were the guest pagetables are stored.

I will send a series of patches to try later.

Olaf

Todo:
- implement xl support
- implement stopping of xenpaging
- implement live migration
- implement config option for XENPAGING_DEBUG and XENPAGING_POLICY_MRU_SIZE
- implement config option for xenpaging_dir
- implement better starting gfn in xenpaging policy
an initial gfn number in the middle of the gfn range may avoid page-ins during BIOS startup
- fix machine_to_phys_mapping[] array handling during page deallocation the gfn of a released page must be maintained properly in the array
http://lists.xensource.com/archives/html/xen-devel/2011-01/msg00824.html
- fix HVMCOPY_gfn_paged_out handling
some callers of __hvm_copy() do not handle HVMCOPY_gfn_paged_out, such as hypercalls and the MMIO emulation
the recently added waitqueue feature in Xen 4.1 should be used
- remove all retry code from gfn_to_mfn() calls
use the waitqueue feature to hide page-in from the caller and cover all cases where a retry is currently missing
- do not bounce p2mt to xenpaging p2m_mem_paging_populate/p2m_mem_paging_resume dont make use of p2mt
- cleanup typeof gfn which is passed around in xenpaging unsigned long, uint64_t, xen_pfn_t
- cleanup return types of functions
some functions return 0 unconditionally, could be void
- remove srand function

Keir Fraser

2011-06-08 16:01 UTC #17 lore raw

Quoted 6 lines · Olaf Hering

Quoted 3 lines · Keir Fraser
Once they're in, what is left to do? Do you need help with the in-Xen waitqueue stuff? Do you have test cases that fail, a private patch queue, etc?

Quoted 8 lines

Keir,
I have no new patches, just returned to xenpaging end of last week. A few months ago I collected my TODO list below.
One thing that I would need help with is the machine_to_phys_mapping[] handling, an attempt to fix it was reverted before the 4.1 release. See the thread in the URL below.

I'm taking a look at this. The hard way I describe in the referenced thread seemed like a mistake when I tried it, so I'll sort out a simpler fix for the original patch.

-- Keir

Quoted 38 lines

My testcase was a SLES11SP1 pv-on-hvm guest, rebooted in an endless loop. That catched all of the issues. With the waitqueue/HVMCOPY_gfn_paged_out issue its best to customize the xenpaging policy to page the range of gfns were the guest pagetables are stored.
I will send a series of patches to try later.
Olaf
Todo:
- implement xl support
- implement stopping of xenpaging
- implement live migration
- implement config option for XENPAGING_DEBUG and XENPAGING_POLICY_MRU_SIZE
- implement config option for xenpaging_dir
- implement better starting gfn in xenpaging policy
an initial gfn number in the middle of the gfn range may avoid page-ins during BIOS startup
- fix machine_to_phys_mapping[] array handling during page deallocation the gfn of a released page must be maintained properly in the array
http://lists.xensource.com/archives/html/xen-devel/2011-01/msg00824.html
- fix HVMCOPY_gfn_paged_out handling
some callers of __hvm_copy() do not handle HVMCOPY_gfn_paged_out, such as hypercalls and the MMIO emulation
the recently added waitqueue feature in Xen 4.1 should be used
- remove all retry code from gfn_to_mfn() calls
use the waitqueue feature to hide page-in from the caller and cover all cases where a retry is currently missing
- do not bounce p2mt to xenpaging p2m_mem_paging_populate/p2m_mem_paging_resume dont make use of p2mt
- cleanup typeof gfn which is passed around in xenpaging unsigned long, uint64_t, xen_pfn_t
- cleanup return types of functions
some functions return 0 unconditionally, could be void
- remove srand function

Keir Fraser

2011-06-10 07:28 UTC #18 lore raw

In reply to Olaf Hering

Quoted 6 lines · Olaf Hering

Quoted 3 lines · Keir Fraser
Once they're in, what is left to do? Do you need help with the in-Xen waitqueue stuff? Do you have test cases that fail, a private patch queue, etc?

Quoted 8 lines

Keir,
I have no new patches, just returned to xenpaging end of last week. A few months ago I collected my TODO list below.
One thing that I would need help with is the machine_to_phys_mapping[] handling, an attempt to fix it was reverted before the 4.1 release. See the thread in the URL below.

I've reapplied this as xen-unstable:23507.

-- Keir

Quoted 38 lines

My testcase was a SLES11SP1 pv-on-hvm guest, rebooted in an endless loop. That catched all of the issues. With the waitqueue/HVMCOPY_gfn_paged_out issue its best to customize the xenpaging policy to page the range of gfns were the guest pagetables are stored.
I will send a series of patches to try later.
Olaf
Todo:
- implement xl support
- implement stopping of xenpaging
- implement live migration
- implement config option for XENPAGING_DEBUG and XENPAGING_POLICY_MRU_SIZE
- implement config option for xenpaging_dir
- implement better starting gfn in xenpaging policy
an initial gfn number in the middle of the gfn range may avoid page-ins during BIOS startup
- fix machine_to_phys_mapping[] array handling during page deallocation the gfn of a released page must be maintained properly in the array
http://lists.xensource.com/archives/html/xen-devel/2011-01/msg00824.html
- fix HVMCOPY_gfn_paged_out handling
some callers of __hvm_copy() do not handle HVMCOPY_gfn_paged_out, such as hypercalls and the MMIO emulation
the recently added waitqueue feature in Xen 4.1 should be used
- remove all retry code from gfn_to_mfn() calls
use the waitqueue feature to hide page-in from the caller and cover all cases where a retry is currently missing
- do not bounce p2mt to xenpaging p2m_mem_paging_populate/p2m_mem_paging_resume dont make use of p2mt
- cleanup typeof gfn which is passed around in xenpaging unsigned long, uint64_t, xen_pfn_t
- cleanup return types of functions
some functions return 0 unconditionally, could be void
- remove srand function

Olaf Hering

author

2011-06-10 11:27 UTC #19 lore raw

Quoted 10 lines · Keir Fraser

Quoted 8 lines · Olaf Hering
Quoted 3 lines · Keir Fraser
Once they're in, what is left to do? Do you need help with the in-Xen waitqueue stuff? Do you have test cases that fail, a private patch queue, etc?
Keir,

Quoted 8 lines

Quoted 6 lines
I have no new patches, just returned to xenpaging end of last week. A few months ago I collected my TODO list below.
One thing that I would need help with is the machine_to_phys_mapping[] handling, an attempt to fix it was reverted before the 4.1 release. See the thread in the URL below.
I've reapplied this as xen-unstable:23507.

Thanks Keir.

But the change breaks 32bit builds:

prelink.o: In function `populate_physmap':
/usr/src/packages/BUILD/xen-unstable.hg-4.2.23508/xen/common/memory.c:155: undefined reference to machine_to_phys_mapping_valid' prelink.o: In function memory_exchange':
/usr/src/packages/BUILD/xen-unstable.hg-4.2.23508/xen/common/memory.c:491: undefined reference to machine_to_phys_mapping_valid' prelink.o: In function free_heap_pages':
/usr/src/packages/BUILD/xen-unstable.hg-4.2.23508/xen/common/page_alloc.c:544: undefined reference to machine_to_phys_mapping_valid' prelink.o: In function share_xen_page_with_guest':
/usr/src/packages/BUILD/xen-unstable.hg-4.2.23508/xen/arch/x86/mm.c:440: undefined reference to machine_to_phys_mapping_valid' prelink.o: In function do_mmu_update':
/usr/src/packages/BUILD/xen-unstable.hg-4.2.23508/xen/arch/x86/mm.c:3688: undefined reference to machine_to_phys_mapping_valid' prelink.o:/usr/src/packages/BUILD/xen-unstable.hg-4.2.23508/xen/arch/x86/mm/p2m.c:373: more undefined references to machine_to_phys_mapping_valid' follow

Keir Fraser

2011-06-10 12:52 UTC #20 lore raw

Quoted 19 lines · Olaf Hering

Quoted 17 lines · Keir Fraser
Quoted 14 lines · Olaf Hering
Quoted 3 lines · Keir Fraser
Once they're in, what is left to do? Do you need help with the in-Xen waitqueue stuff? Do you have test cases that fail, a private patch queue, etc?
Keir,
I have no new patches, just returned to xenpaging end of last week. A few months ago I collected my TODO list below.
One thing that I would need help with is the machine_to_phys_mapping[] handling, an attempt to fix it was reverted before the 4.1 release. See the thread in the URL below.

Quoted 5 lines

Quoted 1 line
I've reapplied this as xen-unstable:23507.
Thanks Keir.
But the change breaks 32bit builds:

Thanks, fixed by xen-unstable:23509.

-- Keir

Quoted 18 lines

prelink.o: In function `populate_physmap':
/usr/src/packages/BUILD/xen-unstable.hg-4.2.23508/xen/common/memory.c:155:
undefined reference to `machine_to_phys_mapping_valid'
prelink.o: In function `memory_exchange':
/usr/src/packages/BUILD/xen-unstable.hg-4.2.23508/xen/common/memory.c:491:
undefined reference to `machine_to_phys_mapping_valid'
prelink.o: In function `free_heap_pages':
/usr/src/packages/BUILD/xen-unstable.hg-4.2.23508/xen/common/page_alloc.c:544:
undefined reference to `machine_to_phys_mapping_valid'
prelink.o: In function `share_xen_page_with_guest':
/usr/src/packages/BUILD/xen-unstable.hg-4.2.23508/xen/arch/x86/mm.c:440:
undefined reference to `machine_to_phys_mapping_valid'
prelink.o: In function `do_mmu_update':
/usr/src/packages/BUILD/xen-unstable.hg-4.2.23508/xen/arch/x86/mm.c:3688:
undefined reference to machine_to_phys_mapping_valid' prelink.o:/usr/src/packages/BUILD/xen-unstable.hg-4.2.23508/xen/arch/x86/mm/p2 m.c:373: more undefined references to machine_to_phys_mapping_valid' follow