2008-10-17 00:00:51

by Mathieu Desnoyers

[permalink] [raw]
Subject: [RFC patch 04/15] get_cycles() : powerpc64 HAVE_GET_CYCLES

This patch selects HAVE_GET_CYCLES and makes sure get_cycles_barrier() and
get_cycles_rate() are implemented.

Signed-off-by: Mathieu Desnoyers <[email protected]>
CC: [email protected]
CC: [email protected]
CC: David Miller <[email protected]>
CC: Linus Torvalds <[email protected]>
CC: Andrew Morton <[email protected]>
CC: Ingo Molnar <[email protected]>
CC: Peter Zijlstra <[email protected]>
CC: Thomas Gleixner <[email protected]>
CC: Steven Rostedt <[email protected]>
CC: [email protected]
---
arch/powerpc/Kconfig | 1 +
arch/powerpc/include/asm/timex.h | 13 +++++++++++++
2 files changed, 14 insertions(+)

Index: linux-2.6-lttng/arch/powerpc/Kconfig
===================================================================
--- linux-2.6-lttng.orig/arch/powerpc/Kconfig 2008-10-16 12:25:48.000000000 -0400
+++ linux-2.6-lttng/arch/powerpc/Kconfig 2008-10-16 12:33:31.000000000 -0400
@@ -122,6 +122,7 @@ config PPC
select HAVE_DMA_ATTRS if PPC64
select USE_GENERIC_SMP_HELPERS if SMP
select HAVE_OPROFILE
+ select HAVE_GET_CYCLES if PPC64

config EARLY_PRINTK
bool
Index: linux-2.6-lttng/arch/powerpc/include/asm/timex.h
===================================================================
--- linux-2.6-lttng.orig/arch/powerpc/include/asm/timex.h 2008-10-16 12:25:48.000000000 -0400
+++ linux-2.6-lttng/arch/powerpc/include/asm/timex.h 2008-10-16 12:33:58.000000000 -0400
@@ -46,5 +46,18 @@ static inline cycles_t get_cycles(void)
#endif
}

+static inline cycles_t get_cycles_rate(void)
+{
+ return CLOCK_TICK_RATE;
+}
+
+/*
+ * To check : assuming mtfb requires isync to synchronize instruction execution.
+ */
+static inline void get_cycles_barrier(void)
+{
+ isync();
+}
+
#endif /* __KERNEL__ */
#endif /* _ASM_POWERPC_TIMEX_H */

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


2008-10-17 00:27:16

by Paul Mackerras

[permalink] [raw]
Subject: Re: [RFC patch 04/15] get_cycles() : powerpc64 HAVE_GET_CYCLES

Mathieu Desnoyers writes:

> This patch selects HAVE_GET_CYCLES and makes sure get_cycles_barrier() and
> get_cycles_rate() are implemented.

[snip]

> +static inline cycles_t get_cycles_rate(void)
> +{
> + return CLOCK_TICK_RATE;
> +}

CLOCK_TICK_RATE is certainly wrong. You want ppc_tb_freq (declared in
asm/time.h). Or tb_ticks_per_sec, since we seem to have two variables
for exactly the same thing, for some reason. :)

Paul.

2008-10-17 00:43:44

by Mathieu Desnoyers

[permalink] [raw]
Subject: Re: [RFC patch 04/15] get_cycles() : powerpc64 HAVE_GET_CYCLES (update)

* Paul Mackerras ([email protected]) wrote:
> Mathieu Desnoyers writes:
>
> > This patch selects HAVE_GET_CYCLES and makes sure get_cycles_barrier() and
> > get_cycles_rate() are implemented.
>
> [snip]
>
> > +static inline cycles_t get_cycles_rate(void)
> > +{
> > + return CLOCK_TICK_RATE;
> > +}
>
> CLOCK_TICK_RATE is certainly wrong. You want ppc_tb_freq (declared in
> asm/time.h). Or tb_ticks_per_sec, since we seem to have two variables
> for exactly the same thing, for some reason. :)
>
> Paul.

Ok, this should work better. Thanks !

Do you know if mtfb implies an instruction synchronization (isync) ? I
think that if it does not, the new get_cycles_barrier() might have to be
used at some locations in the kernel code if more precise timestamp
order is required.

Mathieu


get_cycles() : powerpc64 HAVE_GET_CYCLES

This patch selects HAVE_GET_CYCLES and makes sure get_cycles_barrier() and
get_cycles_rate() are implemented.

Signed-off-by: Mathieu Desnoyers <[email protected]>
CC: [email protected]
CC: [email protected]
CC: David Miller <[email protected]>
CC: Linus Torvalds <[email protected]>
CC: Andrew Morton <[email protected]>
CC: Ingo Molnar <[email protected]>
CC: Peter Zijlstra <[email protected]>
CC: Thomas Gleixner <[email protected]>
CC: Steven Rostedt <[email protected]>
CC: [email protected]
---
arch/powerpc/Kconfig | 1 +
arch/powerpc/include/asm/timex.h | 14 ++++++++++++++
2 files changed, 15 insertions(+)

Index: linux-2.6-lttng/arch/powerpc/Kconfig
===================================================================
--- linux-2.6-lttng.orig/arch/powerpc/Kconfig 2008-10-16 20:31:33.000000000 -0400
+++ linux-2.6-lttng/arch/powerpc/Kconfig 2008-10-16 20:31:36.000000000 -0400
@@ -122,6 +122,7 @@ config PPC
select HAVE_DMA_ATTRS if PPC64
select USE_GENERIC_SMP_HELPERS if SMP
select HAVE_OPROFILE
+ select HAVE_GET_CYCLES if PPC64

config EARLY_PRINTK
bool
Index: linux-2.6-lttng/arch/powerpc/include/asm/timex.h
===================================================================
--- linux-2.6-lttng.orig/arch/powerpc/include/asm/timex.h 2008-10-16 20:31:33.000000000 -0400
+++ linux-2.6-lttng/arch/powerpc/include/asm/timex.h 2008-10-16 20:32:02.000000000 -0400
@@ -9,6 +9,7 @@

#include <asm/cputable.h>
#include <asm/reg.h>
+#include <asm/time.h>

#define CLOCK_TICK_RATE 1024000 /* Underlying HZ */

@@ -46,5 +47,18 @@ static inline cycles_t get_cycles(void)
#endif
}

+static inline cycles_t get_cycles_rate(void)
+{
+ return tb_ticks_per_sec;
+}
+
+/*
+ * To check : assuming mtfb requires isync to synchronize instruction execution.
+ */
+static inline void get_cycles_barrier(void)
+{
+ isync();
+}
+
#endif /* __KERNEL__ */
#endif /* _ASM_POWERPC_TIMEX_H */

--
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68

2008-10-17 01:13:40

by Paul Mackerras

[permalink] [raw]
Subject: Re: [RFC patch 04/15] get_cycles() : powerpc64 HAVE_GET_CYCLES (update)

Mathieu Desnoyers writes:

> Do you know if mtfb implies an instruction synchronization (isync) ? I

It doesn't.

> think that if it does not, the new get_cycles_barrier() might have to be
> used at some locations in the kernel code if more precise timestamp
> order is required.

OK. I'll let you figure out where. :)

Paul.

2008-10-17 01:43:26

by David Miller

[permalink] [raw]
Subject: Re: [RFC patch 04/15] get_cycles() : powerpc64 HAVE_GET_CYCLES (update)

From: Mathieu Desnoyers <[email protected]>
Date: Thu, 16 Oct 2008 20:43:28 -0400

> * Paul Mackerras ([email protected]) wrote:
> > Mathieu Desnoyers writes:
> >
> > > This patch selects HAVE_GET_CYCLES and makes sure get_cycles_barrier() and
> > > get_cycles_rate() are implemented.
> >
> > [snip]
> >
> > > +static inline cycles_t get_cycles_rate(void)
> > > +{
> > > + return CLOCK_TICK_RATE;
> > > +}
> >
> > CLOCK_TICK_RATE is certainly wrong. You want ppc_tb_freq (declared in
> > asm/time.h). Or tb_ticks_per_sec, since we seem to have two variables
> > for exactly the same thing, for some reason. :)
> >
> > Paul.
>
> Ok, this should work better. Thanks !
>
> Do you know if mtfb implies an instruction synchronization (isync) ? I
> think that if it does not, the new get_cycles_barrier() might have to be
> used at some locations in the kernel code if more precise timestamp
> order is required.

You'll need to make a similar fix on sparc64.

2008-10-17 02:08:40

by Mathieu Desnoyers

[permalink] [raw]
Subject: Re: [RFC patch 04/15] get_cycles() : powerpc64 HAVE_GET_CYCLES (update)

* David Miller ([email protected]) wrote:
> From: Mathieu Desnoyers <[email protected]>
> Date: Thu, 16 Oct 2008 20:43:28 -0400
>
> > * Paul Mackerras ([email protected]) wrote:
> > > Mathieu Desnoyers writes:
> > >
> > > > This patch selects HAVE_GET_CYCLES and makes sure get_cycles_barrier() and
> > > > get_cycles_rate() are implemented.
> > >
> > > [snip]
> > >
> > > > +static inline cycles_t get_cycles_rate(void)
> > > > +{
> > > > + return CLOCK_TICK_RATE;
> > > > +}
> > >
> > > CLOCK_TICK_RATE is certainly wrong. You want ppc_tb_freq (declared in
> > > asm/time.h). Or tb_ticks_per_sec, since we seem to have two variables
> > > for exactly the same thing, for some reason. :)
> > >
> > > Paul.
> >
> > Ok, this should work better. Thanks !
> >
> > Do you know if mtfb implies an instruction synchronization (isync) ? I
> > think that if it does not, the new get_cycles_barrier() might have to be
> > used at some locations in the kernel code if more precise timestamp
> > order is required.
>
> You'll need to make a similar fix on sparc64.

I guess you are talking about using sparc64_get_clock_tick rather than
CLOCK_TICK_RATE ? I assume sparc64_get_clock_tick() done on any CPU will
return the same rate ?

Thanks,

Mathieu

--
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68

2008-10-17 02:33:45

by David Miller

[permalink] [raw]
Subject: Re: [RFC patch 04/15] get_cycles() : powerpc64 HAVE_GET_CYCLES (update)

From: Mathieu Desnoyers <[email protected]>
Date: Thu, 16 Oct 2008 22:08:13 -0400

> * David Miller ([email protected]) wrote:
> > Date: Thu, 16 Oct 2008 20:43:28 -0400
> >
> > You'll need to make a similar fix on sparc64.
>
> I guess you are talking about using sparc64_get_clock_tick rather than
> CLOCK_TICK_RATE ? I assume sparc64_get_clock_tick() done on any CPU will
> return the same rate ?

You'll need to use tb_ticks_per_usec or similar.

The ->get_tick() thing you are using uses a TICK source which is
synchronized across the entire system and advances at a non-changing
rate.

sparc64_get_clock_tick() returns something different.