Xen devel - Dec 2006 - [patch] new version of find_domain_by_id() without reference count [0/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.

There are a total of 6 patches

1/6: add new function find_domain_by_id_noref()
2/6: replace find_domain_by_id on acm subtree
3/6: replace find_domain_by_id on common subtree
4/6: replace find_domain_by_id on arch/x86
5/6: replace find_domain_by_id on arch/powerpc
6/6: replace find_domain_by_id on arch/ia64

Regards

Renato



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

On 9/12/06 4:09 am, "Santos, Jose Renato G"
<joserenato.santos@hp.com>
wrote:
> 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.
It can''t go in until RCU is merged and that won''t happen for
3.0.4 anyway.

When you post these again after 3.0.4 (which I''m sure you''ll
have to do) I''d
actually like the names as follows:
 1. Rename find_domain_by_id() to get_domain_by_id().
 2. Call the new function find_domain_by_id().

This has the obvious disadvantage of an old/new name clash so we effectively
change semantics of an existing function. *But* the new names are better
imo: get_domain_by_id() more obviously pairs with put_domain(), and
find_domain_by_id_noref() is ugly and this rename allows us to drop the
_noref suffix unambiguously.

I''d be interested to know what this does for find/get_domain_by_id()
percentages in your network profiles.

 -- Keir


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

> -----Original Message-----
> From: Keir Fraser [mailto:keir@xensource.com] 
> Sent: Saturday, December 09, 2006 1:29 AM
> To: Santos, Jose Renato G; xen-devel@lists.xensource.com
> Cc: Turner, Yoshio; G John Janakiraman
> Subject: Re: [Xen-devel] [patch] new version of 
> find_domain_by_id() without reference count [0/6]
> 
> On 9/12/06 4:09 am, "Santos, Jose Renato G"
<joserenato.santos@hp.com>
> wrote:
> 
> > 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.
> 
> It can''t go in until RCU is merged and that won''t happen
for
> 3.0.4 anyway.
> 
> When you post these again after 3.0.4 (which I''m sure
you''ll
> have to do) I''d actually like the names as follows:
>  1. Rename find_domain_by_id() to get_domain_by_id().
>  2. Call the new function find_domain_by_id().
> 
> This has the obvious disadvantage of an old/new name clash so 
> we effectively change semantics of an existing function. 
> *But* the new names are better
> imo: get_domain_by_id() more obviously pairs with put_domain(), and
> find_domain_by_id_noref() is ugly and this rename allows us 
> to drop the _noref suffix unambiguously.
> 
  Yes, I also thought we needed better names but my thinking was that
  we could change the names in separate patches after we were sure
  no code was still using find_domain_by_id() with the old semantics.
  Since I can only run and compile the x86 version I was afraid that
  I might have missed some code using find_domain_by_id() in the
  other archs. But I guess I was being too conservative. It is 
  probably fine to change the names as you suggested in the next
  patch. 
> I''d be interested to know what this does for 
> find/get_domain_by_id() percentages in your network profiles.
> 
  For TCP TX find_domain_by_id() cost is reduced from 3.05% of the
  total CPU cycles to 0.19% in my machine using the new version. 
  I have not run a RX benchmark yet, but I can get those numbers
  Monday.

  Regards

  Renato
>  -- Keir
> 
> 
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel