Re: [PATCH 1/2] Modify cpupower to schedule itself on cores it is reading MSRs from

From: Thomas Renninger
Date: Sat Oct 05 2019 - 08:40:54 EST


Hi,

On Wednesday, October 2, 2019 4:45:03 PM CEST Natarajan, Janakarajan wrote:
> On 9/27/19 4:48 PM, Thomas Renninger wrote:
>
> > On Friday, September 27, 2019 6:07:56 PM CEST Natarajan, Janakarajan
> > wrote:
>
> >> On 9/18/2019 11:34 AM, Natarajan, Janakarajan wrote:

> On a 256 logical-cpu Rome system we see C0 value from cpupower output go
> from 0.01 to ~(0.1 to 1.00)
>
> for all cpus with the 1st patch.
>
> However, this goes down to ~0.01 when we use the RDPRU instruction
> (which can be used to get
>
> APERF/MPERF from CPL > 0) and avoid using the msr module (patch 2).

And this one only exists on latest AMD cpus, right?

> However, for systems that provide an instruction to get register values
> from userspace, would a command-line parameter be acceptable?

Parameter sounds like a good idea. In fact, there already is such a paramter.
cpupower monitor --help
-c
Schedule the process on every core before starting and ending
measuring. This could be needed for the Idle_Stats monitor when no other MSR
based monitor (has to be run on the core that is measured) is run in parallel.
This is to wake up the processors from deeper sleep states and let the kernel
reaccount its cpuidle (C-state) information before reading the cpuidle timings
from sysfs.

Best is you exchange the order of your patches. The 2nd looks rather straight
forward and you can add my reviewed-by.

If you still need adjustings with -c param, they can be discussed separately.
It would also be nice to mention in which case it makes sense to use it in the
manpage or advantages/drawbacks if you don't.

Thanks!

Thomas