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

[Xen-devel] [PATCH] xenpaging: update xch usage


  • To: xen-devel@xxxxxxxxxxxxxxxxxxx
  • From: Olaf Hering <olaf@xxxxxxxxx>
  • Date: Thu, 2 Dec 2010 10:54:42 +0100
  • Delivery-date: Thu, 02 Dec 2010 01:55:52 -0800
  • List-id: Xen developer discussion <xen-devel.lists.xensource.com>

Fix double-free in xc_interface_close(), xenpaging_teardown() releases
the *xch already. Remove second xc_interface_close() call.

Fix DPRINTF/ERROR usage, both macros reference a xch variable in local
scope. If xc_interface_open fails for some reason, and after
xc_interface_close, both can not be used anymore. Use standard printf
for this case.

Instead of passing xch around, use the handle from *paging. In the
updated functions, use the local xch variable.

Signed-off-by: Olaf Hering <olaf@xxxxxxxxx>

---
 tools/xenpaging/policy.h         |    3 -
 tools/xenpaging/policy_default.c |    4 +-
 tools/xenpaging/xenpaging.c      |   65 +++++++++++++++++++--------------------
 3 files changed, 36 insertions(+), 36 deletions(-)

--- xen-unstable.hg-4.1.22447.orig/tools/xenpaging/policy.h
+++ xen-unstable.hg-4.1.22447/tools/xenpaging/policy.h
@@ -29,8 +29,7 @@
 
 
 int policy_init(xenpaging_t *paging);
-int policy_choose_victim(xc_interface *xch,
-                         xenpaging_t *paging, domid_t domain_id,
+int policy_choose_victim(xenpaging_t *paging, domid_t domain_id,
                          xenpaging_victim_t *victim);
 void policy_notify_paged_out(domid_t domain_id, unsigned long gfn);
 void policy_notify_paged_in(domid_t domain_id, unsigned long gfn);
--- xen-unstable.hg-4.1.22447.orig/tools/xenpaging/policy_default.c
+++ xen-unstable.hg-4.1.22447/tools/xenpaging/policy_default.c
@@ -67,10 +67,10 @@ int policy_init(xenpaging_t *paging)
     return rc;
 }
 
-int policy_choose_victim(xc_interface *xch,
-                         xenpaging_t *paging, domid_t domain_id,
+int policy_choose_victim(xenpaging_t *paging, domid_t domain_id,
                          xenpaging_victim_t *victim)
 {
+    struct xc_interface *xch = paging->xc_handle;
     unsigned long wrap = current_gfn;
     ASSERT(victim != NULL);
 
--- xen-unstable.hg-4.1.22447.orig/tools/xenpaging/xenpaging.c
+++ xen-unstable.hg-4.1.22447/tools/xenpaging/xenpaging.c
@@ -79,7 +79,7 @@ static void *init_page(void)
     return NULL;
 }
 
-xenpaging_t *xenpaging_init(xc_interface **xch_r, domid_t domain_id)
+xenpaging_t *xenpaging_init(domid_t domain_id)
 {
     xenpaging_t *paging;
     xc_interface *xch;
@@ -90,7 +90,6 @@ xenpaging_t *xenpaging_init(xc_interface
         goto err_iface;
 
     DPRINTF("xenpaging init\n");
-    *xch_r = xch;
 
     /* Allocate memory */
     paging = malloc(sizeof(xenpaging_t));
@@ -128,7 +127,7 @@ xenpaging_t *xenpaging_init(xc_interface
     mem_event_ring_lock_init(&paging->mem_event);
     
     /* Initialise Xen */
-    rc = xc_mem_event_enable(paging->xc_handle, paging->mem_event.domain_id,
+    rc = xc_mem_event_enable(xch, paging->mem_event.domain_id,
                              paging->mem_event.shared_page,
                              paging->mem_event.ring_page);
     if ( rc != 0 )
@@ -175,7 +174,7 @@ xenpaging_t *xenpaging_init(xc_interface
         goto err;
     }
 
-    rc = xc_get_platform_info(paging->xc_handle, domain_id,
+    rc = xc_get_platform_info(xch, domain_id,
                               paging->platform_info);
     if ( rc != 1 )
     {
@@ -191,7 +190,7 @@ xenpaging_t *xenpaging_init(xc_interface
         goto err;
     }
 
-    rc = xc_domain_getinfolist(paging->xc_handle, domain_id, 1,
+    rc = xc_domain_getinfolist(xch, domain_id, 1,
                                paging->domain_info);
     if ( rc != 1 )
     {
@@ -224,6 +223,7 @@ xenpaging_t *xenpaging_init(xc_interface
  err:
     if ( paging )
     {
+        xc_interface_close(xch);
         if ( paging->mem_event.shared_page )
         {
             munlock(paging->mem_event.shared_page, PAGE_SIZE);
@@ -246,15 +246,18 @@ xenpaging_t *xenpaging_init(xc_interface
     return NULL;
 }
 
-int xenpaging_teardown(xc_interface *xch, xenpaging_t *paging)
+int xenpaging_teardown(xenpaging_t *paging)
 {
     int rc;
+    struct xc_interface *xch;
 
     if ( paging == NULL )
         return 0;
 
+    xch = paging->xc_handle;
+    paging->xc_handle = NULL;
     /* Tear down domain paging in Xen */
-    rc = xc_mem_event_disable(paging->xc_handle, paging->mem_event.domain_id);
+    rc = xc_mem_event_disable(xch, paging->mem_event.domain_id);
     if ( rc != 0 )
     {
         ERROR("Error tearing down domain paging in xen");
@@ -277,12 +280,11 @@ int xenpaging_teardown(xc_interface *xch
     paging->mem_event.xce_handle = -1;
     
     /* Close connection to Xen */
-    rc = xc_interface_close(paging->xc_handle);
+    rc = xc_interface_close(xch);
     if ( rc != 0 )
     {
         ERROR("Error closing connection to xen");
     }
-    paging->xc_handle = NULL;
 
     return 0;
 
@@ -336,9 +338,9 @@ static int put_response(mem_event_t *mem
     return 0;
 }
 
-int xenpaging_evict_page(xc_interface *xch, xenpaging_t *paging,
-                         xenpaging_victim_t *victim, int fd, int i)
+int xenpaging_evict_page(xenpaging_t *paging, xenpaging_victim_t *victim, int 
fd, int i)
 {
+    struct xc_interface *xch = paging->xc_handle;
     void *page;
     unsigned long gfn;
     int ret;
@@ -348,7 +350,7 @@ int xenpaging_evict_page(xc_interface *x
     /* Map page */
     gfn = victim->gfn;
     ret = -EFAULT;
-    page = xc_map_foreign_pages(paging->xc_handle, victim->domain_id,
+    page = xc_map_foreign_pages(xch, victim->domain_id,
                                 PROT_READ | PROT_WRITE, &gfn, 1);
     if ( page == NULL )
     {
@@ -371,7 +373,7 @@ int xenpaging_evict_page(xc_interface *x
     munmap(page, PAGE_SIZE);
 
     /* Tell Xen to evict page */
-    ret = xc_mem_paging_evict(paging->xc_handle, paging->mem_event.domain_id,
+    ret = xc_mem_paging_evict(xch, paging->mem_event.domain_id,
                               victim->gfn);
     if ( ret != 0 )
     {
@@ -409,10 +411,9 @@ static int xenpaging_resume_page(xenpagi
     return ret;
 }
 
-static int xenpaging_populate_page(
-    xc_interface *xch, xenpaging_t *paging,
-    uint64_t *gfn, int fd, int i)
+static int xenpaging_populate_page(xenpaging_t *paging, uint64_t *gfn, int fd, 
int i)
 {
+    struct xc_interface *xch = paging->xc_handle;
     unsigned long _gfn;
     void *page;
     int ret;
@@ -422,7 +423,7 @@ static int xenpaging_populate_page(
     do
     {
         /* Tell Xen to allocate a page for the domain */
-        ret = xc_mem_paging_prep(paging->xc_handle, 
paging->mem_event.domain_id,
+        ret = xc_mem_paging_prep(xch, paging->mem_event.domain_id,
                                  _gfn);
         if ( ret != 0 )
         {
@@ -441,7 +442,7 @@ static int xenpaging_populate_page(
 
     /* Map page */
     ret = -EFAULT;
-    page = xc_map_foreign_pages(paging->xc_handle, paging->mem_event.domain_id,
+    page = xc_map_foreign_pages(xch, paging->mem_event.domain_id,
                                 PROT_READ | PROT_WRITE, &_gfn, 1);
     *gfn = _gfn;
     if ( page == NULL )
@@ -464,15 +465,16 @@ static int xenpaging_populate_page(
     return ret;
 }
 
-static int evict_victim(xc_interface *xch, xenpaging_t *paging, domid_t 
domain_id,
+static int evict_victim(xenpaging_t *paging, domid_t domain_id,
                         xenpaging_victim_t *victim, int fd, int i)
 {
+    struct xc_interface *xch = paging->xc_handle;
     int j = 0;
     int ret;
 
     do
     {
-        ret = policy_choose_victim(xch, paging, domain_id, victim);
+        ret = policy_choose_victim(paging, domain_id, victim);
         if ( ret != 0 )
         {
             if ( ret != -ENOSPC )
@@ -485,10 +487,10 @@ static int evict_victim(xc_interface *xc
             ret = -EINTR;
             goto out;
         }
-        ret = xc_mem_paging_nominate(paging->xc_handle,
+        ret = xc_mem_paging_nominate(xch,
                                      paging->mem_event.domain_id, victim->gfn);
         if ( ret == 0 )
-            ret = xenpaging_evict_page(xch, paging, victim, fd, i);
+            ret = xenpaging_evict_page(paging, victim, fd, i);
         else
         {
             if ( j++ % 1000 == 0 )
@@ -538,13 +540,14 @@ int main(int argc, char *argv[])
     srand(time(NULL));
 
     /* Initialise domain paging */
-    paging = xenpaging_init(&xch, domain_id);
+    paging = xenpaging_init(domain_id);
     if ( paging == NULL )
     {
-        ERROR("Error initialising paging");
+        fprintf(stderr, "Error initialising paging");
         return 1;
     }
 
+    xch = paging->xc_handle;
     DPRINTF("starting %s %u %d\n", argv[0], domain_id, num_pages);
 
     /* Open file */
@@ -576,7 +579,7 @@ int main(int argc, char *argv[])
     memset(victims, 0, sizeof(xenpaging_victim_t) * num_pages);
     for ( i = 0; i < num_pages; i++ )
     {
-        rc = evict_victim(xch, paging, domain_id, &victims[i], fd, i);
+        rc = evict_victim(paging, domain_id, &victims[i], fd, i);
         if ( rc == -ENOSPC )
             break;
         if ( rc == -EINTR )
@@ -629,7 +632,7 @@ int main(int argc, char *argv[])
                 }
                 
                 /* Populate the page */
-                rc = xenpaging_populate_page(xch, paging, &req.gfn, fd, i);
+                rc = xenpaging_populate_page(paging, &req.gfn, fd, i);
                 if ( rc != 0 )
                 {
                     ERROR("Error populating page");
@@ -650,7 +653,7 @@ int main(int argc, char *argv[])
                 }
 
                 /* Evict a new page to replace the one we just paged in */
-                evict_victim(xch, paging, domain_id, &victims[i], fd, i);
+                evict_victim(paging, domain_id, &victims[i], fd, i);
             }
             else
             {
@@ -688,16 +691,14 @@ int main(int argc, char *argv[])
     free(victims);
 
     /* Tear down domain paging */
-    rc1 = xenpaging_teardown(xch, paging);
+    rc1 = xenpaging_teardown(paging);
     if ( rc1 != 0 )
-        ERROR("Error tearing down paging");
+        fprintf(stderr, "Error tearing down paging");
 
     if ( rc == 0 )
         rc = rc1;
 
-    xc_interface_close(xch);
-
-    DPRINTF("xenpaging exit code %d\n", rc);
+    printf("xenpaging exit code %d\n", rc);
     return rc;
 }
 

_______________________________________________
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®.