Xen devel - Oct 2006 - [PATCH/RFC] Wrong account for cpus other than 0 on hotplug

Hi,

Is a x86_64 system with multiple CPUs, process accounting is done wrong after
one cpu goes off and on again (i.e.: a CPU bounded process is running
but gets 0% cpu for a time, until situation becomes normal again). 

The following patch is a first attempt to fix it. The struct keeps
zeroed after HYPERVISOR_vcpu_op() is called, thus leading to wrong
results. Accounting seems to be done right without it. However, I''m not
100 % sure we can just poke it out. Comments on this are very welcome.


-- 
Glauber de Oliveira Costa
Red Hat Inc.
"Free as in Freedom"


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

On 13/10/06 6:50 pm, "Glauber de Oliveira Costa"
<gcosta@redhat.com> wrote:
> Is a x86_64 system with multiple CPUs, process accounting is done wrong
after
> one cpu goes off and on again (i.e.: a CPU bounded process is running
> but gets 0% cpu for a time, until situation becomes normal again).
> 
> The following patch is a first attempt to fix it. The struct keeps
> zeroed after HYPERVISOR_vcpu_op() is called, thus leading to wrong
> results. Accounting seems to be done right without it. However,
I''m not
> 100 % sure we can just poke it out. Comments on this are very welcome.
This patch turns off the missing ticks accounting entirely, as the
runstate_info structure will never be updated. Are you sure this appears to
be an x86_64-specific issue?

 -- Keir



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

On Fri, Oct 13, 2006 at 09:33:53PM +0100, Keir Fraser
wrote:
> On 13/10/06 6:50 pm, "Glauber de Oliveira Costa"
<gcosta@redhat.com> wrote:
> 
> 
> This patch turns off the missing ticks accounting entirely, as the
> runstate_info structure will never be updated. Are you sure this appears to
> be an x86_64-specific issue?
That''s why I was almost sure it was wrong, and was expecting comments
:-)
Among all machines we have, it just seem to be happening in a few of
them. Besides being x86_64, they''re 8-way, which can be another sort of
problem.

I just found that the offending code is in the HV itself, and not in the
guest. In arch_do_vcpu_op(), we see that the information is only passed
up to the guest, if the check (current == v) holds, which seems to be
the
true source of it. (taking the check away makes it work).

I can see no reason for this check to be there. If there is one, we
could maybe make sure it holds. (maybe calling set_current(v) on HV
side?)

Your comments on this are , as usual, very welcome ;-)


-- 
Glauber de Oliveira Costa
Red Hat Inc.
"Free as in Freedom"

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

On 13/10/06 10:18 pm, "Glauber de Oliveira Costa"
<gcosta@redhat.com> wrote:
> I just found that the offending code is in the HV itself, and not in the
> guest. In arch_do_vcpu_op(), we see that the information is only passed
> up to the guest, if the check (current == v) holds, which seems to be
> the
> true source of it. (taking the check away makes it work).
Makes sense. CPU0 sets up the area for the AP then immediately tries to read
from the area which isn''t initialised at that point (because of the
check
you point out).
> I can see no reason for this check to be there. If there is one, we
> could maybe make sure it holds. (maybe calling set_current(v) on HV
> side?)
Removing it will change the ABI. Assumption is that AP may have a different
virt->phys translation for that virtual address.

I''ll look into fixing this before 3.0.3 goes out. I think it can be
done
with a fairly small patch.

 -- Keir
> Your comments on this are , as usual, very welcome ;-)


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