diff mbox series

tools/power turbostat: Make interval calculation per thread to reduce jitter

Message ID 20190325173232.216357-1-Yazen.Ghannam@amd.com (mailing list archive)
State Accepted
Delegated to: Len Brown
Headers show
Series tools/power turbostat: Make interval calculation per thread to reduce jitter | expand

Commit Message

Yazen Ghannam March 25, 2019, 5:32 p.m. UTC
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>
---
 tools/power/x86/turbostat/turbostat.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

Comments

Yazen Ghannam April 23, 2019, 5:53 p.m. UTC | #1
> -----Original Message-----
> From: linux-kernel-owner@vger.kernel.org <linux-kernel-owner@vger.kernel.org> On Behalf Of Ghannam, Yazen
> Sent: Monday, March 25, 2019 12:33 PM
> To: linux-pm@vger.kernel.org
> Cc: Ghannam, Yazen <Yazen.Ghannam@amd.com>; linux-kernel@vger.kernel.org; lenb@kernel.org
> Subject: [PATCH] tools/power turbostat: Make interval calculation per thread to reduce jitter
> 
> 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>
> ---

Hi Len,

Any comments on this patch?

Thanks,
Yazen
Yazen Ghannam June 7, 2019, 4:28 p.m. UTC | #2
> -----Original Message-----
> From: Ghannam, Yazen <Yazen.Ghannam@amd.com>
> Sent: Tuesday, April 23, 2019 12:53 PM
> To: Ghannam, Yazen <Yazen.Ghannam@amd.com>; linux-pm@vger.kernel.org; len.brown@intel.com
> Cc: linux-kernel@vger.kernel.org; Len Brown <lenb@kernel.org>
> Subject: RE: [PATCH] tools/power turbostat: Make interval calculation per thread to reduce jitter
> 
> > -----Original Message-----
> > From: linux-kernel-owner@vger.kernel.org <linux-kernel-owner@vger.kernel.org> On Behalf Of Ghannam, Yazen
> > Sent: Monday, March 25, 2019 12:33 PM
> > To: linux-pm@vger.kernel.org
> > Cc: Ghannam, Yazen <Yazen.Ghannam@amd.com>; linux-kernel@vger.kernel.org; lenb@kernel.org
> > Subject: [PATCH] tools/power turbostat: Make interval calculation per thread to reduce jitter
> >
> > 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>
> > ---
> 
> Hi Len,
> 
> Any comments on this patch?
> 

Hi Len,

Just wanted to check in. Do you have any comments on this patch?

Thanks,
Yazen
Len Brown Aug. 31, 2019, 4:48 p.m. UTC | #3
Yeah, I like this patch, and I have applied it.

thanks!
-Len

On Fri, Jun 7, 2019 at 12:28 PM Ghannam, Yazen <Yazen.Ghannam@amd.com> wrote:
>
> > -----Original Message-----
> > From: Ghannam, Yazen <Yazen.Ghannam@amd.com>
> > Sent: Tuesday, April 23, 2019 12:53 PM
> > To: Ghannam, Yazen <Yazen.Ghannam@amd.com>; linux-pm@vger.kernel.org; len.brown@intel.com
> > Cc: linux-kernel@vger.kernel.org; Len Brown <lenb@kernel.org>
> > Subject: RE: [PATCH] tools/power turbostat: Make interval calculation per thread to reduce jitter
> >
> > > -----Original Message-----
> > > From: linux-kernel-owner@vger.kernel.org <linux-kernel-owner@vger.kernel.org> On Behalf Of Ghannam, Yazen
> > > Sent: Monday, March 25, 2019 12:33 PM
> > > To: linux-pm@vger.kernel.org
> > > Cc: Ghannam, Yazen <Yazen.Ghannam@amd.com>; linux-kernel@vger.kernel.org; lenb@kernel.org
> > > Subject: [PATCH] tools/power turbostat: Make interval calculation per thread to reduce jitter
> > >
> > > 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>
> > > ---
> >
> > Hi Len,
> >
> > Any comments on this patch?
> >
>
> Hi Len,
>
> Just wanted to check in. Do you have any comments on this patch?
>
> Thanks,
> Yazen
diff mbox series

Patch

diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index c3fad065c89c..6ebcc420021f 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;
@@ -896,7 +897,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;
 
@@ -1279,6 +1280,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;
 
@@ -1374,6 +1376,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;
@@ -1540,6 +1544,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;
@@ -1729,13 +1736,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: