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

[PATCH 3/5] argo: don't pointlessly use get_domain_by_id()



For short-lived references rcu_lock_domain_by_id() is the better
(slightly cheaper) alternative.

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
---
Is it really intentional for fill_ring_data() to return success (0) in
case the domain can't be found or has argo disabled? Even if so, the
uninitialized ent.max_message_size will be leaked to the caller then
(and similarly when find_ring_info_by_match() returns NULL). Perhaps
the field should only be copied back when ent.flags is non-zero?

--- a/xen/common/argo.c
+++ b/xen/common/argo.c
@@ -445,13 +445,13 @@ signal_domain(struct domain *d)
 static void
 signal_domid(domid_t domain_id)
 {
-    struct domain *d = get_domain_by_id(domain_id);
+    struct domain *d = rcu_lock_domain_by_id(domain_id);
 
     if ( !d )
         return;
 
     signal_domain(d);
-    put_domain(d);
+    rcu_unlock_domain(d);
 }
 
 static void
@@ -983,7 +983,7 @@ ringbuf_insert(const struct domain *d, s
 static void
 wildcard_pending_list_remove(domid_t domain_id, struct pending_ent *ent)
 {
-    struct domain *d = get_domain_by_id(domain_id);
+    struct domain *d = rcu_lock_domain_by_id(domain_id);
 
     if ( !d )
         return;
@@ -996,13 +996,13 @@ wildcard_pending_list_remove(domid_t dom
         list_del(&ent->wildcard_node);
         spin_unlock(&d->argo->wildcard_L2_lock);
     }
-    put_domain(d);
+    rcu_unlock_domain(d);
 }
 
 static void
 wildcard_pending_list_insert(domid_t domain_id, struct pending_ent *ent)
 {
-    struct domain *d = get_domain_by_id(domain_id);
+    struct domain *d = rcu_lock_domain_by_id(domain_id);
 
     if ( !d )
         return;
@@ -1015,7 +1015,7 @@ wildcard_pending_list_insert(domid_t dom
         list_add(&ent->wildcard_node, &d->argo->wildcard_pend_list);
         spin_unlock(&d->argo->wildcard_L2_lock);
     }
-    put_domain(d);
+    rcu_unlock_domain(d);
 }
 
 static void
@@ -1283,7 +1283,7 @@ partner_rings_remove(struct domain *src_
                                                       struct argo_send_info,
                                                       node)) )
         {
-            struct domain *dst_d = get_domain_by_id(send_info->id.domain_id);
+            struct domain *dst_d = 
rcu_lock_domain_by_id(send_info->id.domain_id);
 
             if ( dst_d && dst_d->argo )
             {
@@ -1302,7 +1302,7 @@ partner_rings_remove(struct domain *src_
                 ASSERT_UNREACHABLE();
 
             if ( dst_d )
-                put_domain(dst_d);
+                rcu_unlock_domain(dst_d);
 
             list_del(&send_info->node);
             xfree(send_info);
@@ -1330,7 +1330,7 @@ fill_ring_data(const struct domain *curr
 
     ent.flags = 0;
 
-    dst_d = get_domain_by_id(ent.ring.domain_id);
+    dst_d = rcu_lock_domain_by_id(ent.ring.domain_id);
     if ( !dst_d || !dst_d->argo )
         goto out;
 
@@ -1340,10 +1340,7 @@ fill_ring_data(const struct domain *curr
      */
     ret = xsm_argo_send(currd, dst_d);
     if ( ret )
-    {
-        put_domain(dst_d);
-        return ret;
-    }
+        goto out;
 
     read_lock(&dst_d->argo->rings_L2_rwlock);
 
@@ -1405,7 +1402,7 @@ fill_ring_data(const struct domain *curr
 
  out:
     if ( dst_d )
-        put_domain(dst_d);
+        rcu_unlock_domain(dst_d);
 
     if ( !ret && (__copy_field_to_guest(data_ent_hnd, &ent, flags) ||
                   __copy_field_to_guest(data_ent_hnd, &ent, max_message_size)) 
)
@@ -1569,7 +1566,7 @@ unregister_ring(struct domain *currd,
     if ( ring_id.partner_id == XEN_ARGO_DOMID_ANY )
         goto out;
 
-    dst_d = get_domain_by_id(ring_id.partner_id);
+    dst_d = rcu_lock_domain_by_id(ring_id.partner_id);
     if ( !dst_d || !dst_d->argo )
     {
         ASSERT_UNREACHABLE();
@@ -1592,7 +1589,7 @@ unregister_ring(struct domain *currd,
     read_unlock(&L1_global_argo_rwlock);
 
     if ( dst_d )
-        put_domain(dst_d);
+        rcu_unlock_domain(dst_d);
 
     xfree(send_info);
 
@@ -1663,7 +1660,7 @@ register_ring(struct domain *currd,
     }
     else
     {
-        dst_d = get_domain_by_id(reg.partner_id);
+        dst_d = rcu_lock_domain_by_id(reg.partner_id);
         if ( !dst_d )
         {
             argo_dprintk("!dst_d, ESRCH\n");
@@ -1845,7 +1842,7 @@ register_ring(struct domain *currd,
 
  out:
     if ( dst_d )
-        put_domain(dst_d);
+        rcu_unlock_domain(dst_d);
 
     if ( ret )
         xfree(send_info);
@@ -1988,7 +1985,7 @@ sendv(struct domain *src_d, xen_argo_add
     src_id.domain_id = src_d->domain_id;
     src_id.partner_id = dst_addr->domain_id;
 
-    dst_d = get_domain_by_id(dst_addr->domain_id);
+    dst_d = rcu_lock_domain_by_id(dst_addr->domain_id);
     if ( !dst_d )
         return -ESRCH;
 
@@ -1998,7 +1995,7 @@ sendv(struct domain *src_d, xen_argo_add
         gprintk(XENLOG_ERR, "argo: XSM REJECTED %i -> %i\n",
                 src_d->domain_id, dst_d->domain_id);
 
-        put_domain(dst_d);
+        rcu_unlock_domain(dst_d);
 
         return ret;
     }
@@ -2068,7 +2065,7 @@ sendv(struct domain *src_d, xen_argo_add
         signal_domain(dst_d);
 
     if ( dst_d )
-        put_domain(dst_d);
+        rcu_unlock_domain(dst_d);
 
     return ( ret < 0 ) ? ret : len;
 }




 


Rackspace

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