[PATCH 2/7] Simple Performance Counters: x86_64 support

[PATCH 2/7] Simple Performance Counters: x86_64 support

[Christoph Lameter]

Export cycles_to_ns (after renaming the cycles_2_ns to cycles_to_ns to be
conforming to other arches).

Signed-off-by: Christoph Lameter <clameter@sgi.com>
---
 arch/x86_64/kernel/tsc.c   |    4 ++--
 include/asm-x86_64/timex.h |    1 +
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/x86_64/kernel/tsc.c b/arch/x86_64/kernel/tsc.c
index 2a59bde..59f69af 100644
--- a/arch/x86_64/kernel/tsc.c
+++ b/arch/x86_64/kernel/tsc.c
@@ -23,7 +23,7 @@ void set_cyc2ns_scale(unsigned long khz)
 	cyc2ns_scale = (NSEC_PER_MSEC << NS_SCALE) / khz;
 }
 
-static unsigned long long cycles_2_ns(unsigned long long cyc)
+unsigned long long cycles_to_ns(unsigned long long cyc)
 {
 	return (cyc * cyc2ns_scale) >> NS_SCALE;
 }
@@ -39,7 +39,7 @@ unsigned long long sched_clock(void)
 	 */
 
 	rdtscll(a);
-	return cycles_2_ns(a);
+	return cycles_to_ns(a);
 }
 
 static int tsc_unstable;
diff --git a/include/asm-x86_64/timex.h b/include/asm-x86_64/timex.h
index 6ed21f4..d5e126c 100644
--- a/include/asm-x86_64/timex.h
+++ b/include/asm-x86_64/timex.h
@@ -28,4 +28,5 @@ extern int read_current_timer(unsigned long *timer_value);
 
 extern void mark_tsc_unstable(char *msg);
 extern void set_cyc2ns_scale(unsigned long khz);
+unsigned long long cycles_to_ns(unsigned long long cyc);
 #endif
-- 
1.5.2.4

Re: [PATCH 2/7] Simple Performance Counters: x86_64 support

[Andi Kleen]

Christoph Lameter <clameter@sgi.com> writes:

> Export cycles_to_ns (after renaming the cycles_2_ns to cycles_to_ns to be
> conforming to other arches).

This function is broken in some cases (e.g Opteron with powernow or
AMD dual core or older Intel systems) and going away.  The TSCs tick
with different frequencies on different CPU there. In general i would
recommend to only report cycles.

-Andi

Re: [PATCH 2/7] Simple Performance Counters: x86_64 support

[Christoph Lameter]

On Wed, 1 Aug 2007, Andi Kleen wrote:

> Christoph Lameter <clameter@sgi.com> writes:
> 
> > Export cycles_to_ns (after renaming the cycles_2_ns to cycles_to_ns to be
> > conforming to other arches).
> 
> This function is broken in some cases (e.g Opteron with powernow or
> AMD dual core or older Intel systems) and going away.  The TSCs tick
> with different frequencies on different CPU there. In general i would
> recommend to only report cycles.

But then the numbers are not comparable across systems.

Re: [PATCH 2/7] Simple Performance Counters: x86_64 support

[Andi Kleen]

> But then the numbers are not comparable across systems.

That would only matter if you're interested in absolute system
performance of different systems.

But comparable performance testing in general only makes sense when you
only change one variable: either you change the hardware or
you change the software. If you change both at the same time
you'll never know where the difference comes from. 

I assume this facility is aimed at software performance testing:
this means it doesn't make much sense to compare numbers
between different systems; just relative numbers on the same
system with different software.

If you want absolute comparable system performance benchmarks I don't think
micro-instrumenting the kernel is a good approach.

But for relative performance cycles are fine. Anyways if you really
wanted to do ns it's possible too, but far more complicated to do correctly.

-Andi

Re: [PATCH 2/7] Simple Performance Counters: x86_64 support

[Christoph Lameter]

On Wed, 1 Aug 2007, Andi Kleen wrote:

> > But then the numbers are not comparable across systems.
> 
> That would only matter if you're interested in absolute system
> performance of different systems.

Yes I am definitely interested in seeing which type of cpu runs a certain 
section faster.

> If you want absolute comparable system performance benchmarks I don't think
> micro-instrumenting the kernel is a good approach.

It is certainly interesting to compare alternative ways of handling the 
instruction streams by various processors or models of processors.

Re: [PATCH 2/7] Simple Performance Counters: x86_64 support

[Andi Kleen]

> It is certainly interesting to compare alternative ways of handling the 
> instruction streams by various processors or models of processors.

Well you have to do a lot more work then to handle instable TSCs then.

In particular the frequencies can be different between CPUs, they 
change (which you can catch with cpufreq notifiers) and during the
cpufreq change period they're instable (as in you can't tell for
some time which frequency they're currently running at and they
might be running immediate frequencies) 

The current cycles_2_ns() users avoid this problem by not
using the TSC in this case, but that might not be an option
for you (cpufreq is becoming more and more ubiquitous)

Good luck.

-Andi

Re: [PATCH 2/7] Simple Performance Counters: x86_64 support

[Christoph Lameter]

On Wed, 1 Aug 2007, Andi Kleen wrote:

> > It is certainly interesting to compare alternative ways of handling the 
> > instruction streams by various processors or models of processors.
> 
> Well you have to do a lot more work then to handle instable TSCs then.

I have been using this for 2 years. It works fine for my purposes.

> In particular the frequencies can be different between CPUs, they 
> change (which you can catch with cpufreq notifiers) and during the
> cpufreq change period they're instable (as in you can't tell for
> some time which frequency they're currently running at and they
> might be running immediate frequencies) 

Well then simply make sure that they do not change while you measure.
 

Re: [PATCH 2/7] Simple Performance Counters: x86_64 support

[Andi Kleen]

On Wed, Aug 01, 2007 at 11:45:39AM -0700, Christoph Lameter wrote:
> On Wed, 1 Aug 2007, Andi Kleen wrote:
> 
> > > It is certainly interesting to compare alternative ways of handling the 
> > > instruction streams by various processors or models of processors.
> > 
> > Well you have to do a lot more work then to handle instable TSCs then.
> 
> I have been using this for 2 years. It works fine for my purposes.

That might be on your systems, but for a mainline submission the
standards are higher.

> 
> > In particular the frequencies can be different between CPUs, they 
> > change (which you can catch with cpufreq notifiers) and during the
> > cpufreq change period they're instable (as in you can't tell for
> > some time which frequency they're currently running at and they
> > might be running immediate frequencies) 
> 
> Well then simply make sure that they do not change while you measure.

That would be a merge blocker in my opinion. Suitable for local
hacks, but nothing we want in tree.

-Andi

Re: [PATCH 2/7] Simple Performance Counters: x86_64 support

[Christoph Lameter]

On Wed, 1 Aug 2007, Andi Kleen wrote:

> That might be on your systems, but for a mainline submission the
> standards are higher.

Right that is why I asked for someone to take it over and do mainline work 
if that is wanted. I had a couple of requests for these patches.

Re: [PATCH 2/7] Simple Performance Counters: x86_64 support

[Andi Kleen]

On Wed, Aug 01, 2007 at 11:58:44AM -0700, Christoph Lameter wrote:
> On Wed, 1 Aug 2007, Andi Kleen wrote:
> 
> > That might be on your systems, but for a mainline submission the
> > standards are higher.
> 
> Right that is why I asked for someone to take it over and do mainline work 
> if that is wanted. I had a couple of requests for these patches.

The easiest way to solve this would be to switch to reporting cycles.
That would simplify your patch in fact.

-Andi

Re: [PATCH 2/7] Simple Performance Counters: x86_64 support

[Mathieu Desnoyers]

* Andi Kleen (andi@firstfloor.org) wrote:
> On Wed, Aug 01, 2007 at 11:58:44AM -0700, Christoph Lameter wrote:
> > On Wed, 1 Aug 2007, Andi Kleen wrote:
> > 
> > > That might be on your systems, but for a mainline submission the
> > > standards are higher.
> > 
> > Right that is why I asked for someone to take it over and do mainline work 
> > if that is wanted. I had a couple of requests for these patches.
> 
> The easiest way to solve this would be to switch to reporting cycles.
> That would simplify your patch in fact.
>

Actually, I deal with that in LTTng by having a module that verifies
synchronicity of TSCs across CPUs and, if it's over a certain threshold,
falls back to a sophisticated logical clock (actually, it is a mix of
the TSC of the cpu with highest count and a logical clock) and prints a
printk informing the user of the inaccuracy. I also provide suggestions
on my website of what kernel features must be disabled by users to get
precise timestamps (idle=poll, disable frequency scaling) ans what the
limitations are on Intel and AMD models.

It is packed in a lttng-timestamp-* patchset which I plan to eventually
post for review.

It also provides an architecture independent timestamp read consisting
in jiffies or'd with a logical clock. No locking is required by the
reader (no read reqlock at all). This type of clock could be considered
as "always monotonically increasing", which is the basic condition
required to have a meaningful trace.

Mathieu

> -Andi
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68