turbostat formatting is broken with ACPI CST for enumeration. The
problem is that the CX_ACPI% is eight characters long which does not
work with tab formatting. One simple solution is to remove the underbar
from the state name such that C1_ACPI will be displayed as C1ACPI.
Signed-off-by: David Arcari <[email protected]>
Cc: Len Brown <[email protected]>
Cc: [email protected]
---
tools/power/x86/turbostat/turbostat.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index 33b370865d16..5f074879cc0a 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -3474,6 +3474,20 @@ int has_config_tdp(unsigned int family, unsigned int model)
}
}
+static void
+remove_underbar(char *s)
+{
+ char *to = s;
+
+ while (*s) {
+ if (*s != '_')
+ *to++ = *s;
+ s++;
+ }
+
+ *to = 0;
+}
+
static void
dump_cstate_pstate_config_info(unsigned int family, unsigned int model)
{
@@ -3559,6 +3573,8 @@ dump_sysfs_cstate_config(void)
*sp = '\0';
fclose(input);
+ remove_underbar(name_buf);
+
sprintf(path, "/sys/devices/system/cpu/cpu%d/cpuidle/state%d/desc",
base_cpu, state);
input = fopen(path, "r");
@@ -5597,6 +5613,8 @@ void probe_sysfs(void)
*sp = '%';
*(sp + 1) = '\0';
+ remove_underbar(name_buf);
+
fclose(input);
sprintf(path, "cpuidle/state%d/time", state);
@@ -5624,6 +5642,8 @@ void probe_sysfs(void)
*sp = '\0';
fclose(input);
+ remove_underbar(name_buf);
+
sprintf(path, "cpuidle/state%d/usage", state);
if (is_deferred_skip(name_buf))
--
2.27.0
Hi,
Just want to make sure that this doesn't get lost.
Please let me know if you feel there is a better approach.
Thanks,
-Dave
On 8/10/20 10:43 AM, David Arcari wrote:
> turbostat formatting is broken with ACPI CST for enumeration. The
> problem is that the CX_ACPI% is eight characters long which does not
> work with tab formatting. One simple solution is to remove the underbar
> from the state name such that C1_ACPI will be displayed as C1ACPI.
>
> Signed-off-by: David Arcari <[email protected]>
> Cc: Len Brown <[email protected]>
> Cc: [email protected]
> ---
> tools/power/x86/turbostat/turbostat.c | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
> index 33b370865d16..5f074879cc0a 100644
> --- a/tools/power/x86/turbostat/turbostat.c
> +++ b/tools/power/x86/turbostat/turbostat.c
> @@ -3474,6 +3474,20 @@ int has_config_tdp(unsigned int family, unsigned int model)
> }
> }
>
> +static void
> +remove_underbar(char *s)
> +{
> + char *to = s;
> +
> + while (*s) {
> + if (*s != '_')
> + *to++ = *s;
> + s++;
> + }
> +
> + *to = 0;
> +}
> +
> static void
> dump_cstate_pstate_config_info(unsigned int family, unsigned int model)
> {
> @@ -3559,6 +3573,8 @@ dump_sysfs_cstate_config(void)
> *sp = '\0';
> fclose(input);
>
> + remove_underbar(name_buf);
> +
> sprintf(path, "/sys/devices/system/cpu/cpu%d/cpuidle/state%d/desc",
> base_cpu, state);
> input = fopen(path, "r");
> @@ -5597,6 +5613,8 @@ void probe_sysfs(void)
> *sp = '%';
> *(sp + 1) = '\0';
>
> + remove_underbar(name_buf);
> +
> fclose(input);
>
> sprintf(path, "cpuidle/state%d/time", state);
> @@ -5624,6 +5642,8 @@ void probe_sysfs(void)
> *sp = '\0';
> fclose(input);
>
> + remove_underbar(name_buf);
> +
> sprintf(path, "cpuidle/state%d/usage", state);
>
> if (is_deferred_skip(name_buf))
>
Hi Dave,
I think this is fine.
Indeed, I actually went ahead and applied it a week or so ago.
the only alternative that I can think of is actually shortening the
ACPI C-state names in the intel_idle driver -- which is still an
option. It would not be the first time we have tweaked the names used
in that driver to make tools more happy...
My apology for neglecting to send you an ACK.
I had intended to send my queued series to the list, which would
suffice for all the ACKs, but that send and the subsequent push got
delayed by this and that. So I'll try to ack as I go, so it is clear
at any time where a patch stands.
thanks!
Len Brown, Intel Open Source Technology Center
Hi Len,
Thanks for the quick turn around. My apologies for not checking the tree before
sending a follow-up email.
If you decide you prefer to change intel_idle - I'd be happy to do the work if
you'd like. Just let me know.
Thanks,
-Dave
On 8/21/20 2:23 PM, Len Brown wrote:
> Hi Dave,
>
> I think this is fine.
> Indeed, I actually went ahead and applied it a week or so ago.
>
> the only alternative that I can think of is actually shortening the
> ACPI C-state names in the intel_idle driver -- which is still an
> option. It would not be the first time we have tweaked the names used
> in that driver to make tools more happy...
>
> My apology for neglecting to send you an ACK.
> I had intended to send my queued series to the list, which would
> suffice for all the ACKs, but that send and the subsequent push got
> delayed by this and that. So I'll try to ack as I go, so it is clear
> at any time where a patch stands.
>
> thanks!
>
> Len Brown, Intel Open Source Technology Center
>