Santos, Jose Renato G
2006-Dec-09 04:11 UTC
[Xen-devel] [patch] new version of find_domain_by_id() without reference count [3/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:
3/6: replace find_domain_by_id on common subtree
======================================Signed-off-by: Jose Renato Santos
<jsantos@hpl.hp.com>
# HG changeset patch
# User root@lsnpsvr3.hpl.hp.com
# Node ID 129de3dc1b30aa2a687819b4ebcfd9674575ed4a
# Parent ac17c8457896f63da25c5458b5a76b2ab83139cd
Replace find_domain_by_id() with find_domain_by_id_noref() for common
files
diff -r ac17c8457896 -r 129de3dc1b30 xen/common/domctl.c
--- a/xen/common/domctl.c Fri Dec 8 18:13:10 2006 -0800
+++ b/xen/common/domctl.c Fri Dec 8 18:15:55 2006 -0800
@@ -73,10 +73,9 @@ static inline int is_free_domid(domid_t
if ( dom >= DOMID_FIRST_RESERVED )
return 0;
- if ( (d = find_domain_by_id(dom)) == NULL )
+ if ( (d = find_domain_by_id_noref(dom)) == NULL )
return 1;
- put_domain(d);
return 0;
}
@@ -196,19 +195,16 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
case XEN_DOMCTL_setvcpucontext:
{
- struct domain *d = find_domain_by_id(op->domain);
- ret = -ESRCH;
- if ( d != NULL )
- {
+ struct domain *d = find_domain_by_id_noref(op->domain);
+ ret = -ESRCH;
+ if ( d != NULL )
ret = set_info_guest(d, &op->u.vcpucontext);
- put_domain(d);
- }
}
break;
case XEN_DOMCTL_pausedomain:
{
- struct domain *d = find_domain_by_id(op->domain);
+ struct domain *d = find_domain_by_id_noref(op->domain);
ret = -ESRCH;
if ( d != NULL )
{
@@ -218,14 +214,13 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
domain_pause_by_systemcontroller(d);
ret = 0;
}
- put_domain(d);
}
}
break;
case XEN_DOMCTL_unpausedomain:
{
- struct domain *d = find_domain_by_id(op->domain);
+ struct domain *d = find_domain_by_id_noref(op->domain);
ret = -ESRCH;
if ( d != NULL )
{
@@ -236,7 +231,6 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
domain_unpause_by_systemcontroller(d);
ret = 0;
}
- put_domain(d);
}
}
break;
@@ -305,7 +299,7 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
break;
ret = -ESRCH;
- if ( (d = find_domain_by_id(op->domain)) == NULL )
+ if ( (d = find_domain_by_id_noref(op->domain)) == NULL )
break;
/* Needed, for example, to ensure writable p.t. state is
synced. */
@@ -334,13 +328,12 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
maxvcpu_out:
domain_unpause(d);
- put_domain(d);
}
break;
case XEN_DOMCTL_destroydomain:
{
- struct domain *d = find_domain_by_id(op->domain);
+ struct domain *d = find_domain_by_id_noref(op->domain);
ret = -ESRCH;
if ( d != NULL )
{
@@ -350,7 +343,6 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
domain_kill(d);
ret = 0;
}
- put_domain(d);
}
}
break;
@@ -359,7 +351,7 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
case XEN_DOMCTL_getvcpuaffinity:
{
domid_t dom = op->domain;
- struct domain *d = find_domain_by_id(dom);
+ struct domain *d = find_domain_by_id_noref(dom);
struct vcpu *v;
cpumask_t new_affinity;
@@ -369,11 +361,11 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
ret = -EINVAL;
if ( op->u.vcpuaffinity.vcpu >= MAX_VIRT_CPUS )
- goto vcpuaffinity_out;
+ break;
ret = -ESRCH;
if ( (v = d->vcpu[op->u.vcpuaffinity.vcpu]) == NULL )
- goto vcpuaffinity_out;
+ break;
if ( op->cmd == XEN_DOMCTL_setvcpuaffinity )
{
@@ -387,9 +379,6 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
&op->u.vcpuaffinity.cpumap, &v->cpu_affinity);
ret = 0;
}
-
- vcpuaffinity_out:
- put_domain(d);
}
break;
@@ -398,14 +387,12 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
struct domain *d;
ret = -ESRCH;
- if ( (d = find_domain_by_id(op->domain)) == NULL )
+ if ( (d = find_domain_by_id_noref(op->domain)) == NULL )
break;
ret = sched_adjust(d, &op->u.scheduler_op);
if ( copy_to_guest(u_domctl, op, 1) )
ret = -EFAULT;
-
- put_domain(d);
}
break;
@@ -452,24 +439,24 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
struct vcpu *v;
ret = -ESRCH;
- if ( (d = find_domain_by_id(op->domain)) == NULL )
+ if ( (d = find_domain_by_id_noref(op->domain)) == NULL )
break;
ret = -EINVAL;
if ( op->u.vcpucontext.vcpu >= MAX_VIRT_CPUS )
- goto getvcpucontext_out;
+ break;
ret = -ESRCH;
if ( (v = d->vcpu[op->u.vcpucontext.vcpu]) == NULL )
- goto getvcpucontext_out;
+ break;
ret = -ENODATA;
if ( !test_bit(_VCPUF_initialised, &v->vcpu_flags) )
- goto getvcpucontext_out;
+ break;
ret = -ENOMEM;
if ( (c = xmalloc(struct vcpu_guest_context)) == NULL )
- goto getvcpucontext_out;
+ break;
if ( v != current )
vcpu_pause(v);
@@ -487,9 +474,6 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
if ( copy_to_guest(u_domctl, op, 1) )
ret = -EFAULT;
-
- getvcpucontext_out:
- put_domain(d);
}
break;
@@ -500,16 +484,16 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
struct vcpu_runstate_info runstate;
ret = -ESRCH;
- if ( (d = find_domain_by_id(op->domain)) == NULL )
+ if ( (d = find_domain_by_id_noref(op->domain)) == NULL )
break;
ret = -EINVAL;
if ( op->u.getvcpuinfo.vcpu >= MAX_VIRT_CPUS )
- goto getvcpuinfo_out;
+ break;
ret = -ESRCH;
if ( (v = d->vcpu[op->u.getvcpuinfo.vcpu]) == NULL )
- goto getvcpuinfo_out;
+ break;
vcpu_runstate_get(v, &runstate);
@@ -522,9 +506,6 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
if ( copy_to_guest(u_domctl, op, 1) )
ret = -EFAULT;
-
- getvcpuinfo_out:
- put_domain(d);
}
break;
@@ -534,7 +515,7 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
unsigned long new_max;
ret = -ESRCH;
- d = find_domain_by_id(op->domain);
+ d = find_domain_by_id_noref(op->domain);
if ( d == NULL )
break;
@@ -549,7 +530,6 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
}
spin_unlock(&d->page_alloc_lock);
- put_domain(d);
}
break;
@@ -557,12 +537,11 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
{
struct domain *d;
ret = -ESRCH;
- d = find_domain_by_id(op->domain);
+ d = find_domain_by_id_noref(op->domain);
if ( d != NULL )
{
memcpy(d->handle, op->u.setdomainhandle.handle,
sizeof(xen_domain_handle_t));
- put_domain(d);
ret = 0;
}
}
@@ -572,14 +551,13 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
{
struct domain *d;
ret = -ESRCH;
- d = find_domain_by_id(op->domain);
+ d = find_domain_by_id_noref(op->domain);
if ( d != NULL )
{
if ( op->u.setdebugging.enable )
set_bit(_DOMF_debugging, &d->domain_flags);
else
clear_bit(_DOMF_debugging, &d->domain_flags);
- put_domain(d);
ret = 0;
}
}
@@ -595,7 +573,7 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
break;
ret = -ESRCH;
- d = find_domain_by_id(op->domain);
+ d = find_domain_by_id_noref(op->domain);
if ( d == NULL )
break;
@@ -603,8 +581,6 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
ret = irq_permit_access(d, pirq);
else
ret = irq_deny_access(d, pirq);
-
- put_domain(d);
}
break;
@@ -619,7 +595,7 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
break;
ret = -ESRCH;
- d = find_domain_by_id(op->domain);
+ d = find_domain_by_id_noref(op->domain);
if ( d == NULL )
break;
@@ -627,8 +603,6 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
ret = iomem_permit_access(d, mfn, mfn + nr_mfns - 1);
else
ret = iomem_deny_access(d, mfn, mfn + nr_mfns - 1);
-
- put_domain(d);
}
break;
@@ -637,11 +611,10 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
struct domain *d;
ret = -ESRCH;
- d = find_domain_by_id(op->domain);
+ d = find_domain_by_id_noref(op->domain);
if ( d != NULL )
{
d->time_offset_seconds
op->u.settimeoffset.time_offset_seconds;
- put_domain(d);
ret = 0;
}
}
diff -r ac17c8457896 -r 129de3dc1b30 xen/common/event_channel.c
--- a/xen/common/event_channel.c Fri Dec 8 18:13:10 2006 -0800
+++ b/xen/common/event_channel.c Fri Dec 8 18:15:55 2006 -0800
@@ -111,7 +111,7 @@ static long evtchn_alloc_unbound(evtchn_
else if ( !IS_PRIV(current->domain) )
return -EPERM;
- if ( (d = find_domain_by_id(dom)) == NULL )
+ if ( (d = find_domain_by_id_noref(dom)) == NULL )
return -ESRCH;
spin_lock(&d->evtchn_lock);
@@ -128,8 +128,6 @@ static long evtchn_alloc_unbound(evtchn_
out:
spin_unlock(&d->evtchn_lock);
-
- put_domain(d);
return rc;
}
@@ -149,7 +147,7 @@ static long evtchn_bind_interdomain(evtc
if ( rdom == DOMID_SELF )
rdom = current->domain->domain_id;
- if ( (rd = find_domain_by_id(rdom)) == NULL )
+ if ( (rd = find_domain_by_id_noref(rdom)) == NULL )
return -ESRCH;
/* Avoid deadlock by first acquiring lock of domain with smaller
id. */
@@ -197,8 +195,6 @@ static long evtchn_bind_interdomain(evtc
if ( ld != rd )
spin_unlock(&rd->evtchn_lock);
- put_domain(rd);
-
return rc;
}
@@ -597,7 +593,7 @@ static long evtchn_status(evtchn_status_
else if ( !IS_PRIV(current->domain) )
return -EPERM;
- if ( (d = find_domain_by_id(dom)) == NULL )
+ if ( (d = find_domain_by_id_noref(dom)) == NULL )
return -ESRCH;
spin_lock(&d->evtchn_lock);
@@ -644,7 +640,6 @@ static long evtchn_status(evtchn_status_
out:
spin_unlock(&d->evtchn_lock);
- put_domain(d);
return rc;
}
diff -r ac17c8457896 -r 129de3dc1b30 xen/common/grant_table.c
--- a/xen/common/grant_table.c Fri Dec 8 18:13:10 2006 -0800
+++ b/xen/common/grant_table.c Fri Dec 8 18:15:55 2006 -0800
@@ -123,10 +123,8 @@ __gnttab_map_grant_ref(
return;
}
- if ( unlikely((rd = find_domain_by_id(op->dom)) == NULL) )
- {
- if ( rd != NULL )
- put_domain(rd);
+ if ( unlikely((rd = find_domain_by_id_noref(op->dom)) == NULL) )
+ {
gdprintk(XENLOG_INFO, "Could not find domain %d\n",
op->dom);
op->status = GNTST_bad_domain;
return;
@@ -141,7 +139,6 @@ __gnttab_map_grant_ref(
if ( (lgt->maptrack_limit << 1) > MAPTRACK_MAX_ENTRIES )
{
- put_domain(rd);
gdprintk(XENLOG_INFO, "Maptrack table is at maximum
size.\n");
op->status = GNTST_no_device_space;
return;
@@ -151,7 +148,6 @@ __gnttab_map_grant_ref(
new_mt = alloc_xenheap_pages(lgt->maptrack_order + 1);
if ( new_mt == NULL )
{
- put_domain(rd);
gdprintk(XENLOG_INFO, "No more map handles
available.\n");
op->status = GNTST_no_device_space;
return;
@@ -291,7 +287,6 @@ __gnttab_map_grant_ref(
op->handle = handle;
op->status = GNTST_okay;
- put_domain(rd);
return;
undo_out:
@@ -315,7 +310,6 @@ __gnttab_map_grant_ref(
spin_unlock(&rd->grant_table->lock);
op->status = rc;
put_maptrack_handle(ld->grant_table, handle);
- put_domain(rd);
}
static long
@@ -369,7 +363,7 @@ __gnttab_unmap_grant_ref(
ref = map->ref;
flags = map->flags;
- if ( unlikely((rd = find_domain_by_id(dom)) == NULL) )
+ if ( unlikely((rd = find_domain_by_id_noref(dom)) == NULL) )
{
/* This can happen when a grant is implicitly unmapped. */
gdprintk(XENLOG_INFO, "Could not find domain %d\n", dom);
@@ -450,7 +444,6 @@ __gnttab_unmap_grant_ref(
unmap_out:
op->status = rc;
spin_unlock(&rd->grant_table->lock);
- put_domain(rd);
}
static long
@@ -516,7 +509,7 @@ gnttab_setup_table(
goto out;
}
- if ( unlikely((d = find_domain_by_id(dom)) == NULL) )
+ if ( unlikely((d = find_domain_by_id_noref(dom)) == NULL) )
{
gdprintk(XENLOG_INFO, "Bad domid %d.\n", dom);
op.status = GNTST_bad_domain;
@@ -530,8 +523,6 @@ gnttab_setup_table(
gmfn = gnttab_shared_gmfn(d, d->grant_table, i);
(void)copy_to_guest_offset(op.frame_list, i, &gmfn, 1);
}
-
- put_domain(d);
out:
if ( unlikely(copy_to_guest(uop, &op, 1)) )
@@ -653,7 +644,7 @@ gnttab_transfer(
}
/* Find the target domain. */
- if ( unlikely((e = find_domain_by_id(gop.domid)) == NULL) )
+ if ( unlikely((e = find_domain_by_id_noref(gop.domid)) == NULL)
)
{
gdprintk(XENLOG_INFO, "gnttab_transfer: can''t find
domain
%d\n",
gop.domid);
@@ -681,7 +672,6 @@ gnttab_transfer(
"or is dying (%lx)\n",
e->tot_pages, e->max_pages, gop.ref,
e->domain_flags);
spin_unlock(&e->page_alloc_lock);
- put_domain(e);
page->count_info &= ~(PGC_count_mask|PGC_allocated);
free_domheap_page(page);
gop.status = GNTST_general_error;
@@ -704,8 +694,6 @@ gnttab_transfer(
sha->frame = mfn;
wmb();
sha->flags |= GTF_transfer_completed;
-
- put_domain(e);
gop.status = GNTST_okay;
@@ -866,7 +854,7 @@ __gnttab_copy(
sd = current->domain;
get_knownalive_domain(sd);
}
- else if ( (sd = find_domain_by_id(op->source.domid)) == NULL )
+ else if ( (sd = find_domain_by_id_noref(op->source.domid)) == NULL
)
{
PIN_FAIL(error_out, GNTST_bad_domain,
"couldn''t find %d\n", op->source.domid);
@@ -877,7 +865,7 @@ __gnttab_copy(
dd = current->domain;
get_knownalive_domain(dd);
}
- else if ( (dd = find_domain_by_id(op->dest.domid)) == NULL )
+ else if ( (dd = find_domain_by_id_noref(op->dest.domid)) == NULL )
{
PIN_FAIL(error_out, GNTST_bad_domain,
"couldn''t find %d\n", op->dest.domid);
@@ -946,10 +934,6 @@ __gnttab_copy(
__release_grant_for_copy(sd, op->source.u.ref, 1);
if ( have_d_grant )
__release_grant_for_copy(dd, op->dest.u.ref, 0);
- if ( sd )
- put_domain(sd);
- if ( dd )
- put_domain(dd);
op->status = rc;
}
@@ -1128,7 +1112,7 @@ gnttab_release_mappings(
"flags:(%x) dom:(%hu)\n",
handle, ref, map->flags, map->domid);
- rd = find_domain_by_id(map->domid);
+ rd = find_domain_by_id_noref(map->domid);
if ( rd == NULL )
{
/* Nothing to clear up... */
@@ -1183,8 +1167,6 @@ gnttab_release_mappings(
gnttab_clear_flag(_GTF_reading, &sha->flags);
spin_unlock(&rd->grant_table->lock);
-
- put_domain(rd);
map->flags = 0;
}
diff -r ac17c8457896 -r 129de3dc1b30 xen/common/memory.c
--- a/xen/common/memory.c Fri Dec 8 18:13:10 2006 -0800
+++ b/xen/common/memory.c Fri Dec 8 18:15:55 2006 -0800
@@ -248,12 +248,11 @@ static long translate_gpfn_list(
else if ( !IS_PRIV(current->domain) )
return -EPERM;
- if ( (d = find_domain_by_id(op.domid)) == NULL )
+ if ( (d = find_domain_by_id_noref(op.domid)) == NULL )
return -ESRCH;
if ( !shadow_mode_translate(d) )
{
- put_domain(d);
return -EINVAL;
}
@@ -261,14 +260,12 @@ static long translate_gpfn_list(
{
if ( hypercall_preempt_check() )
{
- put_domain(d);
*progress = i;
return -EAGAIN;
}
if ( unlikely(__copy_from_guest_offset(&gpfn, op.gpfn_list, i,
1)) )
{
- put_domain(d);
return -EFAULT;
}
@@ -276,12 +273,10 @@ static long translate_gpfn_list(
if ( unlikely(__copy_to_guest_offset(op.mfn_list, i, &mfn, 1))
)
{
- put_domain(d);
return -EFAULT;
}
}
- put_domain(d);
return 0;
}
@@ -549,7 +544,7 @@ long do_memory_op(unsigned long cmd, XEN
if ( likely(reservation.domid == DOMID_SELF) )
d = current->domain;
else if ( !IS_PRIV(current->domain) ||
- ((d = find_domain_by_id(reservation.domid)) == NULL)
)
+ ((d = find_domain_by_id_noref(reservation.domid)) =NULL) )
return start_extent;
args.domain = d;
@@ -566,9 +561,6 @@ long do_memory_op(unsigned long cmd, XEN
break;
}
- if ( unlikely(reservation.domid != DOMID_SELF) )
- put_domain(d);
-
rc = args.nr_done;
if ( args.preempted )
@@ -595,13 +587,10 @@ long do_memory_op(unsigned long cmd, XEN
d = current->domain;
else if ( !IS_PRIV(current->domain) )
return -EPERM;
- else if ( (d = find_domain_by_id(domid)) == NULL )
+ else if ( (d = find_domain_by_id_noref(domid)) == NULL )
return -ESRCH;
rc = (op == XENMEM_current_reservation) ? d->tot_pages :
d->max_pages;
-
- if ( unlikely(domid != DOMID_SELF) )
- put_domain(d);
break;
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel