linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* turbostat 19.08.31 is available
@ 2019-08-31 19:34 Len Brown
  2019-08-31 19:34 ` [PATCH 01/19] tools/power x86_energy_perf_policy: Fix "uninitialized variable" warnings at -O2 Len Brown
  0 siblings, 1 reply; 20+ messages in thread
From: Len Brown @ 2019-08-31 19:34 UTC (permalink / raw)
  To: linux-pm

Thanks for the patches everybody!

You can download the latest turbostat (and x86_energy_perf_policy) for testing here:

git://git.kernel.org/pub/scm/linux/kernel/git/lenb/linux.git turbostat

 tools/power/x86/turbostat/Makefile                 |   3 +-
 tools/power/x86/turbostat/turbostat.c              | 101 ++++++++++++++-------
 tools/power/x86/x86_energy_perf_policy/Makefile    |   3 +-
 .../x86_energy_perf_policy.8                       |   2 +-
 .../x86_energy_perf_policy.c                       |  28 +++---
 5 files changed, 90 insertions(+), 47 deletions(-)

[PATCH 01/19] tools/power x86_energy_perf_policy: Fix "uninitialized variable" warnings at -O2
[PATCH 02/19] tools/power/x86: Enable compiler optimisations and Fortify by default
[PATCH 03/19] tools/power: Fix typo in man page
[PATCH 04/19] tools/power x86_energy_perf_policy: Fix argument parsing
[PATCH 05/19] tools/power turbostat: remove duplicate pc10 column
[PATCH 06/19] tools/power turbostat: Make interval calculation per thread to reduce jitter
[PATCH 07/19] tools/power turbostat: fix leak of file descriptor on error return path
[PATCH 08/19] tools/power turbostat: fix file descriptor leaks
[PATCH 09/19] tools/power turbostat: fix buffer overrun

Prarit, Naoya,
For now, I went with your initial 1-line fix,
as I currently don't have an appetite for a 400-line patch
when 1-line spending 1KB / CPU will float the boat.

Yeah, it is a bummer that this "bundle formatted output into
a single buffered write" optimiation is even necessary...
you'd think that stdio could be smarter to automatically buffer
multiple prints into a single write...

[PATCH 10/19] tools/power turbostat: add Jacobsville support
[PATCH 11/19] tools/power turbostat: Fix Haswell Core systems
[PATCH 12/19] tools/power turbostat: rename has_hsw_msrs()
[PATCH 13/19] tools/power turbostat: Add Ice Lake NNPI support
[PATCH 14/19] tools/power turbostat: read from pipes too
[PATCH 15/19] tools/power turbostat: do not enforce 1ms
[PATCH 16/19] tools/power turbostat: Fix CPU%C1 display value

[PATCH 17/19] tools/power turbostat: Fix caller parameter of get_tdp_amd()
[PATCH 18/19] tools/power turbostat: Add support for Hygon Fam 18h (Dhyana) RAPL

Although these were marked RFC instead of PATCH, I applied them,
because they were otherwise properly signed, and seemed ready.

[PATCH 19/19] tools/power turbostat: update version number

cheers,
-Len


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 01/19] tools/power x86_energy_perf_policy: Fix "uninitialized variable" warnings at -O2
  2019-08-31 19:34 turbostat 19.08.31 is available Len Brown
@ 2019-08-31 19:34 ` Len Brown
  2019-08-31 19:34   ` [PATCH 02/19] tools/power/x86: Enable compiler optimisations and Fortify by default Len Brown
                     ` (17 more replies)
  0 siblings, 18 replies; 20+ messages in thread
From: Len Brown @ 2019-08-31 19:34 UTC (permalink / raw)
  To: linux-pm; +Cc: Ben Hutchings, Len Brown

From: Ben Hutchings <ben@decadent.org.uk>

x86_energy_perf_policy first uses __get_cpuid() to check the maximum
CPUID level and exits if it is too low.  It then assumes that later
calls will succeed (which I think is architecturally guaranteed).  It
also assumes that CPUID works at all (which is not guaranteed on
x86_32).

If optimisations are enabled, gcc warns about potentially
uninitialized variables.  Fix this by adding an exit-on-error after
every call to __get_cpuid() instead of just checking the maximum
level.

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 .../x86_energy_perf_policy.c                  | 26 +++++++++++--------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/tools/power/x86/x86_energy_perf_policy/x86_energy_perf_policy.c b/tools/power/x86/x86_energy_perf_policy/x86_energy_perf_policy.c
index 34a796b303fe..7663abef51e9 100644
--- a/tools/power/x86/x86_energy_perf_policy/x86_energy_perf_policy.c
+++ b/tools/power/x86/x86_energy_perf_policy/x86_energy_perf_policy.c
@@ -1259,6 +1259,15 @@ void probe_dev_msr(void)
 		if (system("/sbin/modprobe msr > /dev/null 2>&1"))
 			err(-5, "no /dev/cpu/0/msr, Try \"# modprobe msr\" ");
 }
+
+static void get_cpuid_or_exit(unsigned int leaf,
+			     unsigned int *eax, unsigned int *ebx,
+			     unsigned int *ecx, unsigned int *edx)
+{
+	if (!__get_cpuid(leaf, eax, ebx, ecx, edx))
+		errx(1, "Processor not supported\n");
+}
+
 /*
  * early_cpuid()
  * initialize turbo_is_enabled, has_hwp, has_epb
@@ -1266,15 +1275,10 @@ void probe_dev_msr(void)
  */
 void early_cpuid(void)
 {
-	unsigned int eax, ebx, ecx, edx, max_level;
+	unsigned int eax, ebx, ecx, edx;
 	unsigned int fms, family, model;
 
-	__get_cpuid(0, &max_level, &ebx, &ecx, &edx);
-
-	if (max_level < 6)
-		errx(1, "Processor not supported\n");
-
-	__get_cpuid(1, &fms, &ebx, &ecx, &edx);
+	get_cpuid_or_exit(1, &fms, &ebx, &ecx, &edx);
 	family = (fms >> 8) & 0xf;
 	model = (fms >> 4) & 0xf;
 	if (family == 6 || family == 0xf)
@@ -1288,7 +1292,7 @@ void early_cpuid(void)
 		bdx_highest_ratio = msr & 0xFF;
 	}
 
-	__get_cpuid(0x6, &eax, &ebx, &ecx, &edx);
+	get_cpuid_or_exit(0x6, &eax, &ebx, &ecx, &edx);
 	turbo_is_enabled = (eax >> 1) & 1;
 	has_hwp = (eax >> 7) & 1;
 	has_epb = (ecx >> 3) & 1;
@@ -1306,7 +1310,7 @@ void parse_cpuid(void)
 
 	eax = ebx = ecx = edx = 0;
 
-	__get_cpuid(0, &max_level, &ebx, &ecx, &edx);
+	get_cpuid_or_exit(0, &max_level, &ebx, &ecx, &edx);
 
 	if (ebx == 0x756e6547 && edx == 0x49656e69 && ecx == 0x6c65746e)
 		genuine_intel = 1;
@@ -1315,7 +1319,7 @@ void parse_cpuid(void)
 		fprintf(stderr, "CPUID(0): %.4s%.4s%.4s ",
 			(char *)&ebx, (char *)&edx, (char *)&ecx);
 
-	__get_cpuid(1, &fms, &ebx, &ecx, &edx);
+	get_cpuid_or_exit(1, &fms, &ebx, &ecx, &edx);
 	family = (fms >> 8) & 0xf;
 	model = (fms >> 4) & 0xf;
 	stepping = fms & 0xf;
@@ -1340,7 +1344,7 @@ void parse_cpuid(void)
 		errx(1, "CPUID: no MSR");
 
 
-	__get_cpuid(0x6, &eax, &ebx, &ecx, &edx);
+	get_cpuid_or_exit(0x6, &eax, &ebx, &ecx, &edx);
 	/* turbo_is_enabled already set */
 	/* has_hwp already set */
 	has_hwp_notify = eax & (1 << 8);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 02/19] tools/power/x86: Enable compiler optimisations and Fortify by default
  2019-08-31 19:34 ` [PATCH 01/19] tools/power x86_energy_perf_policy: Fix "uninitialized variable" warnings at -O2 Len Brown
@ 2019-08-31 19:34   ` Len Brown
  2019-08-31 19:34   ` [PATCH 03/19] tools/power: Fix typo in man page Len Brown
                     ` (16 subsequent siblings)
  17 siblings, 0 replies; 20+ messages in thread
From: Len Brown @ 2019-08-31 19:34 UTC (permalink / raw)
  To: linux-pm; +Cc: Ben Hutchings, Len Brown

From: Ben Hutchings <ben@decadent.org.uk>

Compiling without optimisations is silly, especially since some
warnings depend on the optimiser.  Use -O2.

Fortify adds warnings for unchecked I/O (among other things), which
seems to be a good idea for user-space code.  Enable that too.

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 tools/power/x86/turbostat/Makefile              | 3 ++-
 tools/power/x86/x86_energy_perf_policy/Makefile | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/power/x86/turbostat/Makefile b/tools/power/x86/turbostat/Makefile
index 045f5f7d68ab..13f1e8b9ac52 100644
--- a/tools/power/x86/turbostat/Makefile
+++ b/tools/power/x86/turbostat/Makefile
@@ -9,9 +9,10 @@ ifeq ("$(origin O)", "command line")
 endif
 
 turbostat : turbostat.c
-override CFLAGS +=	-Wall -I../../../include
+override CFLAGS +=	-O2 -Wall -I../../../include
 override CFLAGS +=	-DMSRHEADER='"../../../../arch/x86/include/asm/msr-index.h"'
 override CFLAGS +=	-DINTEL_FAMILY_HEADER='"../../../../arch/x86/include/asm/intel-family.h"'
+override CFLAGS +=	-D_FORTIFY_SOURCE=2
 
 %: %.c
 	@mkdir -p $(BUILD_OUTPUT)
diff --git a/tools/power/x86/x86_energy_perf_policy/Makefile b/tools/power/x86/x86_energy_perf_policy/Makefile
index 1fdeef864e7c..666b325a62a2 100644
--- a/tools/power/x86/x86_energy_perf_policy/Makefile
+++ b/tools/power/x86/x86_energy_perf_policy/Makefile
@@ -9,8 +9,9 @@ ifeq ("$(origin O)", "command line")
 endif
 
 x86_energy_perf_policy : x86_energy_perf_policy.c
-override CFLAGS +=	-Wall -I../../../include
+override CFLAGS +=	-O2 -Wall -I../../../include
 override CFLAGS +=	-DMSRHEADER='"../../../../arch/x86/include/asm/msr-index.h"'
+override CFLAGS +=	-D_FORTIFY_SOURCE=2
 
 %: %.c
 	@mkdir -p $(BUILD_OUTPUT)
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 03/19] tools/power: Fix typo in man page
  2019-08-31 19:34 ` [PATCH 01/19] tools/power x86_energy_perf_policy: Fix "uninitialized variable" warnings at -O2 Len Brown
  2019-08-31 19:34   ` [PATCH 02/19] tools/power/x86: Enable compiler optimisations and Fortify by default Len Brown
@ 2019-08-31 19:34   ` Len Brown
  2019-08-31 19:34   ` [PATCH 04/19] tools/power x86_energy_perf_policy: Fix argument parsing Len Brown
                     ` (15 subsequent siblings)
  17 siblings, 0 replies; 20+ messages in thread
From: Len Brown @ 2019-08-31 19:34 UTC (permalink / raw)
  To: linux-pm; +Cc: Matt Lupfer, Len Brown

From: Matt Lupfer <mlupfer@ddn.com>

From context, we mean EPB (Enegry Performance Bias).

Signed-off-by: Matt Lupfer <mlupfer@ddn.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 tools/power/x86/x86_energy_perf_policy/x86_energy_perf_policy.8 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/power/x86/x86_energy_perf_policy/x86_energy_perf_policy.8 b/tools/power/x86/x86_energy_perf_policy/x86_energy_perf_policy.8
index 17db1c3af4d0..78c6361898b1 100644
--- a/tools/power/x86/x86_energy_perf_policy/x86_energy_perf_policy.8
+++ b/tools/power/x86/x86_energy_perf_policy/x86_energy_perf_policy.8
@@ -40,7 +40,7 @@ in the same processor package.
 Hardware P-States (HWP) are effectively an expansion of hardware
 P-state control from the opportunistic turbo-mode P-state range
 to include the entire range of available P-states.
-On Broadwell Xeon, the initial HWP implementation, EBP influenced HWP.
+On Broadwell Xeon, the initial HWP implementation, EPB influenced HWP.
 That influence was removed in subsequent generations,
 where it was moved to the
 Energy_Performance_Preference (EPP) field in
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 04/19] tools/power x86_energy_perf_policy: Fix argument parsing
  2019-08-31 19:34 ` [PATCH 01/19] tools/power x86_energy_perf_policy: Fix "uninitialized variable" warnings at -O2 Len Brown
  2019-08-31 19:34   ` [PATCH 02/19] tools/power/x86: Enable compiler optimisations and Fortify by default Len Brown
  2019-08-31 19:34   ` [PATCH 03/19] tools/power: Fix typo in man page Len Brown
@ 2019-08-31 19:34   ` Len Brown
  2019-08-31 19:34   ` [PATCH 05/19] tools/power turbostat: remove duplicate pc10 column Len Brown
                     ` (14 subsequent siblings)
  17 siblings, 0 replies; 20+ messages in thread
From: Len Brown @ 2019-08-31 19:34 UTC (permalink / raw)
  To: linux-pm; +Cc: Zephaniah E. Loss-Cutler-Hull, Len Brown

From: "Zephaniah E. Loss-Cutler-Hull" <zephaniah@gmail.com>

The -w argument in x86_energy_perf_policy currently triggers an
unconditional segfault.

This is because the argument string reads: "+a:c:dD:E:e:f:m:M:rt:u:vw" and
yet the argument handler expects an argument.

When parse_optarg_string is called with a null argument, we then proceed to
crash in strncmp, not horribly friendly.

The man page describes -w as taking an argument, the long form
(--hwp-window) is correctly marked as taking a required argument, and the
code expects it.

As such, this patch simply marks the short form (-w) as requiring an
argument.

Signed-off-by: Zephaniah E. Loss-Cutler-Hull <zephaniah@gmail.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 tools/power/x86/x86_energy_perf_policy/x86_energy_perf_policy.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/power/x86/x86_energy_perf_policy/x86_energy_perf_policy.c b/tools/power/x86/x86_energy_perf_policy/x86_energy_perf_policy.c
index 7663abef51e9..3fe1eed900d4 100644
--- a/tools/power/x86/x86_energy_perf_policy/x86_energy_perf_policy.c
+++ b/tools/power/x86/x86_energy_perf_policy/x86_energy_perf_policy.c
@@ -545,7 +545,7 @@ void cmdline(int argc, char **argv)
 
 	progname = argv[0];
 
-	while ((opt = getopt_long_only(argc, argv, "+a:c:dD:E:e:f:m:M:rt:u:vw",
+	while ((opt = getopt_long_only(argc, argv, "+a:c:dD:E:e:f:m:M:rt:u:vw:",
 				long_options, &option_index)) != -1) {
 		switch (opt) {
 		case 'a':
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 05/19] tools/power turbostat: remove duplicate pc10 column
  2019-08-31 19:34 ` [PATCH 01/19] tools/power x86_energy_perf_policy: Fix "uninitialized variable" warnings at -O2 Len Brown
                     ` (2 preceding siblings ...)
  2019-08-31 19:34   ` [PATCH 04/19] tools/power x86_energy_perf_policy: Fix argument parsing Len Brown
@ 2019-08-31 19:34   ` Len Brown
  2019-08-31 19:34   ` [PATCH 06/19] tools/power turbostat: Make interval calculation per thread to reduce jitter Len Brown
                     ` (13 subsequent siblings)
  17 siblings, 0 replies; 20+ messages in thread
From: Len Brown @ 2019-08-31 19:34 UTC (permalink / raw)
  To: linux-pm; +Cc: Len Brown, Naoya Horiguchi

From: Len Brown <len.brown@intel.com>

Remove the duplicate pc10 column.

Fixes: be0e54c4ebbf ("turbostat: Build-in "Low Power Idle" counters support")
Reported-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 tools/power/x86/turbostat/turbostat.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index 75fc4fb9901c..90f7e8b4d4d4 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -849,7 +849,6 @@ int dump_counters(struct thread_data *t, struct core_data *c,
 		outp += sprintf(outp, "pc8: %016llX\n", p->pc8);
 		outp += sprintf(outp, "pc9: %016llX\n", p->pc9);
 		outp += sprintf(outp, "pc10: %016llX\n", p->pc10);
-		outp += sprintf(outp, "pc10: %016llX\n", p->pc10);
 		outp += sprintf(outp, "cpu_lpi: %016llX\n", p->cpu_lpi);
 		outp += sprintf(outp, "sys_lpi: %016llX\n", p->sys_lpi);
 		outp += sprintf(outp, "Joules PKG: %0X\n", p->energy_pkg);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 06/19] tools/power turbostat: Make interval calculation per thread to reduce jitter
  2019-08-31 19:34 ` [PATCH 01/19] tools/power x86_energy_perf_policy: Fix "uninitialized variable" warnings at -O2 Len Brown
                     ` (3 preceding siblings ...)
  2019-08-31 19:34   ` [PATCH 05/19] tools/power turbostat: remove duplicate pc10 column Len Brown
@ 2019-08-31 19:34   ` Len Brown
  2019-08-31 19:34   ` [PATCH 07/19] tools/power turbostat: fix leak of file descriptor on error return path Len Brown
                     ` (12 subsequent siblings)
  17 siblings, 0 replies; 20+ messages in thread
From: Len Brown @ 2019-08-31 19:34 UTC (permalink / raw)
  To: linux-pm; +Cc: Yazen Ghannam, Len Brown

From: Yazen Ghannam <yazen.ghannam@amd.com>

Turbostat currently normalizes TSC and other values by dividing by an
interval. This interval is the delta between the start of one global
(all counters on all CPUs) sampling and the start of another. However,
this introduces a lot of jitter into the data.

In order to reduce jitter, the interval calculation should be based on
timestamps taken per thread and close to the start of the thread's
sampling.

Define a per thread time value to hold the delta between samples taken
on the thread.

Use the timestamp taken at the beginning of sampling to calculate the
delta.

Move the thread's beginning timestamp to after the CPU migration to
avoid jitter due to the migration.

Use the global time delta for the average time delta.

Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 tools/power/x86/turbostat/turbostat.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index 90f7e8b4d4d4..02813a2a8ffd 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -166,6 +166,7 @@ size_t cpu_present_setsize, cpu_affinity_setsize, cpu_subset_size;
 struct thread_data {
 	struct timeval tv_begin;
 	struct timeval tv_end;
+	struct timeval tv_delta;
 	unsigned long long tsc;
 	unsigned long long aperf;
 	unsigned long long mperf;
@@ -910,7 +911,7 @@ int format_counters(struct thread_data *t, struct core_data *c,
 	if (DO_BIC(BIC_TOD))
 		outp += sprintf(outp, "%10ld.%06ld\t", t->tv_end.tv_sec, t->tv_end.tv_usec);
 
-	interval_float = tv_delta.tv_sec + tv_delta.tv_usec/1000000.0;
+	interval_float = t->tv_delta.tv_sec + t->tv_delta.tv_usec/1000000.0;
 
 	tsc = t->tsc * tsc_tweak;
 
@@ -1308,6 +1309,7 @@ delta_thread(struct thread_data *new, struct thread_data *old,
 	 * over-write old w/ new so we can print end of interval values
 	 */
 
+	timersub(&new->tv_begin, &old->tv_begin, &old->tv_delta);
 	old->tv_begin = new->tv_begin;
 	old->tv_end = new->tv_end;
 
@@ -1403,6 +1405,8 @@ void clear_counters(struct thread_data *t, struct core_data *c, struct pkg_data
 	t->tv_begin.tv_usec = 0;
 	t->tv_end.tv_sec = 0;
 	t->tv_end.tv_usec = 0;
+	t->tv_delta.tv_sec = 0;
+	t->tv_delta.tv_usec = 0;
 
 	t->tsc = 0;
 	t->aperf = 0;
@@ -1572,6 +1576,9 @@ void compute_average(struct thread_data *t, struct core_data *c,
 
 	for_all_cpus(sum_counters, t, c, p);
 
+	/* Use the global time delta for the average. */
+	average.threads.tv_delta = tv_delta;
+
 	average.threads.tsc /= topo.num_cpus;
 	average.threads.aperf /= topo.num_cpus;
 	average.threads.mperf /= topo.num_cpus;
@@ -1761,13 +1768,13 @@ int get_counters(struct thread_data *t, struct core_data *c, struct pkg_data *p)
 	struct msr_counter *mp;
 	int i;
 
-	gettimeofday(&t->tv_begin, (struct timezone *)NULL);
-
 	if (cpu_migrate(cpu)) {
 		fprintf(outf, "Could not migrate to CPU %d\n", cpu);
 		return -1;
 	}
 
+	gettimeofday(&t->tv_begin, (struct timezone *)NULL);
+
 	if (first_counter_read)
 		get_apic_id(t);
 retry:
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 07/19] tools/power turbostat: fix leak of file descriptor on error return path
  2019-08-31 19:34 ` [PATCH 01/19] tools/power x86_energy_perf_policy: Fix "uninitialized variable" warnings at -O2 Len Brown
                     ` (4 preceding siblings ...)
  2019-08-31 19:34   ` [PATCH 06/19] tools/power turbostat: Make interval calculation per thread to reduce jitter Len Brown
@ 2019-08-31 19:34   ` Len Brown
  2019-08-31 19:34   ` [PATCH 08/19] tools/power turbostat: fix file descriptor leaks Len Brown
                     ` (11 subsequent siblings)
  17 siblings, 0 replies; 20+ messages in thread
From: Len Brown @ 2019-08-31 19:34 UTC (permalink / raw)
  To: linux-pm; +Cc: Colin Ian King, Len Brown

From: Colin Ian King <colin.king@canonical.com>

Currently the error return path does not close the file fp and leaks
a file descriptor. Fix this by closing the file.

Fixes: 5ea7647b333f ("tools/power turbostat: Warn on bad ACPI LPIT data")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 tools/power/x86/turbostat/turbostat.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index 02813a2a8ffd..41cf1206273c 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -2944,6 +2944,7 @@ int snapshot_sys_lpi_us(void)
 	if (retval != 1) {
 		fprintf(stderr, "Disabling Low Power Idle System output\n");
 		BIC_NOT_PRESENT(BIC_SYS_LPI);
+		fclose(fp);
 		return -1;
 	}
 	fclose(fp);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 08/19] tools/power turbostat: fix file descriptor leaks
  2019-08-31 19:34 ` [PATCH 01/19] tools/power x86_energy_perf_policy: Fix "uninitialized variable" warnings at -O2 Len Brown
                     ` (5 preceding siblings ...)
  2019-08-31 19:34   ` [PATCH 07/19] tools/power turbostat: fix leak of file descriptor on error return path Len Brown
@ 2019-08-31 19:34   ` Len Brown
  2019-08-31 19:34   ` [PATCH 09/19] tools/power turbostat: fix buffer overrun Len Brown
                     ` (10 subsequent siblings)
  17 siblings, 0 replies; 20+ messages in thread
From: Len Brown @ 2019-08-31 19:34 UTC (permalink / raw)
  To: linux-pm; +Cc: Gustavo A. R. Silva, Prarit Bhargava, Len Brown

From: "Gustavo A. R. Silva" <gustavo@embeddedor.com>

Fix file descriptor leaks by closing fp before return.

Addresses-Coverity-ID: 1444591 ("Resource leak")
Addresses-Coverity-ID: 1444592 ("Resource leak")
Fixes: 5ea7647b333f ("tools/power turbostat: Warn on bad ACPI LPIT data")
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Reviewed-by: Prarit Bhargava <prarit@redhat.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 tools/power/x86/turbostat/turbostat.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index 41cf1206273c..2fb5c155289b 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -2918,6 +2918,7 @@ int snapshot_cpu_lpi_us(void)
 	if (retval != 1) {
 		fprintf(stderr, "Disabling Low Power Idle CPU output\n");
 		BIC_NOT_PRESENT(BIC_CPU_LPI);
+		fclose(fp);
 		return -1;
 	}
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 09/19] tools/power turbostat: fix buffer overrun
  2019-08-31 19:34 ` [PATCH 01/19] tools/power x86_energy_perf_policy: Fix "uninitialized variable" warnings at -O2 Len Brown
                     ` (6 preceding siblings ...)
  2019-08-31 19:34   ` [PATCH 08/19] tools/power turbostat: fix file descriptor leaks Len Brown
@ 2019-08-31 19:34   ` Len Brown
  2019-08-31 19:34   ` [PATCH 10/19] tools/power turbostat: add Jacobsville support Len Brown
                     ` (9 subsequent siblings)
  17 siblings, 0 replies; 20+ messages in thread
From: Len Brown @ 2019-08-31 19:34 UTC (permalink / raw)
  To: linux-pm; +Cc: Naoya Horiguchi, Len Brown

From: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>

turbostat could be terminated by general protection fault on some latest
hardwares which (for example) support 9 levels of C-states and show 18
"tADDED" lines. That bloats the total output and finally causes buffer
overrun.  So let's extend the buffer to avoid this.

Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 tools/power/x86/turbostat/turbostat.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index 2fb5c155289b..f8f4e1c130a6 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -5131,7 +5131,7 @@ int initialize_counters(int cpu_id)
 
 void allocate_output_buffer()
 {
-	output_buffer = calloc(1, (1 + topo.num_cpus) * 1024);
+	output_buffer = calloc(1, (1 + topo.num_cpus) * 2048);
 	outp = output_buffer;
 	if (outp == NULL)
 		err(-1, "calloc output buffer");
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 10/19] tools/power turbostat: add Jacobsville support
  2019-08-31 19:34 ` [PATCH 01/19] tools/power x86_energy_perf_policy: Fix "uninitialized variable" warnings at -O2 Len Brown
                     ` (7 preceding siblings ...)
  2019-08-31 19:34   ` [PATCH 09/19] tools/power turbostat: fix buffer overrun Len Brown
@ 2019-08-31 19:34   ` Len Brown
  2019-08-31 19:34   ` [PATCH 11/19] tools/power turbostat: Fix Haswell Core systems Len Brown
                     ` (8 subsequent siblings)
  17 siblings, 0 replies; 20+ messages in thread
From: Len Brown @ 2019-08-31 19:34 UTC (permalink / raw)
  To: linux-pm; +Cc: Zhang Rui, Len Brown

From: Zhang Rui <rui.zhang@intel.com>

Jacobsville behaves like Denverton.

Signed-off-by: Zhang Rui <rui.zhang@intel.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 tools/power/x86/turbostat/turbostat.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index f8f4e1c130a6..35f4366a522e 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -4591,6 +4591,9 @@ unsigned int intel_model_duplicates(unsigned int model)
 
 	case INTEL_FAM6_ICELAKE_MOBILE:
 		return INTEL_FAM6_CANNONLAKE_MOBILE;
+
+	case INTEL_FAM6_ATOM_TREMONT_X:
+		return INTEL_FAM6_ATOM_GOLDMONT_X;
 	}
 	return model;
 }
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 11/19] tools/power turbostat: Fix Haswell Core systems
  2019-08-31 19:34 ` [PATCH 01/19] tools/power x86_energy_perf_policy: Fix "uninitialized variable" warnings at -O2 Len Brown
                     ` (8 preceding siblings ...)
  2019-08-31 19:34   ` [PATCH 10/19] tools/power turbostat: add Jacobsville support Len Brown
@ 2019-08-31 19:34   ` Len Brown
  2019-08-31 19:34   ` [PATCH 12/19] tools/power turbostat: rename has_hsw_msrs() Len Brown
                     ` (7 subsequent siblings)
  17 siblings, 0 replies; 20+ messages in thread
From: Len Brown @ 2019-08-31 19:34 UTC (permalink / raw)
  To: linux-pm; +Cc: Len Brown, Prarit Bhargava, Kosuke Tatsukawa

From: Len Brown <len.brown@intel.com>

turbostat: cpu0: msr offset 0x630 read failed: Input/output error

because Haswell Core does not have C8-C10.

Output C8-C10 only on Haswell ULT.

Fixes: f5a4c76ad7de ("tools/power turbostat: consolidate duplicate model numbers")

Reported-by: Prarit Bhargava <prarit@redhat.com>
Suggested-by: Kosuke Tatsukawa <tatsu@ab.jp.nec.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 tools/power/x86/turbostat/turbostat.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index 35f4366a522e..78e7c94b94bf 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -3217,6 +3217,7 @@ int probe_nhm_msrs(unsigned int family, unsigned int model)
 		break;
 	case INTEL_FAM6_HASWELL_CORE:	/* HSW */
 	case INTEL_FAM6_HASWELL_X:	/* HSX */
+	case INTEL_FAM6_HASWELL_ULT:	/* HSW */
 	case INTEL_FAM6_HASWELL_GT3E:	/* HSW */
 	case INTEL_FAM6_BROADWELL_CORE:	/* BDW */
 	case INTEL_FAM6_BROADWELL_GT3E:	/* BDW */
@@ -3413,6 +3414,7 @@ int has_config_tdp(unsigned int family, unsigned int model)
 	case INTEL_FAM6_IVYBRIDGE:	/* IVB */
 	case INTEL_FAM6_HASWELL_CORE:	/* HSW */
 	case INTEL_FAM6_HASWELL_X:	/* HSX */
+	case INTEL_FAM6_HASWELL_ULT:	/* HSW */
 	case INTEL_FAM6_HASWELL_GT3E:	/* HSW */
 	case INTEL_FAM6_BROADWELL_CORE:	/* BDW */
 	case INTEL_FAM6_BROADWELL_GT3E:	/* BDW */
@@ -3849,6 +3851,7 @@ void rapl_probe_intel(unsigned int family, unsigned int model)
 	case INTEL_FAM6_SANDYBRIDGE:
 	case INTEL_FAM6_IVYBRIDGE:
 	case INTEL_FAM6_HASWELL_CORE:	/* HSW */
+	case INTEL_FAM6_HASWELL_ULT:	/* HSW */
 	case INTEL_FAM6_HASWELL_GT3E:	/* HSW */
 	case INTEL_FAM6_BROADWELL_CORE:	/* BDW */
 	case INTEL_FAM6_BROADWELL_GT3E:	/* BDW */
@@ -4040,6 +4043,7 @@ void perf_limit_reasons_probe(unsigned int family, unsigned int model)
 
 	switch (model) {
 	case INTEL_FAM6_HASWELL_CORE:	/* HSW */
+	case INTEL_FAM6_HASWELL_ULT:	/* HSW */
 	case INTEL_FAM6_HASWELL_GT3E:	/* HSW */
 		do_gfx_perf_limit_reasons = 1;
 	case INTEL_FAM6_HASWELL_X:	/* HSX */
@@ -4259,6 +4263,7 @@ int has_snb_msrs(unsigned int family, unsigned int model)
 	case INTEL_FAM6_IVYBRIDGE_X:	/* IVB Xeon */
 	case INTEL_FAM6_HASWELL_CORE:	/* HSW */
 	case INTEL_FAM6_HASWELL_X:	/* HSW */
+	case INTEL_FAM6_HASWELL_ULT:	/* HSW */
 	case INTEL_FAM6_HASWELL_GT3E:	/* HSW */
 	case INTEL_FAM6_BROADWELL_CORE:	/* BDW */
 	case INTEL_FAM6_BROADWELL_GT3E:	/* BDW */
@@ -4292,7 +4297,7 @@ int has_hsw_msrs(unsigned int family, unsigned int model)
 		return 0;
 
 	switch (model) {
-	case INTEL_FAM6_HASWELL_CORE:
+	case INTEL_FAM6_HASWELL_ULT:	/* HSW */
 	case INTEL_FAM6_BROADWELL_CORE:	/* BDW */
 	case INTEL_FAM6_SKYLAKE_MOBILE:	/* SKL */
 	case INTEL_FAM6_CANNONLAKE_MOBILE:	/* CNL */
@@ -4576,9 +4581,6 @@ unsigned int intel_model_duplicates(unsigned int model)
 	case INTEL_FAM6_XEON_PHI_KNM:
 		return INTEL_FAM6_XEON_PHI_KNL;
 
-	case INTEL_FAM6_HASWELL_ULT:
-		return INTEL_FAM6_HASWELL_CORE;
-
 	case INTEL_FAM6_BROADWELL_X:
 	case INTEL_FAM6_BROADWELL_XEON_D:	/* BDX-DE */
 		return INTEL_FAM6_BROADWELL_X;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 12/19] tools/power turbostat: rename has_hsw_msrs()
  2019-08-31 19:34 ` [PATCH 01/19] tools/power x86_energy_perf_policy: Fix "uninitialized variable" warnings at -O2 Len Brown
                     ` (9 preceding siblings ...)
  2019-08-31 19:34   ` [PATCH 11/19] tools/power turbostat: Fix Haswell Core systems Len Brown
@ 2019-08-31 19:34   ` Len Brown
  2019-08-31 19:34   ` [PATCH 13/19] tools/power turbostat: Add Ice Lake NNPI support Len Brown
                     ` (6 subsequent siblings)
  17 siblings, 0 replies; 20+ messages in thread
From: Len Brown @ 2019-08-31 19:34 UTC (permalink / raw)
  To: linux-pm; +Cc: Len Brown

From: Len Brown <len.brown@intel.com>

Perhaps if this more descriptive name had been used,
then we wouldn't have had the HSW ULT vs HSW CORE bug,
fixed by the previous commit.

Signed-off-by: Len Brown <len.brown@intel.com>
---
 tools/power/x86/turbostat/turbostat.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index 78e7c94b94bf..51c739043214 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -4280,7 +4280,7 @@ int has_snb_msrs(unsigned int family, unsigned int model)
 }
 
 /*
- * HSW adds support for additional MSRs:
+ * HSW ULT added support for C8/C9/C10 MSRs:
  *
  * MSR_PKG_C8_RESIDENCY		0x00000630
  * MSR_PKG_C9_RESIDENCY		0x00000631
@@ -4291,7 +4291,7 @@ int has_snb_msrs(unsigned int family, unsigned int model)
  * MSR_PKGC10_IRTL		0x00000635
  *
  */
-int has_hsw_msrs(unsigned int family, unsigned int model)
+int has_c8910_msrs(unsigned int family, unsigned int model)
 {
 	if (!genuine_intel)
 		return 0;
@@ -4833,12 +4833,12 @@ void process_cpuid()
 		BIC_NOT_PRESENT(BIC_CPU_c7);
 		BIC_NOT_PRESENT(BIC_Pkgpc7);
 	}
-	if (has_hsw_msrs(family, model)) {
+	if (has_c8910_msrs(family, model)) {
 		BIC_PRESENT(BIC_Pkgpc8);
 		BIC_PRESENT(BIC_Pkgpc9);
 		BIC_PRESENT(BIC_Pkgpc10);
 	}
-	do_irtl_hsw = has_hsw_msrs(family, model);
+	do_irtl_hsw = has_c8910_msrs(family, model);
 	if (has_skl_msrs(family, model)) {
 		BIC_PRESENT(BIC_Totl_c0);
 		BIC_PRESENT(BIC_Any_c0);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 13/19] tools/power turbostat: Add Ice Lake NNPI support
  2019-08-31 19:34 ` [PATCH 01/19] tools/power x86_energy_perf_policy: Fix "uninitialized variable" warnings at -O2 Len Brown
                     ` (10 preceding siblings ...)
  2019-08-31 19:34   ` [PATCH 12/19] tools/power turbostat: rename has_hsw_msrs() Len Brown
@ 2019-08-31 19:34   ` Len Brown
  2019-08-31 19:34   ` [PATCH 14/19] tools/power turbostat: read from pipes too Len Brown
                     ` (5 subsequent siblings)
  17 siblings, 0 replies; 20+ messages in thread
From: Len Brown @ 2019-08-31 19:34 UTC (permalink / raw)
  To: linux-pm; +Cc: Rajneesh Bhardwaj, Len Brown

From: Rajneesh Bhardwaj <rajneesh.bhardwaj@linux.intel.com>

This enables turbostat utility on Ice Lake NNPI SoC.

Link: https://lkml.org/lkml/2019/6/5/1034
Signed-off-by: Rajneesh Bhardwaj <rajneesh.bhardwaj@linux.intel.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 tools/power/x86/turbostat/turbostat.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index 51c739043214..393509655449 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -4592,6 +4592,7 @@ unsigned int intel_model_duplicates(unsigned int model)
 		return INTEL_FAM6_SKYLAKE_MOBILE;
 
 	case INTEL_FAM6_ICELAKE_MOBILE:
+	case INTEL_FAM6_ICELAKE_NNPI:
 		return INTEL_FAM6_CANNONLAKE_MOBILE;
 
 	case INTEL_FAM6_ATOM_TREMONT_X:
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 14/19] tools/power turbostat: read from pipes too
  2019-08-31 19:34 ` [PATCH 01/19] tools/power x86_energy_perf_policy: Fix "uninitialized variable" warnings at -O2 Len Brown
                     ` (11 preceding siblings ...)
  2019-08-31 19:34   ` [PATCH 13/19] tools/power turbostat: Add Ice Lake NNPI support Len Brown
@ 2019-08-31 19:34   ` Len Brown
  2019-08-31 19:34   ` [PATCH 15/19] tools/power turbostat: do not enforce 1ms Len Brown
                     ` (4 subsequent siblings)
  17 siblings, 0 replies; 20+ messages in thread
From: Len Brown @ 2019-08-31 19:34 UTC (permalink / raw)
  To: linux-pm; +Cc: Artem Bityutskiy, Len Brown

From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

Commit '47936f944e78 tools/power turbostat: fix printing on input' make
a valid fix, but it completely disabled piped stdin support, which is
a valuable use-case. Indeed, if stdin is a pipe, turbostat won't read
anything from it, so it becomes impossible to get turbostat output at
user-defined moments, instead of the regular intervals.

There is no reason why this should works for terminals, but not for
pipes. This patch improves the situation. Instead of ignoring pipes, we
read data from them but gracefully handle the EOF case.

Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 tools/power/x86/turbostat/turbostat.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index 393509655449..095bd52cc086 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -100,6 +100,7 @@ unsigned int has_hwp_epp;		/* IA32_HWP_REQUEST[bits 31:24] */
 unsigned int has_hwp_pkg;		/* IA32_HWP_REQUEST_PKG */
 unsigned int has_misc_feature_control;
 unsigned int first_counter_read = 1;
+int ignore_stdin;
 
 #define RAPL_PKG		(1 << 0)
 					/* 0x610 MSR_PKG_POWER_LIMIT */
@@ -3013,26 +3014,37 @@ void setup_signal_handler(void)
 
 void do_sleep(void)
 {
-	struct timeval select_timeout;
+	struct timeval tout;
+	struct timespec rest;
 	fd_set readfds;
 	int retval;
 
 	FD_ZERO(&readfds);
 	FD_SET(0, &readfds);
 
-	if (!isatty(fileno(stdin))) {
+	if (ignore_stdin) {
 		nanosleep(&interval_ts, NULL);
 		return;
 	}
 
-	select_timeout = interval_tv;
-	retval = select(1, &readfds, NULL, NULL, &select_timeout);
+	tout = interval_tv;
+	retval = select(1, &readfds, NULL, NULL, &tout);
 
 	if (retval == 1) {
 		switch (getc(stdin)) {
 		case 'q':
 			exit_requested = 1;
 			break;
+		case EOF:
+			/*
+			 * 'stdin' is a pipe closed on the other end. There
+			 * won't be any further input.
+			 */
+			ignore_stdin = 1;
+			/* Sleep the rest of the time */
+			rest.tv_sec = (tout.tv_sec + tout.tv_usec / 1000000);
+			rest.tv_nsec = (tout.tv_usec % 1000000) * 1000;
+			nanosleep(&rest, NULL);
 		}
 		/* make sure this manually-invoked interval is at least 1ms long */
 		nanosleep(&one_msec, NULL);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 15/19] tools/power turbostat: do not enforce 1ms
  2019-08-31 19:34 ` [PATCH 01/19] tools/power x86_energy_perf_policy: Fix "uninitialized variable" warnings at -O2 Len Brown
                     ` (12 preceding siblings ...)
  2019-08-31 19:34   ` [PATCH 14/19] tools/power turbostat: read from pipes too Len Brown
@ 2019-08-31 19:34   ` Len Brown
  2019-08-31 19:34   ` [PATCH 16/19] tools/power turbostat: Fix CPU%C1 display value Len Brown
                     ` (3 subsequent siblings)
  17 siblings, 0 replies; 20+ messages in thread
From: Len Brown @ 2019-08-31 19:34 UTC (permalink / raw)
  To: linux-pm; +Cc: Artem Bityutskiy, Len Brown

From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

Turbostat works by taking a snapshot of counters, sleeping, taking another
snapshot, calculating deltas, and printing out the table.

The sleep time is controlled via -i option or by user sending a signal or a
character to stdin. In the latter case, turbostat always adds 1 ms
sleep before it reads the counters, in order to avoid larger imprecisions
in the results in prints.

While the 1 ms delay may be a good idea for a "dumb" user, it is a
problem for an "aware" user. I do thousands and thousands of measurements
over a short period of time (like 2ms), and turbostat unconditionally adds
a 1ms to my interval, so I cannot get what I really need.

This patch removes the unconditional 1ms sleep. This is an expert user
tool, after all, and non-experts will unlikely ever use it in the non-fixed
interval mode anyway, so I think it is OK to remove the 1ms delay.

Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 tools/power/x86/turbostat/turbostat.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index 095bd52cc086..7d72268e546d 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -39,7 +39,6 @@ FILE *outf;
 int *fd_percpu;
 struct timeval interval_tv = {5, 0};
 struct timespec interval_ts = {5, 0};
-struct timespec one_msec = {0, 1000000};
 unsigned int num_iterations;
 unsigned int debug;
 unsigned int quiet;
@@ -2994,8 +2993,6 @@ static void signal_handler (int signal)
 			fprintf(stderr, "SIGUSR1\n");
 		break;
 	}
-	/* make sure this manually-invoked interval is at least 1ms long */
-	nanosleep(&one_msec, NULL);
 }
 
 void setup_signal_handler(void)
@@ -3046,8 +3043,6 @@ void do_sleep(void)
 			rest.tv_nsec = (tout.tv_usec % 1000000) * 1000;
 			nanosleep(&rest, NULL);
 		}
-		/* make sure this manually-invoked interval is at least 1ms long */
-		nanosleep(&one_msec, NULL);
 	}
 }
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 16/19] tools/power turbostat: Fix CPU%C1 display value
  2019-08-31 19:34 ` [PATCH 01/19] tools/power x86_energy_perf_policy: Fix "uninitialized variable" warnings at -O2 Len Brown
                     ` (13 preceding siblings ...)
  2019-08-31 19:34   ` [PATCH 15/19] tools/power turbostat: do not enforce 1ms Len Brown
@ 2019-08-31 19:34   ` Len Brown
  2019-08-31 19:34   ` [PATCH 17/19] tools/power turbostat: Fix caller parameter of get_tdp_amd() Len Brown
                     ` (2 subsequent siblings)
  17 siblings, 0 replies; 20+ messages in thread
From: Len Brown @ 2019-08-31 19:34 UTC (permalink / raw)
  To: linux-pm; +Cc: Srinivas Pandruvada, Len Brown

From: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>

In some case C1% will be wrong value, when platform doesn't have MSR for
C1 residency.

For example:
Core    CPU     CPU%c1
-       -       100.00
0       0       100.00
0       2       100.00
1       1       100.00
1       3       100.00

But adding Busy% will fix this
Core    CPU     Busy%   CPU%c1
-       -       99.77   0.23
0       0       99.77   0.23
0       2       99.77   0.23
1       1       99.77   0.23
1       3       99.77   0.23

This issue can be reproduced on most of the recent systems including
Broadwell, Skylake and later.

This is because if we don't select Busy% or Avg_MHz or Bzy_MHz then
mperf value will not be read from MSR, so it will be 0. But this
is required for C1% calculation when MSR for C1 residency is not present.
Same is true for C3, C6 and C7 column selection.

So add another define DO_BIC_READ(), which doesn't depend on user
column selection and use for mperf, C3, C6 and C7 related counters.
So when there is no platform support for C1 residency counters,
we still read these counters, if the CPU has support and user selected
display of CPU%c1.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 tools/power/x86/turbostat/turbostat.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index 7d72268e546d..f57c4023231e 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -507,6 +507,7 @@ unsigned long long bic_enabled = (0xFFFFFFFFFFFFFFFFULL & ~BIC_DISABLED_BY_DEFAU
 unsigned long long bic_present = BIC_USEC | BIC_TOD | BIC_sysfs | BIC_APIC | BIC_X2APIC;
 
 #define DO_BIC(COUNTER_NAME) (bic_enabled & bic_present & COUNTER_NAME)
+#define DO_BIC_READ(COUNTER_NAME) (bic_present & COUNTER_NAME)
 #define ENABLE_BIC(COUNTER_NAME) (bic_enabled |= COUNTER_NAME)
 #define BIC_PRESENT(COUNTER_BIT) (bic_present |= COUNTER_BIT)
 #define BIC_NOT_PRESENT(COUNTER_BIT) (bic_present &= ~COUNTER_BIT)
@@ -1287,6 +1288,14 @@ delta_core(struct core_data *new, struct core_data *old)
 	}
 }
 
+int soft_c1_residency_display(int bic)
+{
+	if (!DO_BIC(BIC_CPU_c1) || use_c1_residency_msr)
+		return 0;
+
+	return DO_BIC_READ(bic);
+}
+
 /*
  * old = new - old
  */
@@ -1323,7 +1332,8 @@ delta_thread(struct thread_data *new, struct thread_data *old,
 
 	old->c1 = new->c1 - old->c1;
 
-	if (DO_BIC(BIC_Avg_MHz) || DO_BIC(BIC_Busy) || DO_BIC(BIC_Bzy_MHz)) {
+	if (DO_BIC(BIC_Avg_MHz) || DO_BIC(BIC_Busy) || DO_BIC(BIC_Bzy_MHz) ||
+	    soft_c1_residency_display(BIC_Avg_MHz)) {
 		if ((new->aperf > old->aperf) && (new->mperf > old->mperf)) {
 			old->aperf = new->aperf - old->aperf;
 			old->mperf = new->mperf - old->mperf;
@@ -1780,7 +1790,8 @@ int get_counters(struct thread_data *t, struct core_data *c, struct pkg_data *p)
 retry:
 	t->tsc = rdtsc();	/* we are running on local CPU of interest */
 
-	if (DO_BIC(BIC_Avg_MHz) || DO_BIC(BIC_Busy) || DO_BIC(BIC_Bzy_MHz)) {
+	if (DO_BIC(BIC_Avg_MHz) || DO_BIC(BIC_Busy) || DO_BIC(BIC_Bzy_MHz) ||
+	    soft_c1_residency_display(BIC_Avg_MHz)) {
 		unsigned long long tsc_before, tsc_between, tsc_after, aperf_time, mperf_time;
 
 		/*
@@ -1857,20 +1868,20 @@ int get_counters(struct thread_data *t, struct core_data *c, struct pkg_data *p)
 	if (!(t->flags & CPU_IS_FIRST_THREAD_IN_CORE))
 		goto done;
 
-	if (DO_BIC(BIC_CPU_c3)) {
+	if (DO_BIC(BIC_CPU_c3) || soft_c1_residency_display(BIC_CPU_c3)) {
 		if (get_msr(cpu, MSR_CORE_C3_RESIDENCY, &c->c3))
 			return -6;
 	}
 
-	if (DO_BIC(BIC_CPU_c6) && !do_knl_cstates) {
+	if ((DO_BIC(BIC_CPU_c6) || soft_c1_residency_display(BIC_CPU_c6)) && !do_knl_cstates) {
 		if (get_msr(cpu, MSR_CORE_C6_RESIDENCY, &c->c6))
 			return -7;
-	} else if (do_knl_cstates) {
+	} else if (do_knl_cstates || soft_c1_residency_display(BIC_CPU_c6)) {
 		if (get_msr(cpu, MSR_KNL_CORE_C6_RESIDENCY, &c->c6))
 			return -7;
 	}
 
-	if (DO_BIC(BIC_CPU_c7))
+	if (DO_BIC(BIC_CPU_c7) || soft_c1_residency_display(BIC_CPU_c7))
 		if (get_msr(cpu, MSR_CORE_C7_RESIDENCY, &c->c7))
 			return -8;
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 17/19] tools/power turbostat: Fix caller parameter of get_tdp_amd()
  2019-08-31 19:34 ` [PATCH 01/19] tools/power x86_energy_perf_policy: Fix "uninitialized variable" warnings at -O2 Len Brown
                     ` (14 preceding siblings ...)
  2019-08-31 19:34   ` [PATCH 16/19] tools/power turbostat: Fix CPU%C1 display value Len Brown
@ 2019-08-31 19:34   ` Len Brown
  2019-08-31 19:34   ` [PATCH 18/19] tools/power turbostat: Add support for Hygon Fam 18h (Dhyana) RAPL Len Brown
  2019-08-31 19:34   ` [PATCH 19/19] tools/power turbostat: update version number Len Brown
  17 siblings, 0 replies; 20+ messages in thread
From: Len Brown @ 2019-08-31 19:34 UTC (permalink / raw)
  To: linux-pm; +Cc: Pu Wen, stable, Calvin Walton, Len Brown

From: Pu Wen <puwen@hygon.cn>

Commit 9392bd98bba760be96ee ("tools/power turbostat: Add support for AMD
Fam 17h (Zen) RAPL") add a function get_tdp_amd(), the parameter is CPU
family. But the rapl_probe_amd() function use wrong model parameter.
Fix the wrong caller parameter of get_tdp_amd() to use family.

Cc: <stable@vger.kernel.org> # v5.1+
Signed-off-by: Pu Wen <puwen@hygon.cn>
Reviewed-by: Calvin Walton <calvin.walton@kepstin.ca>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 tools/power/x86/turbostat/turbostat.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index f57c4023231e..6cec6aa01241 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -4031,7 +4031,7 @@ void rapl_probe_amd(unsigned int family, unsigned int model)
 	rapl_energy_units = ldexp(1.0, -(msr >> 8 & 0x1f));
 	rapl_power_units = ldexp(1.0, -(msr & 0xf));
 
-	tdp = get_tdp_amd(model);
+	tdp = get_tdp_amd(family);
 
 	rapl_joule_counter_range = 0xFFFFFFFF * rapl_energy_units / tdp;
 	if (!quiet)
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 18/19] tools/power turbostat: Add support for Hygon Fam 18h (Dhyana) RAPL
  2019-08-31 19:34 ` [PATCH 01/19] tools/power x86_energy_perf_policy: Fix "uninitialized variable" warnings at -O2 Len Brown
                     ` (15 preceding siblings ...)
  2019-08-31 19:34   ` [PATCH 17/19] tools/power turbostat: Fix caller parameter of get_tdp_amd() Len Brown
@ 2019-08-31 19:34   ` Len Brown
  2019-08-31 19:34   ` [PATCH 19/19] tools/power turbostat: update version number Len Brown
  17 siblings, 0 replies; 20+ messages in thread
From: Len Brown @ 2019-08-31 19:34 UTC (permalink / raw)
  To: linux-pm; +Cc: Pu Wen, Calvin Walton, Len Brown

From: Pu Wen <puwen@hygon.cn>

Commit 9392bd98bba760be96ee ("tools/power turbostat: Add support for AMD
Fam 17h (Zen) RAPL") and the commit 3316f99a9f1b68c578c5 ("tools/power
turbostat: Also read package power on AMD F17h (Zen)") add AMD Fam 17h
RAPL support.

Hygon Family 18h(Dhyana) support RAPL in bit 14 of CPUID 0x80000007 EDX,
and has MSRs RAPL_PWR_UNIT/CORE_ENERGY_STAT/PKG_ENERGY_STAT. So add Hygon
Dhyana Family 18h support for RAPL.

Already tested on Hygon multi-node systems and it shows correct per-core
energy usage and the total package power.

Signed-off-by: Pu Wen <puwen@hygon.cn>
Reviewed-by: Calvin Walton <calvin.walton@kepstin.ca>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 tools/power/x86/turbostat/turbostat.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index 6cec6aa01241..e8b6c608d6d1 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -59,6 +59,7 @@ unsigned int do_irtl_hsw;
 unsigned int units = 1000000;	/* MHz etc */
 unsigned int genuine_intel;
 unsigned int authentic_amd;
+unsigned int hygon_genuine;
 unsigned int max_level, max_extended_level;
 unsigned int has_invariant_tsc;
 unsigned int do_nhm_platform_info;
@@ -1730,7 +1731,7 @@ void get_apic_id(struct thread_data *t)
 	if (!DO_BIC(BIC_X2APIC))
 		return;
 
-	if (authentic_amd) {
+	if (authentic_amd || hygon_genuine) {
 		unsigned int topology_extensions;
 
 		if (max_extended_level < 0x8000001e)
@@ -3831,6 +3832,7 @@ double get_tdp_amd(unsigned int family)
 {
 	switch (family) {
 	case 0x17:
+	case 0x18:
 	default:
 		/* This is the max stock TDP of HEDT/Server Fam17h chips */
 		return 250.0;
@@ -4011,6 +4013,7 @@ void rapl_probe_amd(unsigned int family, unsigned int model)
 
 	switch (family) {
 	case 0x17: /* Zen, Zen+ */
+	case 0x18: /* Hygon Dhyana */
 		do_rapl = RAPL_AMD_F17H | RAPL_PER_CORE_ENERGY;
 		if (rapl_joules) {
 			BIC_PRESENT(BIC_Pkg_J);
@@ -4047,7 +4050,7 @@ void rapl_probe(unsigned int family, unsigned int model)
 {
 	if (genuine_intel)
 		rapl_probe_intel(family, model);
-	if (authentic_amd)
+	if (authentic_amd || hygon_genuine)
 		rapl_probe_amd(family, model);
 }
 
@@ -4632,6 +4635,8 @@ void process_cpuid()
 		genuine_intel = 1;
 	else if (ebx == 0x68747541 && ecx == 0x444d4163 && edx == 0x69746e65)
 		authentic_amd = 1;
+	else if (ebx == 0x6f677948 && ecx == 0x656e6975 && edx == 0x6e65476e)
+		hygon_genuine = 1;
 
 	if (!quiet)
 		fprintf(outf, "CPUID(0): %.4s%.4s%.4s ",
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 19/19] tools/power turbostat: update version number
  2019-08-31 19:34 ` [PATCH 01/19] tools/power x86_energy_perf_policy: Fix "uninitialized variable" warnings at -O2 Len Brown
                     ` (16 preceding siblings ...)
  2019-08-31 19:34   ` [PATCH 18/19] tools/power turbostat: Add support for Hygon Fam 18h (Dhyana) RAPL Len Brown
@ 2019-08-31 19:34   ` Len Brown
  17 siblings, 0 replies; 20+ messages in thread
From: Len Brown @ 2019-08-31 19:34 UTC (permalink / raw)
  To: linux-pm; +Cc: Len Brown

From: Len Brown <len.brown@intel.com>

Today is 19.08.31, at least in some parts of the world.

Signed-off-by: Len Brown <len.brown@intel.com>
---
 tools/power/x86/turbostat/turbostat.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index e8b6c608d6d1..b2a86438f074 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -5306,7 +5306,7 @@ int get_and_dump_counters(void)
 }
 
 void print_version() {
-	fprintf(outf, "turbostat version 19.03.20"
+	fprintf(outf, "turbostat version 19.08.31"
 		" - Len Brown <lenb@kernel.org>\n");
 }
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2019-08-31 19:35 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-31 19:34 turbostat 19.08.31 is available Len Brown
2019-08-31 19:34 ` [PATCH 01/19] tools/power x86_energy_perf_policy: Fix "uninitialized variable" warnings at -O2 Len Brown
2019-08-31 19:34   ` [PATCH 02/19] tools/power/x86: Enable compiler optimisations and Fortify by default Len Brown
2019-08-31 19:34   ` [PATCH 03/19] tools/power: Fix typo in man page Len Brown
2019-08-31 19:34   ` [PATCH 04/19] tools/power x86_energy_perf_policy: Fix argument parsing Len Brown
2019-08-31 19:34   ` [PATCH 05/19] tools/power turbostat: remove duplicate pc10 column Len Brown
2019-08-31 19:34   ` [PATCH 06/19] tools/power turbostat: Make interval calculation per thread to reduce jitter Len Brown
2019-08-31 19:34   ` [PATCH 07/19] tools/power turbostat: fix leak of file descriptor on error return path Len Brown
2019-08-31 19:34   ` [PATCH 08/19] tools/power turbostat: fix file descriptor leaks Len Brown
2019-08-31 19:34   ` [PATCH 09/19] tools/power turbostat: fix buffer overrun Len Brown
2019-08-31 19:34   ` [PATCH 10/19] tools/power turbostat: add Jacobsville support Len Brown
2019-08-31 19:34   ` [PATCH 11/19] tools/power turbostat: Fix Haswell Core systems Len Brown
2019-08-31 19:34   ` [PATCH 12/19] tools/power turbostat: rename has_hsw_msrs() Len Brown
2019-08-31 19:34   ` [PATCH 13/19] tools/power turbostat: Add Ice Lake NNPI support Len Brown
2019-08-31 19:34   ` [PATCH 14/19] tools/power turbostat: read from pipes too Len Brown
2019-08-31 19:34   ` [PATCH 15/19] tools/power turbostat: do not enforce 1ms Len Brown
2019-08-31 19:34   ` [PATCH 16/19] tools/power turbostat: Fix CPU%C1 display value Len Brown
2019-08-31 19:34   ` [PATCH 17/19] tools/power turbostat: Fix caller parameter of get_tdp_amd() Len Brown
2019-08-31 19:34   ` [PATCH 18/19] tools/power turbostat: Add support for Hygon Fam 18h (Dhyana) RAPL Len Brown
2019-08-31 19:34   ` [PATCH 19/19] tools/power turbostat: update version number Len Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).