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

[Xen-changelog] [xen-unstable] xen: Fix event-channel destruction.



# HG changeset patch
# User kfraser@xxxxxxxxxxxxxxxxxxxxx
# Date 1183479737 -3600
# Node ID 9fa9346e1c700d0ea81a99318c564c4b9bccaa8a
# Parent  f1b62eb7f8be42485356382ac0f6bf5f5a430c22
xen: Fix event-channel destruction.

Lots of churn to fix a couple of bugs:

 1. If we tried to close an interdomain channel, and the remote domain
    is being destroyed, we immediately return success. But the remote
    domain will place our port in state 'unbound', not 'free'. Hence
    the port is effectively leaked.

 2. If two domains are being destroyed at the same time, and share an
    interdomain channel, the second to attempt the close may
    dereference an invalid domain pointer. Aiee!

Doing som ework to be able to destroy event-channel context much
earlier in domain death was the civilised thing to do.

Signed-off-by: Keir Fraser <keir@xxxxxxxxxxxxx>
---
 xen/common/domain.c        |   41 +++++++++++++++++++++--------------------
 xen/common/event_channel.c |   21 ++++++++++++---------
 2 files changed, 33 insertions(+), 29 deletions(-)

diff -r f1b62eb7f8be -r 9fa9346e1c70 xen/common/domain.c
--- a/xen/common/domain.c       Tue Jul 03 17:04:50 2007 +0100
+++ b/xen/common/domain.c       Tue Jul 03 17:22:17 2007 +0100
@@ -180,6 +180,8 @@ struct domain *domain_create(
     domid_t domid, unsigned int domcr_flags, ssidref_t ssidref)
 {
     struct domain *d, **pd;
+    enum { INIT_evtchn = 1, INIT_gnttab = 2, INIT_acm = 4, INIT_arch = 8 }; 
+    int init_status = 0;
 
     if ( (d = alloc_domain(domid)) == NULL )
         return NULL;
@@ -195,25 +197,29 @@ struct domain *domain_create(
         atomic_inc(&d->pause_count);
 
         if ( evtchn_init(d) != 0 )
-            goto fail1;
+            goto fail;
+        init_status |= INIT_evtchn;
 
         if ( grant_table_create(d) != 0 )
-            goto fail2;
+            goto fail;
+        init_status |= INIT_gnttab;
 
         if ( acm_domain_create(d, ssidref) != 0 )
-            goto fail3;
+            goto fail;
+        init_status |= INIT_acm;
     }
 
     if ( arch_domain_create(d) != 0 )
-        goto fail4;
+        goto fail;
+    init_status |= INIT_arch;
 
     d->iomem_caps = rangeset_new(d, "I/O Memory", RANGESETF_prettyprint_hex);
     d->irq_caps   = rangeset_new(d, "Interrupts", 0);
     if ( (d->iomem_caps == NULL) || (d->irq_caps == NULL) )
-        goto fail5;
+        goto fail;
 
     if ( sched_init_domain(d) != 0 )
-        goto fail5;
+        goto fail;
 
     if ( !is_idle_domain(d) )
     {
@@ -224,10 +230,6 @@ struct domain *domain_create(
                 break;
         d->next_in_list = *pd;
         d->next_in_hashbucket = domain_hash[DOMAIN_HASH(domid)];
-        /* Two rcu assignments are not atomic 
-         * Readers may see inconsistent domlist and hash table
-         * That is OK as long as each RCU reader-side critical section uses
-         * only one or them  */
         rcu_assign_pointer(*pd, d);
         rcu_assign_pointer(domain_hash[DOMAIN_HASH(domid)], d);
         spin_unlock(&domlist_update_lock);
@@ -235,18 +237,17 @@ struct domain *domain_create(
 
     return d;
 
- fail5:
-    arch_domain_destroy(d);
- fail4:
-    if ( !is_idle_domain(d) )
+ fail:
+    d->is_dying = 1;
+    atomic_set(&d->refcnt, DOMAIN_DESTROYED);
+    if ( init_status & INIT_arch )
+        arch_domain_destroy(d);
+    if ( init_status & INIT_acm )
         acm_domain_destroy(d);
- fail3:
-    if ( !is_idle_domain(d) )
+    if ( init_status & INIT_gnttab )
         grant_table_destroy(d);
- fail2:
-    if ( !is_idle_domain(d) )
+    if ( init_status & INIT_evtchn )
         evtchn_destroy(d);
- fail1:
     rangeset_domain_destroy(d);
     free_domain(d);
     return NULL;
@@ -308,6 +309,7 @@ void domain_kill(struct domain *d)
         return;
     }
 
+    evtchn_destroy(d);
     gnttab_release_mappings(d);
     domain_relinquish_resources(d);
     put_domain(d);
@@ -473,7 +475,6 @@ static void complete_domain_destroy(stru
 
     rangeset_domain_destroy(d);
 
-    evtchn_destroy(d);
     grant_table_destroy(d);
 
     arch_domain_destroy(d);
diff -r f1b62eb7f8be -r 9fa9346e1c70 xen/common/event_channel.c
--- a/xen/common/event_channel.c        Tue Jul 03 17:04:50 2007 +0100
+++ b/xen/common/event_channel.c        Tue Jul 03 17:22:17 2007 +0100
@@ -79,6 +79,9 @@ static int get_free_port(struct domain *
     struct evtchn *chn;
     int            port;
 
+    if ( d->is_dying )
+        return -EINVAL;
+
     for ( port = 0; port_is_valid(d, port); port++ )
         if ( evtchn_from_port(d, port)->state == ECS_FREE )
             return port;
@@ -374,14 +377,7 @@ static long __evtchn_close(struct domain
 
             /* If we unlock d1 then we could lose d2. Must get a reference. */
             if ( unlikely(!get_domain(d2)) )
-            {
-                /*
-                 * Failed to obtain a reference. No matter: d2 must be dying
-                 * and so will close this event channel for us.
-                 */
-                d2 = NULL;
-                goto out;
-            }
+                BUG();
 
             if ( d1 < d2 )
             {
@@ -403,7 +399,6 @@ static long __evtchn_close(struct domain
              * port in ECS_CLOSED. It must have passed through that state for
              * us to end up here, so it's a valid error to return.
              */
-            BUG_ON(d1 != current->domain);
             rc = -EINVAL;
             goto out;
         }
@@ -960,14 +955,22 @@ void evtchn_destroy(struct domain *d)
 {
     int i;
 
+    /* After this barrier no new event-channel allocations can occur. */
+    BUG_ON(!d->is_dying);
+    spin_barrier(&d->evtchn_lock);
+
+    /* Close all existing event channels. */
     for ( i = 0; port_is_valid(d, i); i++ )
     {
         evtchn_from_port(d, i)->consumer_is_xen = 0;
         (void)__evtchn_close(d, i);
     }
 
+    /* Free all event-channel buckets. */
+    spin_lock(&d->evtchn_lock);
     for ( i = 0; i < NR_EVTCHN_BUCKETS; i++ )
         xfree(d->evtchn[i]);
+    spin_unlock(&d->evtchn_lock);
 }
 
 /*

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


 


Rackspace

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