Re: [PATCH 1/3] tracing,x86: add a TSC trace_clock; reset buffer onclock change

[Steven Rostedt]

On Tue, 2012-09-11 at 19:41 -0700, David Sharp wrote:
> In order to promote interoperability between userspace tracers and ftrace,
> add a trace_clock that reports raw TSC values which will then be recorded
> in the ring buffer. Userspace tracers that also record TSCs are then on
> exactly the same time base as the kernel and events can be unambiguously
> interlaced.
> 
> Tested: Enabled a tracepoint and the "tsc" trace_clock and saw very large
> timestamp values.
> 
> Google-Bug-Id: 6980623
> Signed-off-by: David Sharp <dhsharp@xxxxxxxxxx>
> ---
>  include/linux/trace_clock.h |    3 +++
>  kernel/trace/trace.c        |    3 +++
>  kernel/trace/trace_clock.c  |   16 ++++++++++++++++
>  3 files changed, 22 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/trace_clock.h b/include/linux/trace_clock.h
> index 4eb4902..b86c7363 100644
> --- a/include/linux/trace_clock.h
> +++ b/include/linux/trace_clock.h
> @@ -16,5 +16,8 @@ extern u64 notrace trace_clock_local(void);
>  extern u64 notrace trace_clock(void);
>  extern u64 notrace trace_clock_global(void);
>  extern u64 notrace trace_clock_counter(void);
> +#ifdef CONFIG_X86_TSC
> +extern u64 notrace trace_clock_tsc(void);
> +#endif
>  
>  #endif /* _LINUX_TRACE_CLOCK_H */
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 5c38c81..dc1f1fa 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -480,6 +480,9 @@ static struct {
>  	{ trace_clock_local,	"local" },
>  	{ trace_clock_global,	"global" },
>  	{ trace_clock_counter,	"counter" },
> +#ifdef CONFIG_X86_TSC
> +	{ trace_clock_tsc,	"tsc" },
> +#endif
>  };

I really hate adding arch defs in generic code. Perhaps what we could do
is add a define here. Something like:

#ifndef ARCH_TRACE_CLOCKS
# define ARCH_TRACE_CLOCKS
#endif

[...]
	{ trace_clock_counter, "counter" },
	ARCH_TRACE_CLOCKS
};

and have ARCH_TRACE_CLOCKS defined somewhere in an arch specific header.
Not sure what header we could use though :-/

That is, in a header have:

#define ARCH_TRACE_CLOCKS \
	{ trace_clock_x86_tsc,	"tsc" },

and also define trace_clock_x86_tsc in arch/x86/kernel...

-- Steve


>  
>  int trace_clock_id;
> diff --git a/kernel/trace/trace_clock.c b/kernel/trace/trace_clock.c
> index 3947835..1770737 100644
> --- a/kernel/trace/trace_clock.c
> +++ b/kernel/trace/trace_clock.c
> @@ -125,3 +125,19 @@ u64 notrace trace_clock_counter(void)
>  {
>  	return atomic64_add_return(1, &trace_counter);
>  }
> +
> +#ifdef CONFIG_X86_TSC
> +/*
> + * trace_clock_tsc(): A clock that is just the cycle counter.
> + *
> + * Unlike the other clocks, this is not in nanoseconds.
> + */
> +u64 notrace trace_clock_tsc(void)
> +{
> +	u64 ret;
> +	rdtsc_barrier();
> +	rdtscll(ret);
> +
> +	return ret;
> +}
> +#endif


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