Re: [PATCH 1/1] cpupower: monitor: Show how a counter value is exported

[Shuah Khan <skhan@linuxfoundation.org>]

On 8/18/26 03:55, Hemanth Selam wrote:
> "cpupower monitor -l" lists the name, the processor hierarchy level and
> a description of every counter, but not how its value is exported. Some
> counters are a percentage of the time spent in a state, others are
> absolute values with their own unit, for example the Mperf "Freq"
> counter reports MHz and the RAPL zones report micro Joule. Both cannot
> be told apart from the listing.
> 
> Show the value type behind the hierarchy level and document it:
> 
>    [%]   The counter is a percentage of the time spent in the state.
>    [abs] The counter is an absolute value, its unit depends on the
>          counter, MHz for "Freq" or micro Joule for a RAPL zone.
> 
> Before:
> 
>    $ cpupower monitor -l
>    Monitor "Mperf" (3 states) - Might overflow after 922000000 s
>    C0	[T] -> Processor Core not idle
>    Cx	[T] -> Processor Core in an idle state
>    Freq	[T] -> Average Frequency (including boost) in MHz
> 
> After:
> 
>    $ cpupower monitor -l
>    Monitor "Mperf" (3 states) - Might overflow after 922000000 s
>    C0	[T] [%] -> Processor Core not idle
>    Cx	[T] [%] -> Processor Core in an idle state
>    Freq	[T] [abs] -> Average Frequency (including boost) in MHz
> 
> The time granularity part of the ToDo needs a new cstate_t member every
> monitor has to fill in, keep it noted.


> 
> Signed-off-by: Hemanth Selam <hemanth.selam@gmail.com>
> ---
>   tools/power/cpupower/man/cpupower-monitor.1   |  8 +++++++
>   .../utils/idle_monitor/cpupower-monitor.c     | 22 ++++++++++++++++---
>   2 files changed, 27 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/power/cpupower/man/cpupower-monitor.1 b/tools/power/cpupower/man/cpupower-monitor.1
> index 89af019f8dc4..c8008919627c 100644
> --- a/tools/power/cpupower/man/cpupower-monitor.1
> +++ b/tools/power/cpupower/man/cpupower-monitor.1
> @@ -51,6 +51,14 @@ coverage in square brackets:
>   .IP \(bu
>   [M] \-> Machine/Platform wide counter
>   .RE
> +.IP \(bu
> +How the counter is exported, in square brackets behind the hierarchy level:
> +.RS 4
> +.IP \(bu
> +[%] \-> Percentage of the time spent in the state
> +.IP \(bu
> +[abs] \-> Absolute value, the unit depends on the counter
> +.RE
>   .RE
>   .RE
>   .PP
> diff --git a/tools/power/cpupower/utils/idle_monitor/cpupower-monitor.c b/tools/power/cpupower/utils/idle_monitor/cpupower-monitor.c
> index e8b3841d5c0f..376a1e30653a 100644
> --- a/tools/power/cpupower/utils/idle_monitor/cpupower-monitor.c
> +++ b/tools/power/cpupower/utils/idle_monitor/cpupower-monitor.c
> @@ -269,6 +269,20 @@ static void parse_monitor_param(char *param)
>   	avail_monitors = hits;
>   }
>   
> +/*
> + * A monitor exports a counter either as a percentage of the time spent
> + * in the state or as an absolute value. Tell both apart, so that the
> + * numbers a measurement run prints can be interpreted.
> + */
> +static const char *value_abbr(const cstate_t *state)
> +{
> +	if (state->get_count_percent)
> +		return "%";
> +	if (state->get_count)
> +		return "abs";
> +	return "?";
> +}

Way too much complexity to do this.

> +
>   void list_monitors(void)
>   {
>   	unsigned int mon;
> @@ -284,10 +298,12 @@ void list_monitors(void)
>   		for (state = 0; state < monitors[mon]->hw_states_num; state++) {
>   			s = monitors[mon]->hw_states[state];
>   			/*
> -			 * ToDo show more state capabilities:
> -			 * percent, time (granlarity)
> +			 * ToDo show the time granularity of a counter, this
> +			 * needs a new cstate_t member every monitor has to
> +			 * fill in.
>   			 */
> -			printf("%s\t[%c] -> %s\n", s.name, range_abbr[s.range],
> +			printf("%s\t[%c] [%s] -> %s\n", s.name,
> +			       range_abbr[s.range], value_abbr(&s),

Find a simpler way to do this instead of adding a new routine.
Hint: you can check it right here.

>   			       gettext(s.desc));
>   		}
>   	}

thanks,
-- Shuah