direct-io.hg

changeset 15465:9fa9346e1c70

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@xensource.com>
author kfraser@localhost.localdomain
date Tue Jul 03 17:22:17 2007 +0100 (2007-07-03)
parents f1b62eb7f8be
children e6d5e4709466
files xen/common/domain.c xen/common/event_channel.c
line diff
     1.1 --- a/xen/common/domain.c	Tue Jul 03 17:04:50 2007 +0100
     1.2 +++ b/xen/common/domain.c	Tue Jul 03 17:22:17 2007 +0100
     1.3 @@ -180,6 +180,8 @@ struct domain *domain_create(
     1.4      domid_t domid, unsigned int domcr_flags, ssidref_t ssidref)
     1.5  {
     1.6      struct domain *d, **pd;
     1.7 +    enum { INIT_evtchn = 1, INIT_gnttab = 2, INIT_acm = 4, INIT_arch = 8 }; 
     1.8 +    int init_status = 0;
     1.9  
    1.10      if ( (d = alloc_domain(domid)) == NULL )
    1.11          return NULL;
    1.12 @@ -195,25 +197,29 @@ struct domain *domain_create(
    1.13          atomic_inc(&d->pause_count);
    1.14  
    1.15          if ( evtchn_init(d) != 0 )
    1.16 -            goto fail1;
    1.17 +            goto fail;
    1.18 +        init_status |= INIT_evtchn;
    1.19  
    1.20          if ( grant_table_create(d) != 0 )
    1.21 -            goto fail2;
    1.22 +            goto fail;
    1.23 +        init_status |= INIT_gnttab;
    1.24  
    1.25          if ( acm_domain_create(d, ssidref) != 0 )
    1.26 -            goto fail3;
    1.27 +            goto fail;
    1.28 +        init_status |= INIT_acm;
    1.29      }
    1.30  
    1.31      if ( arch_domain_create(d) != 0 )
    1.32 -        goto fail4;
    1.33 +        goto fail;
    1.34 +    init_status |= INIT_arch;
    1.35  
    1.36      d->iomem_caps = rangeset_new(d, "I/O Memory", RANGESETF_prettyprint_hex);
    1.37      d->irq_caps   = rangeset_new(d, "Interrupts", 0);
    1.38      if ( (d->iomem_caps == NULL) || (d->irq_caps == NULL) )
    1.39 -        goto fail5;
    1.40 +        goto fail;
    1.41  
    1.42      if ( sched_init_domain(d) != 0 )
    1.43 -        goto fail5;
    1.44 +        goto fail;
    1.45  
    1.46      if ( !is_idle_domain(d) )
    1.47      {
    1.48 @@ -224,10 +230,6 @@ struct domain *domain_create(
    1.49                  break;
    1.50          d->next_in_list = *pd;
    1.51          d->next_in_hashbucket = domain_hash[DOMAIN_HASH(domid)];
    1.52 -        /* Two rcu assignments are not atomic 
    1.53 -         * Readers may see inconsistent domlist and hash table
    1.54 -         * That is OK as long as each RCU reader-side critical section uses
    1.55 -         * only one or them  */
    1.56          rcu_assign_pointer(*pd, d);
    1.57          rcu_assign_pointer(domain_hash[DOMAIN_HASH(domid)], d);
    1.58          spin_unlock(&domlist_update_lock);
    1.59 @@ -235,18 +237,17 @@ struct domain *domain_create(
    1.60  
    1.61      return d;
    1.62  
    1.63 - fail5:
    1.64 -    arch_domain_destroy(d);
    1.65 - fail4:
    1.66 -    if ( !is_idle_domain(d) )
    1.67 + fail:
    1.68 +    d->is_dying = 1;
    1.69 +    atomic_set(&d->refcnt, DOMAIN_DESTROYED);
    1.70 +    if ( init_status & INIT_arch )
    1.71 +        arch_domain_destroy(d);
    1.72 +    if ( init_status & INIT_acm )
    1.73          acm_domain_destroy(d);
    1.74 - fail3:
    1.75 -    if ( !is_idle_domain(d) )
    1.76 +    if ( init_status & INIT_gnttab )
    1.77          grant_table_destroy(d);
    1.78 - fail2:
    1.79 -    if ( !is_idle_domain(d) )
    1.80 +    if ( init_status & INIT_evtchn )
    1.81          evtchn_destroy(d);
    1.82 - fail1:
    1.83      rangeset_domain_destroy(d);
    1.84      free_domain(d);
    1.85      return NULL;
    1.86 @@ -308,6 +309,7 @@ void domain_kill(struct domain *d)
    1.87          return;
    1.88      }
    1.89  
    1.90 +    evtchn_destroy(d);
    1.91      gnttab_release_mappings(d);
    1.92      domain_relinquish_resources(d);
    1.93      put_domain(d);
    1.94 @@ -473,7 +475,6 @@ static void complete_domain_destroy(stru
    1.95  
    1.96      rangeset_domain_destroy(d);
    1.97  
    1.98 -    evtchn_destroy(d);
    1.99      grant_table_destroy(d);
   1.100  
   1.101      arch_domain_destroy(d);
     2.1 --- a/xen/common/event_channel.c	Tue Jul 03 17:04:50 2007 +0100
     2.2 +++ b/xen/common/event_channel.c	Tue Jul 03 17:22:17 2007 +0100
     2.3 @@ -79,6 +79,9 @@ static int get_free_port(struct domain *
     2.4      struct evtchn *chn;
     2.5      int            port;
     2.6  
     2.7 +    if ( d->is_dying )
     2.8 +        return -EINVAL;
     2.9 +
    2.10      for ( port = 0; port_is_valid(d, port); port++ )
    2.11          if ( evtchn_from_port(d, port)->state == ECS_FREE )
    2.12              return port;
    2.13 @@ -374,14 +377,7 @@ static long __evtchn_close(struct domain
    2.14  
    2.15              /* If we unlock d1 then we could lose d2. Must get a reference. */
    2.16              if ( unlikely(!get_domain(d2)) )
    2.17 -            {
    2.18 -                /*
    2.19 -                 * Failed to obtain a reference. No matter: d2 must be dying
    2.20 -                 * and so will close this event channel for us.
    2.21 -                 */
    2.22 -                d2 = NULL;
    2.23 -                goto out;
    2.24 -            }
    2.25 +                BUG();
    2.26  
    2.27              if ( d1 < d2 )
    2.28              {
    2.29 @@ -403,7 +399,6 @@ static long __evtchn_close(struct domain
    2.30               * port in ECS_CLOSED. It must have passed through that state for
    2.31               * us to end up here, so it's a valid error to return.
    2.32               */
    2.33 -            BUG_ON(d1 != current->domain);
    2.34              rc = -EINVAL;
    2.35              goto out;
    2.36          }
    2.37 @@ -960,14 +955,22 @@ void evtchn_destroy(struct domain *d)
    2.38  {
    2.39      int i;
    2.40  
    2.41 +    /* After this barrier no new event-channel allocations can occur. */
    2.42 +    BUG_ON(!d->is_dying);
    2.43 +    spin_barrier(&d->evtchn_lock);
    2.44 +
    2.45 +    /* Close all existing event channels. */
    2.46      for ( i = 0; port_is_valid(d, i); i++ )
    2.47      {
    2.48          evtchn_from_port(d, i)->consumer_is_xen = 0;
    2.49          (void)__evtchn_close(d, i);
    2.50      }
    2.51  
    2.52 +    /* Free all event-channel buckets. */
    2.53 +    spin_lock(&d->evtchn_lock);
    2.54      for ( i = 0; i < NR_EVTCHN_BUCKETS; i++ )
    2.55          xfree(d->evtchn[i]);
    2.56 +    spin_unlock(&d->evtchn_lock);
    2.57  }
    2.58  
    2.59  /*