[ltt-dev] trace_clock_update() spinning

[ltt-dev] trace_clock_update() spinning

[Mike McTernan]

Hi,

I'm using Linux 2.6.28.2 + LTTng 0.88 on an ARM Freescale i.MX51 board.


The system works quite well - in fact LTTng is awesome!  But there's a
problem that the output in LTTV shows run_timer_softirq() taking almost
all the time between timer ticks.  HZ is set to 100 so the timer IRQ
runs every 10ms, with run_timer_softirq() typically taking about 9.8ms
as measured in LTTV.

I added some trace_mark() calls to find the consumer of this time to be
trace_clock_update().  Specifically the atomic update:

  n = 0;
  
  do {
    n++;
    
    old_clock = atomic_long_read(&trace_clock);
    new_clock = (old_clock + (ticks << TRACE_CLOCK_SHIFT))
      & (~((1 << TRACE_CLOCK_SHIFT) - 1));             
  } while (atomic_long_cmpxchg(&trace_clock, old_clock, new_clock)
             != old_clock);

  trace_mark(trace_clock_update, exit, "exit %u", n);

This never appears to loop as the "exit" value I added is always 1.
However I notice that on ARM atomic_long_cmpxchg() contains a loop, and
so is presumably racing around in there.

I'm not sure if this something completely unexpected, or normal
behaviour?  Looking through the code I can't see that trace_clock is
updated in many cases, although my grep maybe off.  I wondered if the
ARM atomic_long_cmpxchg() is broken, but didn't find much there either.

Since I'm on a uniprocessor system, could I safely modify the update
function to just atomically update the trace_clock without using the
cmpxchg() operation since it's called in the soft IRQ?
 
Regards,

Mike

[ltt-dev] trace_clock_update() spinning

[Mathieu Desnoyers]

Hi Mike,

The answer to your question is in the LTTng Manual, "Supported
architectures"

http://lttng.org/cgi-bin/gitweb.cgi?p=lttv.git;a=blob_plain;f=LTTngManual.html

it states that "Other ARM (with limited timestamping precision, e.g.
1HZ. Need architecture-specific support for better precision)"

So it's not that LTTng itself is taking time, but that the trace clock
resolution available for your architecture is really coarse. You could
possibly get a better resolution by creating a trace clock specific for
your architecture.

Mathieu

* Mike McTernan (mmcternan at airvana.com) wrote:
> Hi,
> 
> I'm using Linux 2.6.28.2 + LTTng 0.88 on an ARM Freescale i.MX51 board.
> 
> 
> The system works quite well - in fact LTTng is awesome!  But there's a
> problem that the output in LTTV shows run_timer_softirq() taking almost
> all the time between timer ticks.  HZ is set to 100 so the timer IRQ
> runs every 10ms, with run_timer_softirq() typically taking about 9.8ms
> as measured in LTTV.
> 
> I added some trace_mark() calls to find the consumer of this time to be
> trace_clock_update().  Specifically the atomic update:
> 
>   n = 0;
>   
>   do {
>     n++;
>     
>     old_clock = atomic_long_read(&trace_clock);
>     new_clock = (old_clock + (ticks << TRACE_CLOCK_SHIFT))
>       & (~((1 << TRACE_CLOCK_SHIFT) - 1));             
>   } while (atomic_long_cmpxchg(&trace_clock, old_clock, new_clock)
>              != old_clock);
> 
>   trace_mark(trace_clock_update, exit, "exit %u", n);
> 
> This never appears to loop as the "exit" value I added is always 1.
> However I notice that on ARM atomic_long_cmpxchg() contains a loop, and
> so is presumably racing around in there.
> 
> I'm not sure if this something completely unexpected, or normal
> behaviour?  Looking through the code I can't see that trace_clock is
> updated in many cases, although my grep maybe off.  I wondered if the
> ARM atomic_long_cmpxchg() is broken, but didn't find much there either.
> 
> Since I'm on a uniprocessor system, could I safely modify the update
> function to just atomically update the trace_clock without using the
> cmpxchg() operation since it's called in the soft IRQ?
>  
> Regards,
> 
> Mike
> 
> 
> _______________________________________________
> ltt-dev mailing list
> ltt-dev at lists.casi.polymtl.ca
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
> 

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

[ltt-dev] trace_clock_update() spinning

[Mike McTernan]

> Mathieu Desnoyers wrote:
>
> So it's not that LTTng itself is taking time, but that the trace clock
> resolution available for your architecture is really coarse. 

Excellent!  I understand what that means now.  Everything in the trace
is crushed up using the bottom 13-bits of the counter advanced with each
marker, then at the instance the upper bits are updated on the timer
tick the output appears to jump along making it _look_ like the update
function took all the remaining time.  What a fool I was!

> You could possibly get a better resolution by creating a trace clock
> specific for your architecture.

I'm pretty sure there's a general purpose timer hanging around somewhere
on this board if I can't find some sort of performance counter register
with which to improve things.

Many Thanks,

Mike

[ltt-dev] trace_clock_update() spinning

[Mike McTernan]

* Mathieu Desnoyers wrote:
> You could possibly get a better resolution by creating a trace clock
> specific for your architecture.

So I took this advice.  The Freescale i.MX51 has a Cortex A8 processor,
so has the same ARM performance counters available as the OMAP3.  In
fact I think these counters exist on all ARMs from ARM11 onwards - I'm
not sure if later LTT patches provide generic ARM support, but that
would be nice (except the platform specifics such as the broken bit 31
on OMAP!).  

Anyway, the results are brilliant.  Using LTTng I've quickly found some
areas of poor performance in our own drivers (excessive interrupts &
busy waiting), and also some scheduler mal-configuration with SCHED_FIFO
threads.  Thanks for this great tool!

A patch of my changes follows.  I'm not proposing it for inclusion in
LTTng, just optimistically publishing it in case others using i.MX51
could benefit.  It's based on the 2.6.28.2 kernel (Freescale's ER9 BSP)
patched with the 0.88 LTTng patch set.

Kind Regards,

Mike

diff --exclude=.svn -Naur
linux-2.6.28.mx51-er9.clean/arch/arm/plat-mxc/Makefile
linux-2.6.28.mx51-er9/arch/arm/plat-mxc/Makefile
--- linux-2.6.28.mx51-er9.clean/arch/arm/plat-mxc/Makefile
2010-02-04 19:19:29.980122357 +0000
+++ linux-2.6.28.mx51-er9/arch/arm/plat-mxc/Makefile	2010-02-02
21:45:13.028732364 +0000
@@ -56,3 +56,6 @@
 obj-y += serialxc.o
 endif
 endif
+
+# LTTng trace clock
+obj-$(CONFIG_HAVE_TRACE_CLOCK) += trace-clock.o
diff --exclude=.svn -Naur
linux-2.6.28.mx51-er9.clean/arch/arm/plat-mxc/trace-clock.c
linux-2.6.28.mx51-er9/arch/arm/plat-mxc/trace-clock.c
--- linux-2.6.28.mx51-er9.clean/arch/arm/plat-mxc/trace-clock.c
1970-01-01 01:00:00.000000000 +0100
+++ linux-2.6.28.mx51-er9/arch/arm/plat-mxc/trace-clock.c
2010-02-04 19:14:26.302127386 +0000
@@ -0,0 +1,90 @@
+/*
+ * kernel/arch/arm/plat-mxc/trace-clock.c
+ *
+ * (C) Copyright	2010 -
+ * 		Michael McTernan (Michael.McTernan at airvana.com)
+ *
+ * Tracing clock for i.MX51.
+ */
+
+#include <linux/module.h>
+#include <linux/ioport.h>
+#include <linux/init.h>
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/trace-clock.h>
+#include <mach/hardware.h>
+
+static int trace_clock_refcount;
+static DEFINE_MUTEX(trace_clock_mutex);
+
+unsigned long trace_clk_rate;
+
+static void enable_trace_clock(void)
+{
+  unsigned int r1;
+
+  /* Start perf counters */
+  r1 = 0;
+  asm("mrc p15, 0, %0, c9, c12, 0" : "=r" (r1));
+  r1 |= 0x1; /* enable counters */
+  asm("mcr p15, 0, %0, c9, c12, 0" : : "r" (r1));
+
+   /* Enable cycle counter */
+   r1 = 0x80000000;
+   asm("mcr p15, 0, %0, c9, c12, 1" : : "r" (r1));
+}
+
+static void disable_trace_clock(void)
+{
+  /* Disable cycle counter */
+  unsigned int r1 = 0x80000000;
+  asm("mcr p15, 0, %0, c9, c12, 2" : : "r" (r1));
+}
+
+void get_trace_clock(void)
+{
+	get_synthetic_tsc();
+	mutex_lock(&trace_clock_mutex);
+	if (trace_clock_refcount++)
+		goto end;
+	enable_trace_clock();
+end:
+	mutex_unlock(&trace_clock_mutex);
+}
+EXPORT_SYMBOL_GPL(get_trace_clock);
+
+void put_trace_clock(void)
+{
+	mutex_lock(&trace_clock_mutex);
+	WARN_ON(trace_clock_refcount <= 0);
+	if (trace_clock_refcount != 1)
+		goto end;
+	disable_trace_clock();
+end:
+	trace_clock_refcount--;
+	mutex_unlock(&trace_clock_mutex);
+	put_synthetic_tsc();
+}
+EXPORT_SYMBOL_GPL(put_trace_clock);
+
+
+static int __init init_trace_clock(void)
+{
+  struct clk *cpu_clk;
+
+  /* Get the CPU clock */
+  cpu_clk = clk_get(NULL, "cpu_clk");
+  if(IS_ERR(cpu_clk)) {
+    return -ENOMEM;
+  }
+
+  trace_clk_rate = clk_get_rate(cpu_clk);
+
+  clk_put(cpu_clk);
+
+  return 0;
+}
+
+early_initcall(init_trace_clock);
+
diff --exclude=.svn -Naur
linux-2.6.28.mx51-er9.clean/include/asm-arm/trace-clock.h
linux-2.6.28.mx51-er9/include/asm-arm/trace-clock.h
--- linux-2.6.28.mx51-er9.clean/include/asm-arm/trace-clock.h
1970-01-01 01:00:00.000000000 +0100
+++ linux-2.6.28.mx51-er9/include/asm-arm/trace-clock.h	2010-02-03
13:07:15.324714176 +0000
@@ -0,0 +1,65 @@
+#ifndef _ASM_ARM_TRACE_CLOCK_H
+#define _ASM_ARM_TRACE_CLOCK_H
+
+/*
+ * include/asm-generic/trace-clock.h
+ *
+ * Copyright (C) 2010 - Michael McTernan (mmcternan at airvana.com)
+ *
+ * Tracing clock for i.MX51.
+ */
+
+#include <asm/atomic.h>
+
+extern unsigned long trace_clk_rate;
+
+static inline u32 trace_clock_read32(void)
+{
+  u32 val;
+
+  __asm__ __volatile__ ("mrc p15, 0, %0, c9, c13, 0" : "=r" (val));
+
+  return val;
+}
+
+#ifdef CONFIG_HAVE_TRACE_CLOCK_32_TO_64
+extern u64 trace_clock_read_synthetic_tsc(void);
+extern void get_synthetic_tsc(void);
+extern void put_synthetic_tsc(void);
+
+static inline u64 trace_clock_read64(void)
+{
+	return trace_clock_read_synthetic_tsc();
+}
+#else
+static inline void get_synthetic_tsc(void)
+{
+}
+
+static inline void put_synthetic_tsc(void)
+{
+}
+
+static inline u64 trace_clock_read64(void)
+{
+	return trace_clock_read32();
+}
+#endif
+
+static inline unsigned int trace_clock_frequency(void)
+{
+	return trace_clk_rate;
+}
+
+static inline u32 trace_clock_freq_scale(void)
+{
+	return 1;
+}
+
+extern void get_trace_clock(void);
+extern void put_trace_clock(void);
+
+static inline void set_trace_clock_is_sync(int state)
+{
+}
+#endif /* _ASM_ARM_TRACE_CLOCK_H */
diff --exclude=.svn -Naur linux-2.6.28.mx51-er9.clean/init/Kconfig
linux-2.6.28.mx51-er9/init/Kconfig
--- linux-2.6.28.mx51-er9.clean/init/Kconfig	2010-02-05
08:01:53.582861126 +0000
+++ linux-2.6.28.mx51-er9/init/Kconfig	2010-02-02 21:43:08.340585489
+0000
@@ -344,7 +344,7 @@
 # Architectures with a specialized tracing clock should select this.
 #
 config HAVE_TRACE_CLOCK
-	def_bool n
+	def_bool y
 
 config HAVE_TRACE_CLOCK_GENERIC
 	bool
@@ -356,7 +356,7 @@
 # Architectures with only a 32-bits clock source should select this.
 #
 config HAVE_TRACE_CLOCK_32_TO_64
-	def_bool n
+	def_bool y
 
 #
 # Architectures which need to dynamically detect if their TSC is
unsynchronized


--
Mike

[ltt-dev] trace_clock_update() spinning

[Mathieu Desnoyers]

* Mike McTernan (mmcternan at airvana.com) wrote:
> * Mathieu Desnoyers wrote:
> > You could possibly get a better resolution by creating a trace clock
> > specific for your architecture.
> 
> So I took this advice.  The Freescale i.MX51 has a Cortex A8 processor,
> so has the same ARM performance counters available as the OMAP3.  In
> fact I think these counters exist on all ARMs from ARM11 onwards - I'm
> not sure if later LTT patches provide generic ARM support, but that
> would be nice (except the platform specifics such as the broken bit 31
> on OMAP!).  
> 
> Anyway, the results are brilliant.  Using LTTng I've quickly found some
> areas of poor performance in our own drivers (excessive interrupts &
> busy waiting), and also some scheduler mal-configuration with SCHED_FIFO
> threads.  Thanks for this great tool!
> 
> A patch of my changes follows.  I'm not proposing it for inclusion in
> LTTng, just optimistically publishing it in case others using i.MX51
> could benefit.  It's based on the 2.6.28.2 kernel (Freescale's ER9 BSP)
> patched with the 0.88 LTTng patch set.

Hi Mike,

Thanks for the patch!

A few questions:

- Does your architecture support power management sleep modes ? If yes, does the
  cycle counter stop in sleep modes ?
- Does your architecture support dynamic ferquency scaling ?
- Would you agree for me to integrate it in the lttng project and transfer
  non-exclusive relicensing rights to the LTTng maintainer ? Basically, the idea
  is to release it under dual GPLv2/LGPLv2.1 licenses, but I start to think that
  it might be just easier to use the gdb approach for LTTng contributions, where
  the project maintainers have the freedom to decide license changes. This way
  we would not have to ask permission from everyone if we need to do a license
  change for the LTTng project overall.

Thanks,

Mathieu

> 
> Kind Regards,
> 
> Mike
> 
> diff --exclude=.svn -Naur
> linux-2.6.28.mx51-er9.clean/arch/arm/plat-mxc/Makefile
> linux-2.6.28.mx51-er9/arch/arm/plat-mxc/Makefile
> --- linux-2.6.28.mx51-er9.clean/arch/arm/plat-mxc/Makefile
> 2010-02-04 19:19:29.980122357 +0000
> +++ linux-2.6.28.mx51-er9/arch/arm/plat-mxc/Makefile	2010-02-02
> 21:45:13.028732364 +0000
> @@ -56,3 +56,6 @@
>  obj-y += serialxc.o
>  endif
>  endif
> +
> +# LTTng trace clock
> +obj-$(CONFIG_HAVE_TRACE_CLOCK) += trace-clock.o
> diff --exclude=.svn -Naur
> linux-2.6.28.mx51-er9.clean/arch/arm/plat-mxc/trace-clock.c
> linux-2.6.28.mx51-er9/arch/arm/plat-mxc/trace-clock.c
> --- linux-2.6.28.mx51-er9.clean/arch/arm/plat-mxc/trace-clock.c
> 1970-01-01 01:00:00.000000000 +0100
> +++ linux-2.6.28.mx51-er9/arch/arm/plat-mxc/trace-clock.c
> 2010-02-04 19:14:26.302127386 +0000
> @@ -0,0 +1,90 @@
> +/*
> + * kernel/arch/arm/plat-mxc/trace-clock.c
> + *
> + * (C) Copyright	2010 -
> + * 		Michael McTernan (Michael.McTernan at airvana.com)
> + *
> + * Tracing clock for i.MX51.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/ioport.h>
> +#include <linux/init.h>
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/trace-clock.h>
> +#include <mach/hardware.h>
> +
> +static int trace_clock_refcount;
> +static DEFINE_MUTEX(trace_clock_mutex);
> +
> +unsigned long trace_clk_rate;
> +
> +static void enable_trace_clock(void)
> +{
> +  unsigned int r1;
> +
> +  /* Start perf counters */
> +  r1 = 0;
> +  asm("mrc p15, 0, %0, c9, c12, 0" : "=r" (r1));
> +  r1 |= 0x1; /* enable counters */
> +  asm("mcr p15, 0, %0, c9, c12, 0" : : "r" (r1));
> +
> +   /* Enable cycle counter */
> +   r1 = 0x80000000;
> +   asm("mcr p15, 0, %0, c9, c12, 1" : : "r" (r1));
> +}
> +
> +static void disable_trace_clock(void)
> +{
> +  /* Disable cycle counter */
> +  unsigned int r1 = 0x80000000;
> +  asm("mcr p15, 0, %0, c9, c12, 2" : : "r" (r1));
> +}
> +
> +void get_trace_clock(void)
> +{
> +	get_synthetic_tsc();
> +	mutex_lock(&trace_clock_mutex);
> +	if (trace_clock_refcount++)
> +		goto end;
> +	enable_trace_clock();
> +end:
> +	mutex_unlock(&trace_clock_mutex);
> +}
> +EXPORT_SYMBOL_GPL(get_trace_clock);
> +
> +void put_trace_clock(void)
> +{
> +	mutex_lock(&trace_clock_mutex);
> +	WARN_ON(trace_clock_refcount <= 0);
> +	if (trace_clock_refcount != 1)
> +		goto end;
> +	disable_trace_clock();
> +end:
> +	trace_clock_refcount--;
> +	mutex_unlock(&trace_clock_mutex);
> +	put_synthetic_tsc();
> +}
> +EXPORT_SYMBOL_GPL(put_trace_clock);
> +
> +
> +static int __init init_trace_clock(void)
> +{
> +  struct clk *cpu_clk;
> +
> +  /* Get the CPU clock */
> +  cpu_clk = clk_get(NULL, "cpu_clk");
> +  if(IS_ERR(cpu_clk)) {
> +    return -ENOMEM;
> +  }
> +
> +  trace_clk_rate = clk_get_rate(cpu_clk);
> +
> +  clk_put(cpu_clk);
> +
> +  return 0;
> +}
> +
> +early_initcall(init_trace_clock);
> +
> diff --exclude=.svn -Naur
> linux-2.6.28.mx51-er9.clean/include/asm-arm/trace-clock.h
> linux-2.6.28.mx51-er9/include/asm-arm/trace-clock.h
> --- linux-2.6.28.mx51-er9.clean/include/asm-arm/trace-clock.h
> 1970-01-01 01:00:00.000000000 +0100
> +++ linux-2.6.28.mx51-er9/include/asm-arm/trace-clock.h	2010-02-03
> 13:07:15.324714176 +0000
> @@ -0,0 +1,65 @@
> +#ifndef _ASM_ARM_TRACE_CLOCK_H
> +#define _ASM_ARM_TRACE_CLOCK_H
> +
> +/*
> + * include/asm-generic/trace-clock.h
> + *
> + * Copyright (C) 2010 - Michael McTernan (mmcternan at airvana.com)
> + *
> + * Tracing clock for i.MX51.
> + */
> +
> +#include <asm/atomic.h>
> +
> +extern unsigned long trace_clk_rate;
> +
> +static inline u32 trace_clock_read32(void)
> +{
> +  u32 val;
> +
> +  __asm__ __volatile__ ("mrc p15, 0, %0, c9, c13, 0" : "=r" (val));
> +
> +  return val;
> +}
> +
> +#ifdef CONFIG_HAVE_TRACE_CLOCK_32_TO_64
> +extern u64 trace_clock_read_synthetic_tsc(void);
> +extern void get_synthetic_tsc(void);
> +extern void put_synthetic_tsc(void);
> +
> +static inline u64 trace_clock_read64(void)
> +{
> +	return trace_clock_read_synthetic_tsc();
> +}
> +#else
> +static inline void get_synthetic_tsc(void)
> +{
> +}
> +
> +static inline void put_synthetic_tsc(void)
> +{
> +}
> +
> +static inline u64 trace_clock_read64(void)
> +{
> +	return trace_clock_read32();
> +}
> +#endif
> +
> +static inline unsigned int trace_clock_frequency(void)
> +{
> +	return trace_clk_rate;
> +}
> +
> +static inline u32 trace_clock_freq_scale(void)
> +{
> +	return 1;
> +}
> +
> +extern void get_trace_clock(void);
> +extern void put_trace_clock(void);
> +
> +static inline void set_trace_clock_is_sync(int state)
> +{
> +}
> +#endif /* _ASM_ARM_TRACE_CLOCK_H */
> diff --exclude=.svn -Naur linux-2.6.28.mx51-er9.clean/init/Kconfig
> linux-2.6.28.mx51-er9/init/Kconfig
> --- linux-2.6.28.mx51-er9.clean/init/Kconfig	2010-02-05
> 08:01:53.582861126 +0000
> +++ linux-2.6.28.mx51-er9/init/Kconfig	2010-02-02 21:43:08.340585489
> +0000
> @@ -344,7 +344,7 @@
>  # Architectures with a specialized tracing clock should select this.
>  #
>  config HAVE_TRACE_CLOCK
> -	def_bool n
> +	def_bool y
>  
>  config HAVE_TRACE_CLOCK_GENERIC
>  	bool
> @@ -356,7 +356,7 @@
>  # Architectures with only a 32-bits clock source should select this.
>  #
>  config HAVE_TRACE_CLOCK_32_TO_64
> -	def_bool n
> +	def_bool y
>  
>  #
>  # Architectures which need to dynamically detect if their TSC is
> unsynchronized
> 
> 
> --
> Mike

[ltt-dev] trace_clock_update() spinning

[Josh Boyer]

On Fri, Feb 05, 2010 at 10:16:13AM -0500, Mathieu Desnoyers wrote:
>Hi Mike,
>- Would you agree for me to integrate it in the lttng project and transfer
>  non-exclusive relicensing rights to the LTTng maintainer ? Basically, the idea
>  is to release it under dual GPLv2/LGPLv2.1 licenses, but I start to think that
>  it might be just easier to use the gdb approach for LTTng contributions, where
>  the project maintainers have the freedom to decide license changes. This way
>  we would not have to ask permission from everyone if we need to do a license
>  change for the LTTng project overall.

That's called copyright assignment, and I think you might want to discuss what
is really needed from a legal perspective with someone if you wish to go that
route.  The FSF has paperwork you have to have on-file with them for contributions.
I would expect something similar is needed (though I am certainly not a lawyer
and this is not legal advice).

josh

[ltt-dev] trace_clock_update() spinning

[Mathieu Desnoyers]

* Josh Boyer (jwboyer at gmail.com) wrote:
> On Fri, Feb 05, 2010 at 10:16:13AM -0500, Mathieu Desnoyers wrote:
> >Hi Mike,
> >- Would you agree for me to integrate it in the lttng project and transfer
> >  non-exclusive relicensing rights to the LTTng maintainer ? Basically, the idea
> >  is to release it under dual GPLv2/LGPLv2.1 licenses, but I start to think that
> >  it might be just easier to use the gdb approach for LTTng contributions, where
> >  the project maintainers have the freedom to decide license changes. This way
> >  we would not have to ask permission from everyone if we need to do a license
> >  change for the LTTng project overall.
> 
> That's called copyright assignment, and I think you might want to discuss what
> is really needed from a legal perspective with someone if you wish to go that
> route.  The FSF has paperwork you have to have on-file with them for contributions.
> I would expect something similar is needed (though I am certainly not a lawyer
> and this is not legal advice).
> 
> josh

Thanks for the hint. Well then maybe it's easier to just ask for GPLv2/LGPLv2.1
licensing. I am trying to find out what can be the easiest way to proceed both
for now and for the future.

Thanks,

Mathieu

[ltt-dev] trace_clock_update() spinning

[Mike McTernan]

* Mathieu Desnoyers wrote:
> - Does your architecture support power management sleep modes ? If
yes,
> does the cycle counter stop in sleep modes ?
> - Does your architecture support dynamic ferquency scaling ?

Yes and yes and yes.  So as per your paper on exotic embedded
architectures it would need to use a timer to work out real time after a
sleep period like on OMAP3.  Making that stuff generic (like the 32 to
64 code) would be great.

Luckily in my application we disable DVFS and power management so this
is ok.

Alternatively i.MX51 has a programmable timer that runs reasonably fast
(66.5MHz on my setup) and is independent of power saving.  It's called
the EPIT, and I had that working too, but used the cycle count for
greater resolution and for it's similarities with OMAP3.

> - Would you agree for me to integrate it in the lttng project and
transfer
> non-exclusive relicensing rights to the LTTng maintainer ? Basically,
> the idea is to release it under dual GPLv2/LGPLv2.1 licenses, but I
> start to think that it might be just easier to use the gdb approach
for
> LTTng contributions, where the project maintainers have the freedom to
> decide license changes. 
 
The patch is small and unspecial.  I'd be happy to reassign the
copyright in this case.  

> This way we would not have to ask permission from everyone if we need
to
> do a license change for the LTTng project overall.

I wouldn't be too happy with that if I were to submit something
substantial.  

Kind Regards,

Mike

[ltt-dev] trace_clock_update() spinning

[Mathieu Desnoyers]

* Mike McTernan (mmcternan at airvana.com) wrote:
> * Mathieu Desnoyers wrote:
> > - Does your architecture support power management sleep modes ? If
> yes,
> > does the cycle counter stop in sleep modes ?
> > - Does your architecture support dynamic ferquency scaling ?
> 
> Yes and yes and yes.  So as per your paper on exotic embedded
> architectures it would need to use a timer to work out real time after a
> sleep period like on OMAP3.  Making that stuff generic (like the 32 to
> 64 code) would be great.
> 
> Luckily in my application we disable DVFS and power management so this
> is ok.

Could you update the patch so it uses the generic timestamp if DVFS or PM are
enabled in the kernel config ?

> 
> Alternatively i.MX51 has a programmable timer that runs reasonably fast
> (66.5MHz on my setup) and is independent of power saving.  It's called
> the EPIT, and I had that working too, but used the cycle count for
> greater resolution and for it's similarities with OMAP3.

Usually, anything that is outside of the CPU is terribly slow to read (memory
mapped I/O). So using the TSC seems to be the best solution.

> 
> > - Would you agree for me to integrate it in the lttng project and
> transfer
> > non-exclusive relicensing rights to the LTTng maintainer ? Basically,
> > the idea is to release it under dual GPLv2/LGPLv2.1 licenses, but I
> > start to think that it might be just easier to use the gdb approach
> for
> > LTTng contributions, where the project maintainers have the freedom to
> > decide license changes. 
>  
> The patch is small and unspecial.  I'd be happy to reassign the
> copyright in this case.  
> 
> > This way we would not have to ask permission from everyone if we need
> to
> > do a license change for the LTTng project overall.
> 
> I wouldn't be too happy with that if I were to submit something
> substantial.  

Well, I'm happy with whatever is the less trouble for everyone and ensures that
the code stays open. So GPLv2/LGPLv2.1 is fine. Just as a thought, if you
copy-pasted code from the other architectures into these headers, perharps it
would be better to leave the copyright notices already in place in addition to
yours. That would help copyright traceability. ;)

Thanks !

Mathieu


> 
> Kind Regards,
> 
> Mike

[ltt-dev] trace_clock_update() spinning

[Mike McTernan]

* Mathieu Desnoyers wrote:
> * Mike McTernan (mmcternan at airvana.com) wrote:
> > Luckily in my application we disable DVFS and power management so
this
> > is ok.
> 
> Could you update the patch so it uses the generic timestamp if DVFS or
PM
> are enabled in the kernel config ?

I would like to say yes, but in truth I have neither the time or means
to test this.  Note I didn't submit the patch requesting inclusion,
merely to perhaps help others considering use of LTTng on iMX51's and
similar.

That said, I passed the patch to Freescale and asked that they consider
getting LTTng supported in their BSP as it is such an excellent tool.

> > Alternatively i.MX51 has a programmable timer that runs reasonably
fast
> > (66.5MHz on my setup) and is independent of power saving.  It's
called
> > the EPIT, and I had that working too, but used the cycle count for
> > greater resolution and for it's similarities with OMAP3.
> 
> Usually, anything that is outside of the CPU is terribly slow to read
> (memory mapped I/O). So using the TSC seems to be the best solution.

I agree that the TSC is better, also because its use could be
generalised for all ARM Cortex A8's and probably later ARMs.

> Well, I'm happy with whatever is the less trouble for everyone and
ensures
> that the code stays open. So GPLv2/LGPLv2.1 is fine. 

That's a good outlook & keeping it GPL or LGPL is cool.  My only worry
about reassignment would be that a future version could be under a less
permissive license. 

> Just as a thought, if you copy-pasted code from the other
architectures
> into these headers, perharps it would be better to leave the copyright
> notices already in place in addition to yours. That would help
copyright
> traceability. ;)

The assembly is derivative from the ARM reference, which will be the
same on every Cortex A8 (bar chip errata like the broken bit 31 on
OMAP):

http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0344j/Bgb
deggf.html

The empty functions to complete the interface follow from the generic
implementation, as they must in order to compile.  The mutex & reference
count to safely enable/disable the clock is the same as the generic
code; I should probably credit you for those 8 lines.

Regards,

Mike

[ltt-dev] trace_clock_update() spinning

[Mathieu Desnoyers]

* Mike McTernan (mmcternan at airvana.com) wrote:
> * Mathieu Desnoyers wrote:
> > * Mike McTernan (mmcternan at airvana.com) wrote:
> > > Luckily in my application we disable DVFS and power management so
> this
> > > is ok.
> > 
> > Could you update the patch so it uses the generic timestamp if DVFS or
> PM
> > are enabled in the kernel config ?
> 
> I would like to say yes, but in truth I have neither the time or means
> to test this.  Note I didn't submit the patch requesting inclusion,
> merely to perhaps help others considering use of LTTng on iMX51's and
> similar.
> 
> That said, I passed the patch to Freescale and asked that they consider
> getting LTTng supported in their BSP as it is such an excellent tool.

Yep, I understand your time constraints. Thanks for passing this on to
Freescale.

> 
> > > Alternatively i.MX51 has a programmable timer that runs reasonably
> fast
> > > (66.5MHz on my setup) and is independent of power saving.  It's
> called
> > > the EPIT, and I had that working too, but used the cycle count for
> > > greater resolution and for it's similarities with OMAP3.
> > 
> > Usually, anything that is outside of the CPU is terribly slow to read
> > (memory mapped I/O). So using the TSC seems to be the best solution.
> 
> I agree that the TSC is better, also because its use could be
> generalised for all ARM Cortex A8's and probably later ARMs.
> 
> > Well, I'm happy with whatever is the less trouble for everyone and
> ensures
> > that the code stays open. So GPLv2/LGPLv2.1 is fine. 
> 
> That's a good outlook & keeping it GPL or LGPL is cool.  My only worry
> about reassignment would be that a future version could be under a less
> permissive license. 
> 
> > Just as a thought, if you copy-pasted code from the other
> architectures
> > into these headers, perharps it would be better to leave the copyright
> > notices already in place in addition to yours. That would help
> copyright
> > traceability. ;)
> 
> The assembly is derivative from the ARM reference, which will be the
> same on every Cortex A8 (bar chip errata like the broken bit 31 on
> OMAP):
> 
> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0344j/Bgb
> deggf.html
> 
> The empty functions to complete the interface follow from the generic
> implementation, as they must in order to compile.  The mutex & reference
> count to safely enable/disable the clock is the same as the generic
> code; I should probably credit you for those 8 lines.

Well, if it's "trivial", then credits are unnecessary, but I've always stayed on
the safe side license-wise. It makes it easier in the future if we have to dig
back into these patches.

Thanks !

Mathieu

> 
> Regards,
> 
> Mike
>