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

[Xen-devel] [patch] new version of find_domain_by_id() without reference count [2/6]



Following Keir suggestion, this is set of patches to add a new version
of find_domain_by_id() which does not increment the domain reference
counter. This reduces the overhead and can be used by any function which
does not need to keep a domain reference beyond its current invocation,
as the rcu mechanism prevents the domain from being removed under our
feet. Of course, this can only be applied after the RCU patch posted
earlier.

Beyond adding the function the patch also replaces most invocations to
find_domain_by_id() with the new function find_domain_by_id_noref().
Only a few places needed to continue using the old function as the
reference was kept beyond the function invocation.
  
I only did minor tests on x86-32. Xen and dom0 boots fine and I can
create and destroy domains. But, no more exaustive tests were done. I
carefully checked if I removed all put_domain() associated with each
modified invocation of find_domain_by_id but mistakes are always
possible. It would be good to put this to some more exhaustive tests
before pushing it to the main tree. Waiting for post 3.0.4 release is
strongly suggested.

I also decomposed the patch in multiple parts so that the mantainers of
each architecture can review changes in their subtree, test  and apply
them at their convenience.

This patch:
2/6: replace find_domain_by_id on acm subtree

=======================================
Signed-off-by: Jose Renato Santos <jsantos@xxxxxxxxxx>

# HG changeset patch
# User root@xxxxxxxxxxxxxxxxxxx
# Node ID ac17c8457896f63da25c5458b5a76b2ab83139cd
# Parent  2301c76e27c401a04f29745024091a374d49f424
Replace find_domain_by_id() with find_domain_by_id_noref() for acm files

diff -r 2301c76e27c4 -r ac17c8457896 xen/acm/acm_core.c
--- a/xen/acm/acm_core.c        Fri Dec  8 18:09:25 2006 -0800
+++ b/xen/acm/acm_core.c        Fri Dec  8 18:13:10 2006 -0800
@@ -276,7 +276,7 @@ acm_init_domain_ssid(domid_t id, ssidref
 acm_init_domain_ssid(domid_t id, ssidref_t ssidref)
 {
     struct acm_ssid_domain *ssid;
-    struct domain *subj = find_domain_by_id(id);
+    struct domain *subj = find_domain_by_id_noref(id);
     int ret1, ret2;
  
     if (subj == NULL)
@@ -286,7 +286,6 @@ acm_init_domain_ssid(domid_t id, ssidref
     }
     if ((ssid = xmalloc(struct acm_ssid_domain)) == NULL)
     {
-        put_domain(subj);
         return ACM_INIT_SSID_ERROR;
     }
 
@@ -318,12 +317,10 @@ acm_init_domain_ssid(domid_t id, ssidref
         printk("%s: ERROR instantiating individual ssids for domain
0x%02x.\n",
                __func__, subj->domain_id);
         acm_free_domain_ssid(ssid);
-        put_domain(subj);
         return ACM_INIT_SSID_ERROR;
     }
     printkd("%s: assigned domain %x the ssidref=%x.\n",
            __func__, id, ssid->ssidref);
-    put_domain(subj);
     return ACM_OK;
 }
 
diff -r 2301c76e27c4 -r ac17c8457896
xen/acm/acm_simple_type_enforcement_hooks.c
--- a/xen/acm/acm_simple_type_enforcement_hooks.c       Fri Dec  8
18:09:25 2006 -0800
+++ b/xen/acm/acm_simple_type_enforcement_hooks.c       Fri Dec  8
18:13:10 2006 -0800
@@ -206,7 +206,7 @@ ste_init_state(struct acm_ste_policy_buf
                 ste_rssidref = ste_rssid->ste_ssidref;
             } else if (d->evtchn[port]->state == ECS_UNBOUND) {
                 rdomid = d->evtchn[port]->u.unbound.remote_domid;
-                if ((rdom = find_domain_by_id(rdomid)) == NULL) {
+                if ((rdom = find_domain_by_id_noref(rdomid)) == NULL) {
                     printk("%s: Error finding domain to id %x!\n",
__func__, rdomid);
                     goto out;
                 }
@@ -214,7 +214,6 @@ ste_init_state(struct acm_ste_policy_buf
                 ste_rssid =
GET_SSIDP(ACM_SIMPLE_TYPE_ENFORCEMENT_POLICY, 
                                       (struct acm_ssid_domain
*)(rdom->ssid));
                 ste_rssidref = ste_rssid->ste_ssidref;
-                put_domain(rdom);
             } else {
                 spin_unlock(&d->evtchn_lock);
                 continue; /* port unused */
@@ -246,7 +245,7 @@ ste_init_state(struct acm_ste_policy_buf
                         __func__, (d->domain_id, i, sha_copy.flags,
sha_copy.domid, 
                         (unsigned long)sha_copy.frame);
                 rdomid = sha_copy.domid;
-                if ((rdom = find_domain_by_id(rdomid)) == NULL) {
+                if ((rdom = find_domain_by_id_noref(rdomid)) == NULL) {
                     printkd("%s: domain not found ERROR!\n", __func__);
                     goto out;
                 };
@@ -254,7 +253,6 @@ ste_init_state(struct acm_ste_policy_buf
                 ste_rssid =
GET_SSIDP(ACM_SIMPLE_TYPE_ENFORCEMENT_POLICY, 
                                       (struct acm_ssid_domain
*)(rdom->ssid));
                 ste_rssidref = ste_rssid->ste_ssidref;
-                put_domain(rdom);
                 if (!have_common_type(ste_ssidref, ste_rssidref)) {
                     printkd("%s: Policy violation in grant table
sharing domain %x -> domain %x.\n",
                             __func__, d->domain_id, rdomid);
@@ -510,8 +508,8 @@ ste_pre_eventchannel_unbound(domid_t id1
     if (id1 == DOMID_SELF) id1 = current->domain->domain_id;
     if (id2 == DOMID_SELF) id2 = current->domain->domain_id;
 
-    subj = find_domain_by_id(id1);
-    obj  = find_domain_by_id(id2);
+    subj = find_domain_by_id_noref(id1);
+    obj  = find_domain_by_id_noref(id2);
     if ((subj == NULL) || (obj == NULL)) {
         ret = ACM_ACCESS_DENIED;
         goto out;
@@ -532,10 +530,6 @@ ste_pre_eventchannel_unbound(domid_t id1
         ret = ACM_ACCESS_DENIED;
     }
   out:
-    if (obj != NULL)
-        put_domain(obj);
-    if (subj != NULL)
-        put_domain(subj);
     return ret;
 }
 
@@ -555,7 +549,7 @@ ste_pre_eventchannel_interdomain(domid_t
     if (id == DOMID_SELF) id = current->domain->domain_id;
 
     subj = current->domain;
-    obj  = find_domain_by_id(id);
+    obj  = find_domain_by_id_noref(id);
     if (obj == NULL) {
         ret = ACM_ACCESS_DENIED;
         goto out;
@@ -578,8 +572,6 @@ ste_pre_eventchannel_interdomain(domid_t
         ret = ACM_ACCESS_DENIED;
     }
  out:
-    if (obj != NULL)
-        put_domain(obj);
     return ret;
 }
 
@@ -598,7 +590,7 @@ ste_pre_grant_map_ref (domid_t id) {
     }
     atomic_inc(&ste_bin_pol.gt_eval_count);
     subj = current->domain;
-    obj = find_domain_by_id(id);
+    obj = find_domain_by_id_noref(id);
 
     if (share_common_type(subj, obj)) {
         cache_result(subj, obj);
@@ -608,8 +600,6 @@ ste_pre_grant_map_ref (domid_t id) {
         printkd("%s: ACCESS DENIED!\n", __func__);
         ret = ACM_ACCESS_DENIED;
     }
-    if (obj != NULL)
-        put_domain(obj);
     return ret;
 }
 
@@ -636,7 +626,7 @@ ste_pre_grant_setup (domid_t id) {
     }
     /* b) check types */
     subj = current->domain;
-    obj = find_domain_by_id(id);
+    obj = find_domain_by_id_noref(id);
 
     if (share_common_type(subj, obj)) {
         cache_result(subj, obj);
@@ -645,8 +635,6 @@ ste_pre_grant_setup (domid_t id) {
         atomic_inc(&ste_bin_pol.gt_denied_count);
         ret = ACM_ACCESS_DENIED;
     }
-    if (obj != NULL)
-        put_domain(obj);
     return ret;
 }
 
diff -r 2301c76e27c4 -r ac17c8457896 xen/common/acm_ops.c
--- a/xen/common/acm_ops.c      Fri Dec  8 18:09:25 2006 -0800
+++ b/xen/common/acm_ops.c      Fri Dec  8 18:13:10 2006 -0800
@@ -108,20 +108,18 @@ long do_acm_op(int cmd, XEN_GUEST_HANDLE
             ssidref = getssid.id.ssidref;
         else if (getssid.get_ssid_by == ACM_GETBY_domainid)
         {
-            struct domain *subj =
find_domain_by_id(getssid.id.domainid);
-            if (!subj)
-            {
-                rc = -ESRCH; /* domain not found */
-                break;
-            }
-            if (subj->ssid == NULL)
-            {
-                put_domain(subj);
+            struct domain *subj =
find_domain_by_id_noref(getssid.id.domainid);
+            if (!subj)
+            {
+                rc = -ESRCH; /* domain not found */
+                break;
+            }
+            if (subj->ssid == NULL)
+            {
                 rc = -ESRCH;
                 break;
             }
             ssidref = ((struct acm_ssid_domain
*)(subj->ssid))->ssidref;
-            put_domain(subj);
         }
         else
         {
@@ -145,20 +143,18 @@ long do_acm_op(int cmd, XEN_GUEST_HANDLE
             ssidref1 = getdecision.id1.ssidref;
         else if (getdecision.get_decision_by1 == ACM_GETBY_domainid)
         {
-            struct domain *subj =
find_domain_by_id(getdecision.id1.domainid);
-            if (!subj)
-            {
-                rc = -ESRCH; /* domain not found */
-                break;
-            }
-            if (subj->ssid == NULL)
-            {
-                put_domain(subj);
+            struct domain *subj =
find_domain_by_id_noref(getdecision.id1.domainid);
+            if (!subj)
+            {
+                rc = -ESRCH; /* domain not found */
+                break;
+            }
+            if (subj->ssid == NULL)
+            {
                 rc = -ESRCH;
                 break;
             }
             ssidref1 = ((struct acm_ssid_domain
*)(subj->ssid))->ssidref;
-            put_domain(subj);
         }
         else
         {
@@ -169,7 +165,7 @@ long do_acm_op(int cmd, XEN_GUEST_HANDLE
             ssidref2 = getdecision.id2.ssidref;
         else if (getdecision.get_decision_by2 == ACM_GETBY_domainid)
         {
-            struct domain *subj =
find_domain_by_id(getdecision.id2.domainid);
+            struct domain *subj =
find_domain_by_id_noref(getdecision.id2.domainid);
             if (!subj)
             {
                 rc = -ESRCH; /* domain not found */
@@ -177,12 +173,10 @@ long do_acm_op(int cmd, XEN_GUEST_HANDLE
             }
             if (subj->ssid == NULL)
             {
-                put_domain(subj);
                 rc = -ESRCH;
                 break;
             }
             ssidref2 = ((struct acm_ssid_domain
*)(subj->ssid))->ssidref;
-            put_domain(subj);
         }
         else
         {
diff -r 2301c76e27c4 -r ac17c8457896 xen/include/acm/acm_hooks.h
--- a/xen/include/acm/acm_hooks.h       Fri Dec  8 18:09:25 2006 -0800
+++ b/xen/include/acm/acm_hooks.h       Fri Dec  8 18:13:10 2006 -0800
@@ -241,7 +241,7 @@ static inline int acm_pre_domctl(struct 
                     __func__);
             return -EACCES;
         }
-        d = find_domain_by_id(op->domain);
+        d = find_domain_by_id_noref(op->domain);
         if (d != NULL) {
             *ssid = d->ssid; /* save for post destroy when d is gone */
             if (*ssid == NULL) {
@@ -252,7 +252,6 @@ static inline int acm_pre_domctl(struct 
             }
             d->ssid = NULL; /* make sure it's not used any more */
              /* no policy-specific hook */
-            put_domain(d);
             ret = 0;
         }
         break;

Attachment: find_domain_by_id_noref_2.patch
Description: find_domain_by_id_noref_2.patch

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