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

RE: [Xen-devel] RE: xenpm: provide core/package cstate residencies



On 2010 07 14 02:43, Ian Jackson wrote:
> Can I ask why these new performance counters are just named with
> numbers (pc3, pc6, pc7, cc3, cc6) ?  Is this normal ?  Should they be
> given more expansive names in the output from xenpm ?

pc stands for package cstate, cc stands for core cstate, it should be succinct. 
Peopole caring about those numbers should understanding them correctly. More 
expansive names in the xenpm output are not really needed.
 
> Also, I just went and looked at xenpm.c since you were adding a lot of
> code to the SIGINT handler, and it seems to me that the code is signal
> unsafe.  It calls printf both from the main program and from the
> signal handler, and if you hit ^C while it's starting up it will
> probably crash in stdio.  This is of course not your fault and not a
> reason not to apply your patch.

Let's discuss how to improve it. Maybe we can move those printf & loops in 
SIGINT to main program and use a flag instead in SIGINT to control. But anyway, 
let's do another incremental patch for this.

> Also, why loop again over the CPU topology, rather than doing the work
> in cpu_topology_func ?

cpu_topology_func just outputs the topology got via hypercall. But here we want 
to calculate the cstate residencs for every core/package in a period, it is not 
suitable to add this into cpu_topology_func.

> Sorry to ask lots of questions but I'm not very familiar with the
> xenpm code and how people use it.

Thanks for review it carefully. :-)

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