| Message ID | 20200810144330.75613-1-darcari@redhat.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Len Brown |
| Headers | show |
| Series | tools/power turbostat: fix output formatting for ACPI CST enumeration | expand |
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 <darcari@redhat.com> > Cc: Len Brown <lenb@kernel.org> > Cc: linux-kernel@vger.kernel.org > --- > 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 >
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))
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 <darcari@redhat.com> Cc: Len Brown <lenb@kernel.org> Cc: linux-kernel@vger.kernel.org --- tools/power/x86/turbostat/turbostat.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)