[PATCH 1/4] perf tools: callchain: Warn only once in empty node detection

[PATCH 1/4] perf tools: callchain: Warn only once in empty node detection

[Frederic Weisbecker]

When we fill a new node that is found to be empty (sign of a bug),
we print a message each time that happens. Sometimes we reach thousands
of lines printed.

Just warn only once in that case (and use stderr instead of stdout).

Reported-by: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
---
 tools/perf/util/callchain.c |    3 ++-
 tools/perf/util/util.h      |    3 +++
 tools/perf/util/wrapper.c   |   16 ++++++++++++++++
 3 files changed, 21 insertions(+), 1 deletions(-)

diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index 98c5627..ce0c6d4 100644
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -13,6 +13,7 @@
 #include <stdio.h>
 #include <stdbool.h>
 #include <errno.h>
+#include "util.h"
 
 #include "callchain.h"
 
@@ -203,7 +204,7 @@ fill_node(struct callchain_node *node, struct ip_callchain *chain,
 	}
 	node->val_nr = chain->nr - start;
 	if (!node->val_nr)
-		printf("Warning: empty node in callchain tree\n");
+		fprintf_once(stderr, "Warning: empty node in callchain tree\n");
 }
 
 static void
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index 68fe157..386ea40 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -148,6 +148,9 @@ static inline const char *skip_prefix(const char *str, const char *prefix)
 	return strncmp(str, prefix, len) ? NULL : str + len;
 }
 
+extern int fprintf_once(FILE *fp, const char *fmt, ...)
+	__attribute__((format (printf, 2, 3)));
+
 #if defined(NO_MMAP) || defined(USE_WIN32_MMAP)
 
 #ifndef PROT_READ
diff --git a/tools/perf/util/wrapper.c b/tools/perf/util/wrapper.c
index 4574ac2..026c87c 100644
--- a/tools/perf/util/wrapper.c
+++ b/tools/perf/util/wrapper.c
@@ -205,3 +205,19 @@ int xmkstemp(char *template)
 		die("Unable to create temporary file: %s", strerror(errno));
 	return fd;
 }
+
+int fprintf_once(FILE *fp, const char *fmt, ...)
+{
+	va_list args;
+	int ret;
+	static int once;
+
+	if (once++)
+		return 1;
+
+	va_start(args, fmt);
+	ret = vfprintf(fp, fmt, args);
+	va_end(args);
+
+	return ret;
+}
-- 
1.6.2.3

[PATCH 1/3] perfcounter: Initialize tracepoint record before any use

[Frederic Weisbecker]

Despite that the tracepoint record is always present when the
PERF_SAMPLE_TP_RECORD flag is set, gcc whines, thinking it might not
be initialized:

kernel/perf_counter.c: In function ‘perf_counter_output’:
kernel/perf_counter.c:2650: warning: ‘tp’ may be used uninitialized in this function

Then, initialize it to NULL and always check if it's not NULL before
dereference it.

Reported-by: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
---
 kernel/perf_counter.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/kernel/perf_counter.c b/kernel/perf_counter.c
index 142ae5a..a6963e2 100644
--- a/kernel/perf_counter.c
+++ b/kernel/perf_counter.c
@@ -2647,7 +2647,7 @@ void perf_counter_output(struct perf_counter *counter, int nmi,
 		u64 counter;
 	} group_entry;
 	struct perf_callchain_entry *callchain = NULL;
-	struct perf_tracepoint_record *tp;
+	struct perf_tracepoint_record *tp = NULL;
 	int callchain_size = 0;
 	u64 time;
 	struct {
@@ -2718,7 +2718,8 @@ void perf_counter_output(struct perf_counter *counter, int nmi,
 
 	if (sample_type & PERF_SAMPLE_TP_RECORD) {
 		tp = data->private;
-		header.size += tp->size;
+		if (tp)
+			header.size += tp->size;
 	}
 
 	ret = perf_output_begin(&handle, counter, header.size, nmi, 1);
@@ -2784,7 +2785,7 @@ void perf_counter_output(struct perf_counter *counter, int nmi,
 		}
 	}
 
-	if (sample_type & PERF_SAMPLE_TP_RECORD)
+	if ((sample_type & PERF_SAMPLE_TP_RECORD) && tp)
 		perf_output_copy(&handle, tp->record, tp->size);
 
 	perf_output_end(&handle);
-- 
1.6.2.3

Re: [PATCH 1/3] perfcounter: Initialize tracepoint record before any use

[Frederic Weisbecker]

On Sat, Aug 08, 2009 at 04:26:35AM +0200, Frederic Weisbecker wrote:
> Despite that the tracepoint record is always present when the
> PERF_SAMPLE_TP_RECORD flag is set, gcc whines, thinking it might not
> be initialized:
> 
> kernel/perf_counter.c: In function ‘perf_counter_output’:
> kernel/perf_counter.c:2650: warning: ‘tp’ may be used uninitialized in this function
> 
> Then, initialize it to NULL and always check if it's not NULL before
> dereference it.
> 
> Reported-by: Ingo Molnar <mingo@elte.hu>
> Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Mike Galbraith <efault@gmx.de>
> Cc: Paul Mackerras <paulus@samba.org>
> ---


Hmm, sorry I've resent the perf tools series by mistake. Please
ignore the [PATCH */4] "perf tools: *" series that has already been
sent.

The only ones are [PATCH */3] perfcounter: *

Thanks.


>  kernel/perf_counter.c |    7 ++++---
>  1 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/perf_counter.c b/kernel/perf_counter.c
> index 142ae5a..a6963e2 100644
> --- a/kernel/perf_counter.c
> +++ b/kernel/perf_counter.c
> @@ -2647,7 +2647,7 @@ void perf_counter_output(struct perf_counter *counter, int nmi,
>  		u64 counter;
>  	} group_entry;
>  	struct perf_callchain_entry *callchain = NULL;
> -	struct perf_tracepoint_record *tp;
> +	struct perf_tracepoint_record *tp = NULL;
>  	int callchain_size = 0;
>  	u64 time;
>  	struct {
> @@ -2718,7 +2718,8 @@ void perf_counter_output(struct perf_counter *counter, int nmi,
>  
>  	if (sample_type & PERF_SAMPLE_TP_RECORD) {
>  		tp = data->private;
> -		header.size += tp->size;
> +		if (tp)
> +			header.size += tp->size;
>  	}
>  
>  	ret = perf_output_begin(&handle, counter, header.size, nmi, 1);
> @@ -2784,7 +2785,7 @@ void perf_counter_output(struct perf_counter *counter, int nmi,
>  		}
>  	}
>  
> -	if (sample_type & PERF_SAMPLE_TP_RECORD)
> +	if ((sample_type & PERF_SAMPLE_TP_RECORD) && tp)
>  		perf_output_copy(&handle, tp->record, tp->size);
>  
>  	perf_output_end(&handle);
> -- 
> 1.6.2.3
> 

[PATCH 4/3] perf_counter: Correct PERF_SAMPLE_RAW output

[Peter Zijlstra]

Subject: perf_counter: Correct PERF_SAMPLE_RAW output
From: Peter Zijlstra <a.p.zijlstra@chello.nl>
Date: Mon Aug 10 11:16:52 CEST 2009

PERF_SAMPLE_* output switches should unconditionally output the
correct format, as they are the only way to unambiguously parse the
PERF_EVENT_SAMPLE data.

Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
 include/linux/perf_counter.h |    2 ++
 include/trace/ftrace.h       |    3 ++-
 kernel/perf_counter.c        |   30 ++++++++++++++++++++++++------
 3 files changed, 28 insertions(+), 7 deletions(-)

Index: linux-2.6/include/linux/perf_counter.h
===================================================================
--- linux-2.6.orig/include/linux/perf_counter.h
+++ linux-2.6/include/linux/perf_counter.h
@@ -369,6 +369,8 @@ enum perf_event_type {
 	 *
 	 *	{ u64			nr,
 	 *	  u64			ips[nr];  } && PERF_SAMPLE_CALLCHAIN
+	 *	{ u32			size;
+	 *	  char                  data[size];}&& PERF_SAMPLE_RAW
 	 * };
 	 */
 	PERF_EVENT_SAMPLE		= 9,
Index: linux-2.6/include/trace/ftrace.h
===================================================================
--- linux-2.6.orig/include/trace/ftrace.h
+++ linux-2.6/include/trace/ftrace.h
@@ -687,7 +687,8 @@ static void ftrace_profile_##call(proto)
 	pc = preempt_count();						\
 									\
 	__data_size = ftrace_get_offsets_##call(&__data_offsets, args); \
-	__entry_size = ALIGN(__data_size + sizeof(*entry), sizeof(u64));\
+	__entry_size = ALIGN(__data_size + sizeof(*entry) + sizeof(u32),\
+			     sizeof(u64));				\
 									\
 	do {								\
 		char raw_data[__entry_size];				\
Index: linux-2.6/kernel/perf_counter.c
===================================================================
--- linux-2.6.orig/kernel/perf_counter.c
+++ linux-2.6/kernel/perf_counter.c
@@ -2647,7 +2647,6 @@ void perf_counter_output(struct perf_cou
 		u64 counter;
 	} group_entry;
 	struct perf_callchain_entry *callchain = NULL;
-	struct perf_raw_record *raw = NULL;
 	int callchain_size = 0;
 	u64 time;
 	struct {
@@ -2717,9 +2716,15 @@ void perf_counter_output(struct perf_cou
 	}
 
 	if (sample_type & PERF_SAMPLE_RAW) {
-		raw = data->raw;
-		if (raw)
-			header.size += raw->size;
+		int size = sizeof(u32);
+
+		if (data->raw)
+			size += data->raw->size;
+		else
+			size += sizeof(u32);
+
+		WARN_ON_ONCE(size & (sizeof(u64)-1));
+		header.size += size;
 	}
 
 	ret = perf_output_begin(&handle, counter, header.size, nmi, 1);
@@ -2785,8 +2790,21 @@ void perf_counter_output(struct perf_cou
 		}
 	}
 
-	if ((sample_type & PERF_SAMPLE_RAW) && raw)
-		perf_output_copy(&handle, raw->data, raw->size);
+	if (sample_type & PERF_SAMPLE_RAW) {
+		if (data->raw) {
+			perf_output_put(&handle, data->raw->size);
+			perf_output_copy(&handle, data->raw->data, data->raw->size);
+		} else {
+			struct {
+				u32	size;
+				u32	data;
+			} raw = {
+				.size = sizeof(u32),
+				.data = 0,
+			};
+			perf_output_put(&handle, raw);
+		}
+	}
 
 	perf_output_end(&handle);
 }

[tip:perfcounters/urgent] perf_counter: Correct PERF_SAMPLE_RAW output

[tip-bot for Peter Zijlstra]

Commit-ID:  a044560c3a1f0ad75ce685c1ed7604820b9ed319
Gitweb:     http://git.kernel.org/tip/a044560c3a1f0ad75ce685c1ed7604820b9ed319
Author:     Peter Zijlstra <a.p.zijlstra@chello.nl>
AuthorDate: Mon, 10 Aug 2009 11:16:52 +0200
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Mon, 10 Aug 2009 11:33:09 +0200

perf_counter: Correct PERF_SAMPLE_RAW output

PERF_SAMPLE_* output switches should unconditionally output the
correct format, as they are the only way to unambiguously parse
the PERF_EVENT_SAMPLE data.

Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
LKML-Reference: <1249896447.17467.74.camel@twins>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 include/linux/perf_counter.h |    2 ++
 include/trace/ftrace.h       |    3 ++-
 kernel/perf_counter.c        |   30 ++++++++++++++++++++++++------
 3 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/include/linux/perf_counter.h b/include/linux/perf_counter.h
index 2aabe43..a9d823a 100644
--- a/include/linux/perf_counter.h
+++ b/include/linux/perf_counter.h
@@ -369,6 +369,8 @@ enum perf_event_type {
 	 *
 	 *	{ u64			nr,
 	 *	  u64			ips[nr];  } && PERF_SAMPLE_CALLCHAIN
+	 *	{ u32			size;
+	 *	  char                  data[size];}&& PERF_SAMPLE_RAW
 	 * };
 	 */
 	PERF_EVENT_SAMPLE		= 9,
diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index 7fb16d9..7167b9b 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -685,7 +685,8 @@ static void ftrace_profile_##call(proto)				\
 	pc = preempt_count();						\
 									\
 	__data_size = ftrace_get_offsets_##call(&__data_offsets, args); \
-	__entry_size = ALIGN(__data_size + sizeof(*entry), sizeof(u64));\
+	__entry_size = ALIGN(__data_size + sizeof(*entry) + sizeof(u32),\
+			     sizeof(u64));				\
 									\
 	do {								\
 		char raw_data[__entry_size];				\
diff --git a/kernel/perf_counter.c b/kernel/perf_counter.c
index 546e62d..5229d16 100644
--- a/kernel/perf_counter.c
+++ b/kernel/perf_counter.c
@@ -2646,7 +2646,6 @@ static void perf_counter_output(struct perf_counter *counter, int nmi,
 		u64 counter;
 	} group_entry;
 	struct perf_callchain_entry *callchain = NULL;
-	struct perf_raw_record *raw = NULL;
 	int callchain_size = 0;
 	u64 time;
 	struct {
@@ -2716,9 +2715,15 @@ static void perf_counter_output(struct perf_counter *counter, int nmi,
 	}
 
 	if (sample_type & PERF_SAMPLE_RAW) {
-		raw = data->raw;
-		if (raw)
-			header.size += raw->size;
+		int size = sizeof(u32);
+
+		if (data->raw)
+			size += data->raw->size;
+		else
+			size += sizeof(u32);
+
+		WARN_ON_ONCE(size & (sizeof(u64)-1));
+		header.size += size;
 	}
 
 	ret = perf_output_begin(&handle, counter, header.size, nmi, 1);
@@ -2784,8 +2789,21 @@ static void perf_counter_output(struct perf_counter *counter, int nmi,
 		}
 	}
 
-	if ((sample_type & PERF_SAMPLE_RAW) && raw)
-		perf_output_copy(&handle, raw->data, raw->size);
+	if (sample_type & PERF_SAMPLE_RAW) {
+		if (data->raw) {
+			perf_output_put(&handle, data->raw->size);
+			perf_output_copy(&handle, data->raw->data, data->raw->size);
+		} else {
+			struct {
+				u32	size;
+				u32	data;
+			} raw = {
+				.size = sizeof(u32),
+				.data = 0,
+			};
+			perf_output_put(&handle, raw);
+		}
+	}
 
 	perf_output_end(&handle);
 }

Re: [PATCH 4/3] perf_counter: Correct PERF_SAMPLE_RAW output

[Frederic Weisbecker]

On Mon, Aug 10, 2009 at 11:27:27AM +0200, Peter Zijlstra wrote:
> Subject: perf_counter: Correct PERF_SAMPLE_RAW output
> From: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Date: Mon Aug 10 11:16:52 CEST 2009
> 
> PERF_SAMPLE_* output switches should unconditionally output the
> correct format, as they are the only way to unambiguously parse the
> PERF_EVENT_SAMPLE data.
> 
> Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>


I economized this part because I thought ftrace events could
be parsed the same way we parse binary events through debugfs.
The size of an ftrace event can be deduced from its format.

But I must confess that would require a pre-parsing of dynamic
strings.

Acked-by: Frederic Weisbecker <fweisbec@gmail.com>


> ---
>  include/linux/perf_counter.h |    2 ++
>  include/trace/ftrace.h       |    3 ++-
>  kernel/perf_counter.c        |   30 ++++++++++++++++++++++++------
>  3 files changed, 28 insertions(+), 7 deletions(-)
> 
> Index: linux-2.6/include/linux/perf_counter.h
> ===================================================================
> --- linux-2.6.orig/include/linux/perf_counter.h
> +++ linux-2.6/include/linux/perf_counter.h
> @@ -369,6 +369,8 @@ enum perf_event_type {
>  	 *
>  	 *	{ u64			nr,
>  	 *	  u64			ips[nr];  } && PERF_SAMPLE_CALLCHAIN
> +	 *	{ u32			size;
> +	 *	  char                  data[size];}&& PERF_SAMPLE_RAW
>  	 * };
>  	 */
>  	PERF_EVENT_SAMPLE		= 9,
> Index: linux-2.6/include/trace/ftrace.h
> ===================================================================
> --- linux-2.6.orig/include/trace/ftrace.h
> +++ linux-2.6/include/trace/ftrace.h
> @@ -687,7 +687,8 @@ static void ftrace_profile_##call(proto)
>  	pc = preempt_count();						\
>  									\
>  	__data_size = ftrace_get_offsets_##call(&__data_offsets, args); \
> -	__entry_size = ALIGN(__data_size + sizeof(*entry), sizeof(u64));\
> +	__entry_size = ALIGN(__data_size + sizeof(*entry) + sizeof(u32),\
> +			     sizeof(u64));				\
>  									\
>  	do {								\
>  		char raw_data[__entry_size];				\
> Index: linux-2.6/kernel/perf_counter.c
> ===================================================================
> --- linux-2.6.orig/kernel/perf_counter.c
> +++ linux-2.6/kernel/perf_counter.c
> @@ -2647,7 +2647,6 @@ void perf_counter_output(struct perf_cou
>  		u64 counter;
>  	} group_entry;
>  	struct perf_callchain_entry *callchain = NULL;
> -	struct perf_raw_record *raw = NULL;
>  	int callchain_size = 0;
>  	u64 time;
>  	struct {
> @@ -2717,9 +2716,15 @@ void perf_counter_output(struct perf_cou
>  	}
>  
>  	if (sample_type & PERF_SAMPLE_RAW) {
> -		raw = data->raw;
> -		if (raw)
> -			header.size += raw->size;
> +		int size = sizeof(u32);
> +
> +		if (data->raw)
> +			size += data->raw->size;
> +		else
> +			size += sizeof(u32);
> +
> +		WARN_ON_ONCE(size & (sizeof(u64)-1));
> +		header.size += size;
>  	}
>  
>  	ret = perf_output_begin(&handle, counter, header.size, nmi, 1);
> @@ -2785,8 +2790,21 @@ void perf_counter_output(struct perf_cou
>  		}
>  	}
>  
> -	if ((sample_type & PERF_SAMPLE_RAW) && raw)
> -		perf_output_copy(&handle, raw->data, raw->size);
> +	if (sample_type & PERF_SAMPLE_RAW) {
> +		if (data->raw) {
> +			perf_output_put(&handle, data->raw->size);
> +			perf_output_copy(&handle, data->raw->data, data->raw->size);
> +		} else {
> +			struct {
> +				u32	size;
> +				u32	data;
> +			} raw = {
> +				.size = sizeof(u32),
> +				.data = 0,
> +			};
> +			perf_output_put(&handle, raw);
> +		}
> +	}
>  
>  	perf_output_end(&handle);
>  }
> 

Re: [PATCH 4/3] perf_counter: Correct PERF_SAMPLE_RAW output

[Frederic Weisbecker]

On Mon, Aug 10, 2009 at 11:27:27AM +0200, Peter Zijlstra wrote:
> Subject: perf_counter: Correct PERF_SAMPLE_RAW output
> From: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Date: Mon Aug 10 11:16:52 CEST 2009
> 
> PERF_SAMPLE_* output switches should unconditionally output the
> correct format, as they are the only way to unambiguously parse the
> PERF_EVENT_SAMPLE data.
> 
> Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
> ---
>  include/linux/perf_counter.h |    2 ++
>  include/trace/ftrace.h       |    3 ++-
>  kernel/perf_counter.c        |   30 ++++++++++++++++++++++++------
>  3 files changed, 28 insertions(+), 7 deletions(-)
> 
> Index: linux-2.6/include/linux/perf_counter.h
> ===================================================================
> --- linux-2.6.orig/include/linux/perf_counter.h
> +++ linux-2.6/include/linux/perf_counter.h
> @@ -369,6 +369,8 @@ enum perf_event_type {
>  	 *
>  	 *	{ u64			nr,
>  	 *	  u64			ips[nr];  } && PERF_SAMPLE_CALLCHAIN
> +	 *	{ u32			size;
> +	 *	  char                  data[size];}&& PERF_SAMPLE_RAW
>  	 * };
>  	 */
>  	PERF_EVENT_SAMPLE		= 9,
> Index: linux-2.6/include/trace/ftrace.h
> ===================================================================
> --- linux-2.6.orig/include/trace/ftrace.h
> +++ linux-2.6/include/trace/ftrace.h
> @@ -687,7 +687,8 @@ static void ftrace_profile_##call(proto)
>  	pc = preempt_count();						\
>  									\
>  	__data_size = ftrace_get_offsets_##call(&__data_offsets, args); \
> -	__entry_size = ALIGN(__data_size + sizeof(*entry), sizeof(u64));\
> +	__entry_size = ALIGN(__data_size + sizeof(*entry) + sizeof(u32),\
> +			     sizeof(u64));				\



Here you are reserving a room for the size of the buffer inside the buffer.



>  									\
>  	do {								\
>  		char raw_data[__entry_size];				\
> Index: linux-2.6/kernel/perf_counter.c
> ===================================================================
> --- linux-2.6.orig/kernel/perf_counter.c
> +++ linux-2.6/kernel/perf_counter.c
> @@ -2647,7 +2647,6 @@ void perf_counter_output(struct perf_cou
>  		u64 counter;
>  	} group_entry;
>  	struct perf_callchain_entry *callchain = NULL;
> -	struct perf_raw_record *raw = NULL;
>  	int callchain_size = 0;
>  	u64 time;
>  	struct {
> @@ -2717,9 +2716,15 @@ void perf_counter_output(struct perf_cou
>  	}
>  
>  	if (sample_type & PERF_SAMPLE_RAW) {
> -		raw = data->raw;
> -		if (raw)
> -			header.size += raw->size;
> +		int size = sizeof(u32);
> +
> +		if (data->raw)
> +			size += data->raw->size;



And here you reserve a size for the buffer once again?



> +		else
> +			size += sizeof(u32);
> +
> +		WARN_ON_ONCE(size & (sizeof(u64)-1));
> +		header.size += size;
>  	}
>  
>  	ret = perf_output_begin(&handle, counter, header.size, nmi, 1);
> @@ -2785,8 +2790,21 @@ void perf_counter_output(struct perf_cou
>  		}
>  	}
>  
> -	if ((sample_type & PERF_SAMPLE_RAW) && raw)
> -		perf_output_copy(&handle, raw->data, raw->size);
> +	if (sample_type & PERF_SAMPLE_RAW) {
> +		if (data->raw) {
> +			perf_output_put(&handle, data->raw->size);
> +			perf_output_copy(&handle, data->raw->data, data->raw->size);



And actually you copy the buffer that has the size of the buffer
plus the u32 reserved for the size, whereas the size has already been
copied.

When I look at a perf sample raw it gives me the following:

.  0000:  09 00 00 00 01 00 54 00 d0 c7 00 81 ff ff ff ff  ......T........
.  0010:  69 01 00 00 69 01 00 00 e6 15 00 00 00 00 00 00  i...i..........
.  0020:  30 00 00 00 2b 00 01 02 69 01 00 00 69 01 00 00  0...+...i...i..

          ^  ^  ^  ^  ^  ^  ^  ^  ^  ^ .................
           Buf size   struct trace_entry



.  0030:  6b 6f 6e 64 65 6d 61 6e 64 2f 31 00 00 00 00 00  kondemand/1....

          ^  ^  ^  ^  ..................................
          Rest of struct ftrace_raw_<call>

.  0040:  69 01 00 00 70 82 46 81 ff ff ff ff 00 00 00 00  i...p.F........

          ..................................  ^  ^  ^  ^
                                              The room you have
                                              reserved for the buffer size
                                              (zeroed from a tmp patch)
.  0050:  00 00 00 00
          ^  ^  ^  ^
          padding from alignment (ALIGN(__data_size + sizeof(*entry) + sizeof(u32),\
			     sizeof(u64));

I first thought the 0x4c bytes was a padding from gcc because
sizeof(struct ftrace_raw_<call>) % 8 != 0
But no, it is equal to 0: It's between 0x4c and 0x24 = 40 bytes.

I'm preparing a patch to fix this. Just wanted to expose my idea here,
because I'm perhaps wrong in the middle of this dump.

I'll do the alignment right in the end, just before inserting the entry
in the output. That will be more easy. I'll zeroe the rest in the same time.


> +		} else {
> +			struct {
> +				u32	size;
> +				u32	data;
> +			} raw = {
> +				.size = sizeof(u32),
> +				.data = 0,
> +			};
> +			perf_output_put(&handle, raw);
> +		}
> +	}
>  
>  	perf_output_end(&handle);
>  }
> 

Re: [PATCH 4/3] perf_counter: Correct PERF_SAMPLE_RAW output

[Peter Zijlstra]

On Mon, 2009-08-10 at 14:57 +0200, Frederic Weisbecker wrote:

> > Index: linux-2.6/include/trace/ftrace.h
> > ===================================================================
> > --- linux-2.6.orig/include/trace/ftrace.h
> > +++ linux-2.6/include/trace/ftrace.h
> > @@ -687,7 +687,8 @@ static void ftrace_profile_##call(proto)
> >  	pc = preempt_count();						\
> >  									\
> >  	__data_size = ftrace_get_offsets_##call(&__data_offsets, args); \
> > -	__entry_size = ALIGN(__data_size + sizeof(*entry), sizeof(u64));\
> > +	__entry_size = ALIGN(__data_size + sizeof(*entry) + sizeof(u32),\
> > +			     sizeof(u64));				\
> 
> 
> 
> Here you are reserving a room for the size of the buffer inside the buffer.

No, here I align so that __entry_size + the u32 ends up at a u64
boundary.

Oh gah, now I see.. I should have subtracted sizeof(u32) after the
alignment.

> > @@ -2717,9 +2716,15 @@ void perf_counter_output(struct perf_cou
> >  	}
> >  
> >  	if (sample_type & PERF_SAMPLE_RAW) {
> > -		raw = data->raw;
> > -		if (raw)
> > -			header.size += raw->size;
> > +		int size = sizeof(u32);
> > +
> > +		if (data->raw)
> > +			size += data->raw->size;
> 
> 
> 
> And here you reserve a size for the buffer once again?

This is the actual output buffer reservation code, that needs the u32
above and the data->raw size.

> 
> > +		else
> > +			size += sizeof(u32);
> > +
> > +		WARN_ON_ONCE(size & (sizeof(u64)-1));

Which when taken together should be u64 aligned.

So this will always fail with the above.

> > +		header.size += size;
> >  	}
> >  
> >  	ret = perf_output_begin(&handle, counter, header.size, nmi, 1);
> > @@ -2785,8 +2790,21 @@ void perf_counter_output(struct perf_cou
> >  		}
> >  	}
> >  
> > -	if ((sample_type & PERF_SAMPLE_RAW) && raw)
> > -		perf_output_copy(&handle, raw->data, raw->size);
> > +	if (sample_type & PERF_SAMPLE_RAW) {
> > +		if (data->raw) {
> > +			perf_output_put(&handle, data->raw->size);
> > +			perf_output_copy(&handle, data->raw->data, data->raw->size);
> 

> And actually you copy the buffer that has the size of the buffer
> plus the u32 reserved for the size, whereas the size has already been
> copied.
> 
> When I look at a perf sample raw it gives me the following:
> 
> ..  0000:  09 00 00 00 01 00 54 00 d0 c7 00 81 ff ff ff ff  ......T........
> ..  0010:  69 01 00 00 69 01 00 00 e6 15 00 00 00 00 00 00  i...i..........
> ..  0020:  30 00 00 00 2b 00 01 02 69 01 00 00 69 01 00 00  0...+...i...i..
> 
>           ^  ^  ^  ^  ^  ^  ^  ^  ^  ^ .................
>            Buf size   struct trace_entry
> 
> 
> 
> ..  0030:  6b 6f 6e 64 65 6d 61 6e 64 2f 31 00 00 00 00 00  kondemand/1....
> 
>           ^  ^  ^  ^  ..................................
>           Rest of struct ftrace_raw_<call>
> 
> ..  0040:  69 01 00 00 70 82 46 81 ff ff ff ff 00 00 00 00  i...p.F........
> 
>           ..................................  ^  ^  ^  ^
>                                               The room you have
>                                               reserved for the buffer size
>                                               (zeroed from a tmp patch)
> ..  0050:  00 00 00 00
>           ^  ^  ^  ^
>           padding from alignment (ALIGN(__data_size + sizeof(*entry) + sizeof(u32),\
> 			     sizeof(u64));

Right, one u32 too much :/

> I first thought the 0x4c bytes was a padding from gcc because
> sizeof(struct ftrace_raw_<call>) % 8 != 0
> But no, it is equal to 0: It's between 0x4c and 0x24 = 40 bytes.
> 
> I'm preparing a patch to fix this. Just wanted to expose my idea here,
> because I'm perhaps wrong in the middle of this dump.
> 
> I'll do the alignment right in the end, just before inserting the entry
> in the output. That will be more easy. I'll zeroe the rest in the same time.

OK.

[PATCH 6/3] perfcounter: Substract the buffer size field from the event record size

[Frederic Weisbecker]

On Mon, Aug 10, 2009 at 03:06:25PM +0200, Peter Zijlstra wrote:
> > I'll do the alignment right in the end, just before inserting the entry
> > in the output. That will be more easy. I'll zeroe the rest in the same time.
> 
> OK.


Actually I shouldn't, the alignment should be computed early, taking the
field size into account.

That's not a problem though, I've just substracted the u32 field once
I found the final aligned size.

See below:

---
From: Frederic Weisbecker <fweisbec@gmail.com>
Date: Mon, 10 Aug 2009 15:56:46 +0200
Subject: [PATCH 6/3] perfcounter: Substract the buffer size field from the event record size

We compute the perf raw sample size by aligning the raw ftrace event
size plus the buffer size field itself. We do that instead of aligning
only the perf raw sample size, so that we might economize some in some
cases.

But this buffer size field is not stored in the perf raw sample, we
must then substract its size from the buffer once we computed the
alignment unless we may get a useless u32 field in the buffer.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 include/trace/ftrace.h      |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletions(-)

diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index 4dc1a60..f98ff56 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -639,7 +639,12 @@ __attribute__((section("_ftrace_events"))) event_##call = {		\
  *	pc = preempt_count();
  *
  *	__data_size = ftrace_get_offsets_<call>(&__data_offsets, args);
- *	__entry_size = __data_size + sizeof(*entry);
+ *
+ *	// Below we want to get the aligned size by taking into account
+ *	// the u32 field that will later store the buffer size
+ *	__entry_size = ALIGN(__data_size + sizeof(*entry) + sizeof(u32),
+ *			     sizeof(u64));
+ *	__entry_size -= sizeof(u32);
  *
  *	do {
  *		char raw_data[__entry_size]; <- allocate our sample in the stack
@@ -689,6 +694,7 @@ static void ftrace_profile_##call(proto)				\
 	__data_size = ftrace_get_offsets_##call(&__data_offsets, args); \
 	__entry_size = ALIGN(__data_size + sizeof(*entry) + sizeof(u32),\
 			     sizeof(u64));				\
+	__entry_size -= sizeof(u32);					\
 									\
 	do {								\
 		char raw_data[__entry_size];				\
-- 
1.6.2.3

Re: [PATCH 6/3] perfcounter: Substract the buffer size field from the event record size

[Peter Zijlstra]

On Mon, 2009-08-10 at 16:11 +0200, Frederic Weisbecker wrote:

> From: Frederic Weisbecker <fweisbec@gmail.com>
> Date: Mon, 10 Aug 2009 15:56:46 +0200
> Subject: [PATCH 6/3] perfcounter: Substract the buffer size field from the event record size
> 
> We compute the perf raw sample size by aligning the raw ftrace event
> size plus the buffer size field itself. We do that instead of aligning
> only the perf raw sample size, so that we might economize some in some
> cases.
> 
> But this buffer size field is not stored in the perf raw sample, we
> must then substract its size from the buffer once we computed the
> alignment unless we may get a useless u32 field in the buffer.
> 
> Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>

Acked-by: Peter Zijlstra <a.p.zijlstra@chello.nl>

Thanks!

> ---
>  include/trace/ftrace.h      |    8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletions(-)
> 
> diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
> index 4dc1a60..f98ff56 100644
> --- a/include/trace/ftrace.h
> +++ b/include/trace/ftrace.h
> @@ -639,7 +639,12 @@ __attribute__((section("_ftrace_events"))) event_##call = {		\
>   *	pc = preempt_count();
>   *
>   *	__data_size = ftrace_get_offsets_<call>(&__data_offsets, args);
> - *	__entry_size = __data_size + sizeof(*entry);
> + *
> + *	// Below we want to get the aligned size by taking into account
> + *	// the u32 field that will later store the buffer size
> + *	__entry_size = ALIGN(__data_size + sizeof(*entry) + sizeof(u32),
> + *			     sizeof(u64));
> + *	__entry_size -= sizeof(u32);
>   *
>   *	do {
>   *		char raw_data[__entry_size]; <- allocate our sample in the stack
> @@ -689,6 +694,7 @@ static void ftrace_profile_##call(proto)				\
>  	__data_size = ftrace_get_offsets_##call(&__data_offsets, args); \
>  	__entry_size = ALIGN(__data_size + sizeof(*entry) + sizeof(u32),\
>  			     sizeof(u64));				\
> +	__entry_size -= sizeof(u32);					\
>  									\
>  	do {								\
>  		char raw_data[__entry_size];				\

[tip:perfcounters/urgent] perf_counter: Subtract the buffer size field from the event record size

[tip-bot for Frederic Weisbecker]

Commit-ID:  304703aba31a87903b8c0db8f5e6890cac2d596d
Gitweb:     http://git.kernel.org/tip/304703aba31a87903b8c0db8f5e6890cac2d596d
Author:     Frederic Weisbecker <fweisbec@gmail.com>
AuthorDate: Mon, 10 Aug 2009 16:11:32 +0200
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Mon, 10 Aug 2009 16:18:50 +0200

perf_counter: Subtract the buffer size field from the event record size

We compute the perf raw sample size by aligning the raw ftrace
event size plus the buffer size field itself. We do that
instead of aligning only the perf raw sample size, so that we
might economize some in some cases.

But this buffer size field is not stored in the perf raw
sample, we must then substract its size from the buffer once we
computed the alignment unless we may get a useless u32 field in
the buffer.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Acked-by: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
LKML-Reference: <20090810141129.GA5124@nowhere>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 include/trace/ftrace.h |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index 7167b9b..a05524f 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -637,7 +637,12 @@ __attribute__((section("_ftrace_events"))) event_##call = {		\
  *	pc = preempt_count();
  *
  *	__data_size = ftrace_get_offsets_<call>(&__data_offsets, args);
- *	__entry_size = __data_size + sizeof(*entry);
+ *
+ *	// Below we want to get the aligned size by taking into account
+ *	// the u32 field that will later store the buffer size
+ *	__entry_size = ALIGN(__data_size + sizeof(*entry) + sizeof(u32),
+ *			     sizeof(u64));
+ *	__entry_size -= sizeof(u32);
  *
  *	do {
  *		char raw_data[__entry_size]; <- allocate our sample in the stack
@@ -687,6 +692,7 @@ static void ftrace_profile_##call(proto)				\
 	__data_size = ftrace_get_offsets_##call(&__data_offsets, args); \
 	__entry_size = ALIGN(__data_size + sizeof(*entry) + sizeof(u32),\
 			     sizeof(u64));				\
+	__entry_size -= sizeof(u32);					\
 									\
 	do {								\
 		char raw_data[__entry_size];				\

[PATCH 5/3] perf_counter: Require CAP_SYS_ADMIN for raw tracepoint data

[Peter Zijlstra]

Subject: perf_counter: Require CAP_SYS_ADMIN for raw tracepoint data
From: Peter Zijlstra <a.p.zijlstra@chello.nl>
Date: Mon Aug 10 11:20:12 CEST 2009

Raw tracepoint data is a severe data leak, restrict this to root only.

Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
 kernel/perf_counter.c |    8 ++++++++
 1 file changed, 8 insertions(+)

Index: linux-2.6/kernel/perf_counter.c
===================================================================
--- linux-2.6.orig/kernel/perf_counter.c
+++ linux-2.6/kernel/perf_counter.c
@@ -3788,6 +3788,14 @@ static void tp_perf_counter_destroy(stru
 
 static const struct pmu *tp_perf_counter_init(struct perf_counter *counter)
 {
+	/*
+	 * Raw tracepoint data is a severe data leak, only allow root to
+	 * have these.
+	 */
+	if ((counter->attr.sample_type & PERF_SAMPLE_RAW) &&
+			!capable(CAP_SYS_ADMIN))
+		return ERR_PTR(-EPERM);
+
 	if (ftrace_profile_enable(counter->attr.config))
 		return NULL;
 

[tip:perfcounters/urgent] perf_counter: Require CAP_SYS_ADMIN for raw tracepoint data

[tip-bot for Peter Zijlstra]

Commit-ID:  a4e95fc2cbb31d70a65beffeaf8773f881328c34
Gitweb:     http://git.kernel.org/tip/a4e95fc2cbb31d70a65beffeaf8773f881328c34
Author:     Peter Zijlstra <a.p.zijlstra@chello.nl>
AuthorDate: Mon, 10 Aug 2009 11:20:12 +0200
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Mon, 10 Aug 2009 11:33:09 +0200

perf_counter: Require CAP_SYS_ADMIN for raw tracepoint data

Raw tracepoint data contains various kernel internals and
data from other users, so restrict this to CAP_SYS_ADMIN.

Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
LKML-Reference: <1249896452.17467.75.camel@twins>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 kernel/perf_counter.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/kernel/perf_counter.c b/kernel/perf_counter.c
index 5229d16..b0b20a0 100644
--- a/kernel/perf_counter.c
+++ b/kernel/perf_counter.c
@@ -3787,6 +3787,14 @@ static void tp_perf_counter_destroy(struct perf_counter *counter)
 
 static const struct pmu *tp_perf_counter_init(struct perf_counter *counter)
 {
+	/*
+	 * Raw tracepoint data is a severe data leak, only allow root to
+	 * have these.
+	 */
+	if ((counter->attr.sample_type & PERF_SAMPLE_RAW) &&
+			!capable(CAP_SYS_ADMIN))
+		return ERR_PTR(-EPERM);
+
 	if (ftrace_profile_enable(counter->attr.config))
 		return NULL;
 

[PATCH 2/4] perf tools: callchain: Ignore empty callchains

[Frederic Weisbecker]

When the callchain tree comes to insert an empty backtrace, it raises
a spurious warning about the fact we are inserting an empty.
This is spurious because the radix tree assumes it did something
wrong to reach this state. But it didn't, we just met an empty
callchain that has to be ignored.

Reported-by: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
---
 tools/perf/util/callchain.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index ce0c6d4..4c09b25 100644
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -337,5 +337,7 @@ __append_chain(struct callchain_node *root, struct ip_callchain *chain,
 void append_chain(struct callchain_node *root, struct ip_callchain *chain,
 		  struct symbol **syms)
 {
+	if (!chain->nr)
+		return;
 	__append_chain_children(root, chain, syms, 0);
 }
-- 
1.6.2.3

[PATCH 2/3] perfcounter: Generalize the tracepoint sampling to generic sampling

[Frederic Weisbecker]

Rename:

- PERF_SAMPLE_TP_RECORD to PERF_SAMPLE_RAW
- struct perf_tracepoint_record to perf_raw_record

We want the system in place that transport tracepoints raw samples
events into the perf ring buffer to be generalized and usable by any
type of counter.

Reported-by; Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
---
 include/linux/perf_counter.h |   10 +++++-----
 kernel/perf_counter.c        |   20 ++++++++++----------
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/include/linux/perf_counter.h b/include/linux/perf_counter.h
index c484834..0a6f320 100644
--- a/include/linux/perf_counter.h
+++ b/include/linux/perf_counter.h
@@ -121,7 +121,7 @@ enum perf_counter_sample_format {
 	PERF_SAMPLE_CPU				= 1U << 7,
 	PERF_SAMPLE_PERIOD			= 1U << 8,
 	PERF_SAMPLE_STREAM_ID			= 1U << 9,
-	PERF_SAMPLE_TP_RECORD			= 1U << 10,
+	PERF_SAMPLE_RAW				= 1U << 10,
 
 	PERF_SAMPLE_MAX = 1U << 11,		/* non-ABI */
 };
@@ -414,9 +414,9 @@ struct perf_callchain_entry {
 	__u64				ip[PERF_MAX_STACK_DEPTH];
 };
 
-struct perf_tracepoint_record {
-	int				size;
-	char				*record;
+struct perf_raw_record {
+	u32				size;
+	void				*data;
 };
 
 struct task_struct;
@@ -687,7 +687,7 @@ struct perf_sample_data {
 	struct pt_regs			*regs;
 	u64				addr;
 	u64				period;
-	void				*private;
+	struct perf_raw_record		*raw;
 };
 
 extern int perf_counter_overflow(struct perf_counter *counter, int nmi,
diff --git a/kernel/perf_counter.c b/kernel/perf_counter.c
index a6963e2..22e2839 100644
--- a/kernel/perf_counter.c
+++ b/kernel/perf_counter.c
@@ -2647,7 +2647,7 @@ void perf_counter_output(struct perf_counter *counter, int nmi,
 		u64 counter;
 	} group_entry;
 	struct perf_callchain_entry *callchain = NULL;
-	struct perf_tracepoint_record *tp = NULL;
+	struct perf_raw_record *raw = NULL;
 	int callchain_size = 0;
 	u64 time;
 	struct {
@@ -2716,10 +2716,10 @@ void perf_counter_output(struct perf_counter *counter, int nmi,
 			header.size += sizeof(u64);
 	}
 
-	if (sample_type & PERF_SAMPLE_TP_RECORD) {
-		tp = data->private;
-		if (tp)
-			header.size += tp->size;
+	if (sample_type & PERF_SAMPLE_RAW) {
+		raw = data->raw;
+		if (raw)
+			header.size += raw->size;
 	}
 
 	ret = perf_output_begin(&handle, counter, header.size, nmi, 1);
@@ -2785,8 +2785,8 @@ void perf_counter_output(struct perf_counter *counter, int nmi,
 		}
 	}
 
-	if ((sample_type & PERF_SAMPLE_TP_RECORD) && tp)
-		perf_output_copy(&handle, tp->record, tp->size);
+	if ((sample_type & PERF_SAMPLE_RAW) && raw)
+		perf_output_copy(&handle, raw->data, raw->size);
 
 	perf_output_end(&handle);
 }
@@ -3741,15 +3741,15 @@ static const struct pmu perf_ops_task_clock = {
 void perf_tpcounter_event(int event_id, u64 addr, u64 count, void *record,
 			  int entry_size)
 {
-	struct perf_tracepoint_record tp = {
+	struct perf_raw_record raw = {
 		.size = entry_size,
-		.record = record,
+		.data = record,
 	};
 
 	struct perf_sample_data data = {
 		.regs = get_irq_regs(),
 		.addr = addr,
-		.private = &tp,
+		.raw = &raw,
 	};
 
 	if (!data.regs)
-- 
1.6.2.3

[tip:perfcounters/core] perf_counter: Fix tracepoint sampling to be part of generic sampling

[tip-bot for Frederic Weisbecker]

Commit-ID:  6455883ad50c575b0bc5f6d6ab45d42cd56b83c4
Gitweb:     http://git.kernel.org/tip/6455883ad50c575b0bc5f6d6ab45d42cd56b83c4
Author:     Frederic Weisbecker <fweisbec@gmail.com>
AuthorDate: Sat, 8 Aug 2009 04:26:37 +0200
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Sat, 8 Aug 2009 13:49:22 +0200

perf_counter: Fix tracepoint sampling to be part of generic sampling

Based on Peter's comments, make tracepoint sampling generic
just like all the other sampling bits are. This is a rename
with no code changes:

- PERF_SAMPLE_TP_RECORD to PERF_SAMPLE_RAW
- struct perf_tracepoint_record to perf_raw_record

We want the system in place that transport tracepoints raw
samples events into the perf ring buffer to be generalized and
usable by any type of counter.

Reported-by; Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
LKML-Reference: <1249698400-5441-4-git-send-email-fweisbec@gmail.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 include/linux/perf_counter.h |   10 +++++-----
 kernel/perf_counter.c        |   20 ++++++++++----------
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/include/linux/perf_counter.h b/include/linux/perf_counter.h
index a67dd5c..2aabe43 100644
--- a/include/linux/perf_counter.h
+++ b/include/linux/perf_counter.h
@@ -121,7 +121,7 @@ enum perf_counter_sample_format {
 	PERF_SAMPLE_CPU				= 1U << 7,
 	PERF_SAMPLE_PERIOD			= 1U << 8,
 	PERF_SAMPLE_STREAM_ID			= 1U << 9,
-	PERF_SAMPLE_TP_RECORD			= 1U << 10,
+	PERF_SAMPLE_RAW				= 1U << 10,
 
 	PERF_SAMPLE_MAX = 1U << 11,		/* non-ABI */
 };
@@ -414,9 +414,9 @@ struct perf_callchain_entry {
 	__u64				ip[PERF_MAX_STACK_DEPTH];
 };
 
-struct perf_tracepoint_record {
-	int				size;
-	char				*record;
+struct perf_raw_record {
+	u32				size;
+	void				*data;
 };
 
 struct task_struct;
@@ -687,7 +687,7 @@ struct perf_sample_data {
 	struct pt_regs			*regs;
 	u64				addr;
 	u64				period;
-	void				*private;
+	struct perf_raw_record		*raw;
 };
 
 extern int perf_counter_overflow(struct perf_counter *counter, int nmi,
diff --git a/kernel/perf_counter.c b/kernel/perf_counter.c
index 117622c..0023105 100644
--- a/kernel/perf_counter.c
+++ b/kernel/perf_counter.c
@@ -2646,7 +2646,7 @@ static void perf_counter_output(struct perf_counter *counter, int nmi,
 		u64 counter;
 	} group_entry;
 	struct perf_callchain_entry *callchain = NULL;
-	struct perf_tracepoint_record *tp = NULL;
+	struct perf_raw_record *raw = NULL;
 	int callchain_size = 0;
 	u64 time;
 	struct {
@@ -2715,10 +2715,10 @@ static void perf_counter_output(struct perf_counter *counter, int nmi,
 			header.size += sizeof(u64);
 	}
 
-	if (sample_type & PERF_SAMPLE_TP_RECORD) {
-		tp = data->private;
-		if (tp)
-			header.size += tp->size;
+	if (sample_type & PERF_SAMPLE_RAW) {
+		raw = data->raw;
+		if (raw)
+			header.size += raw->size;
 	}
 
 	ret = perf_output_begin(&handle, counter, header.size, nmi, 1);
@@ -2784,8 +2784,8 @@ static void perf_counter_output(struct perf_counter *counter, int nmi,
 		}
 	}
 
-	if ((sample_type & PERF_SAMPLE_TP_RECORD) && tp)
-		perf_output_copy(&handle, tp->record, tp->size);
+	if ((sample_type & PERF_SAMPLE_RAW) && raw)
+		perf_output_copy(&handle, raw->data, raw->size);
 
 	perf_output_end(&handle);
 }
@@ -3740,15 +3740,15 @@ static const struct pmu perf_ops_task_clock = {
 void perf_tpcounter_event(int event_id, u64 addr, u64 count, void *record,
 			  int entry_size)
 {
-	struct perf_tracepoint_record tp = {
+	struct perf_raw_record raw = {
 		.size = entry_size,
-		.record = record,
+		.data = record,
 	};
 
 	struct perf_sample_data data = {
 		.regs = get_irq_regs(),
 		.addr = addr,
-		.private = &tp,
+		.raw = &raw,
 	};
 
 	if (!data.regs)

[tip:perfcounters/core] perf_counter: Fix tracepoint sampling to be part of generic sampling

[tip-bot for Frederic Weisbecker]

Commit-ID:  3a43ce68ae1758fa6a839386025ef45acb6baa22
Gitweb:     http://git.kernel.org/tip/3a43ce68ae1758fa6a839386025ef45acb6baa22
Author:     Frederic Weisbecker <fweisbec@gmail.com>
AuthorDate: Sat, 8 Aug 2009 04:26:37 +0200
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Sun, 9 Aug 2009 12:54:45 +0200

perf_counter: Fix tracepoint sampling to be part of generic sampling

Based on Peter's comments, make tracepoint sampling generic
just like all the other sampling bits are. This is a rename
with no code changes:

- PERF_SAMPLE_TP_RECORD to PERF_SAMPLE_RAW
- struct perf_tracepoint_record to perf_raw_record

We want the system in place that transport tracepoints raw
samples events into the perf ring buffer to be generalized and
usable by any type of counter.

Reported-by; Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
LKML-Reference: <1249698400-5441-4-git-send-email-fweisbec@gmail.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 include/linux/perf_counter.h |   10 +++++-----
 kernel/perf_counter.c        |   20 ++++++++++----------
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/include/linux/perf_counter.h b/include/linux/perf_counter.h
index a67dd5c..2aabe43 100644
--- a/include/linux/perf_counter.h
+++ b/include/linux/perf_counter.h
@@ -121,7 +121,7 @@ enum perf_counter_sample_format {
 	PERF_SAMPLE_CPU				= 1U << 7,
 	PERF_SAMPLE_PERIOD			= 1U << 8,
 	PERF_SAMPLE_STREAM_ID			= 1U << 9,
-	PERF_SAMPLE_TP_RECORD			= 1U << 10,
+	PERF_SAMPLE_RAW				= 1U << 10,
 
 	PERF_SAMPLE_MAX = 1U << 11,		/* non-ABI */
 };
@@ -414,9 +414,9 @@ struct perf_callchain_entry {
 	__u64				ip[PERF_MAX_STACK_DEPTH];
 };
 
-struct perf_tracepoint_record {
-	int				size;
-	char				*record;
+struct perf_raw_record {
+	u32				size;
+	void				*data;
 };
 
 struct task_struct;
@@ -687,7 +687,7 @@ struct perf_sample_data {
 	struct pt_regs			*regs;
 	u64				addr;
 	u64				period;
-	void				*private;
+	struct perf_raw_record		*raw;
 };
 
 extern int perf_counter_overflow(struct perf_counter *counter, int nmi,
diff --git a/kernel/perf_counter.c b/kernel/perf_counter.c
index 117622c..0023105 100644
--- a/kernel/perf_counter.c
+++ b/kernel/perf_counter.c
@@ -2646,7 +2646,7 @@ static void perf_counter_output(struct perf_counter *counter, int nmi,
 		u64 counter;
 	} group_entry;
 	struct perf_callchain_entry *callchain = NULL;
-	struct perf_tracepoint_record *tp = NULL;
+	struct perf_raw_record *raw = NULL;
 	int callchain_size = 0;
 	u64 time;
 	struct {
@@ -2715,10 +2715,10 @@ static void perf_counter_output(struct perf_counter *counter, int nmi,
 			header.size += sizeof(u64);
 	}
 
-	if (sample_type & PERF_SAMPLE_TP_RECORD) {
-		tp = data->private;
-		if (tp)
-			header.size += tp->size;
+	if (sample_type & PERF_SAMPLE_RAW) {
+		raw = data->raw;
+		if (raw)
+			header.size += raw->size;
 	}
 
 	ret = perf_output_begin(&handle, counter, header.size, nmi, 1);
@@ -2784,8 +2784,8 @@ static void perf_counter_output(struct perf_counter *counter, int nmi,
 		}
 	}
 
-	if ((sample_type & PERF_SAMPLE_TP_RECORD) && tp)
-		perf_output_copy(&handle, tp->record, tp->size);
+	if ((sample_type & PERF_SAMPLE_RAW) && raw)
+		perf_output_copy(&handle, raw->data, raw->size);
 
 	perf_output_end(&handle);
 }
@@ -3740,15 +3740,15 @@ static const struct pmu perf_ops_task_clock = {
 void perf_tpcounter_event(int event_id, u64 addr, u64 count, void *record,
 			  int entry_size)
 {
-	struct perf_tracepoint_record tp = {
+	struct perf_raw_record raw = {
 		.size = entry_size,
-		.record = record,
+		.data = record,
 	};
 
 	struct perf_sample_data data = {
 		.regs = get_irq_regs(),
 		.addr = addr,
-		.private = &tp,
+		.raw = &raw,
 	};
 
 	if (!data.regs)

[PATCH 3/4] perf tools: callchain: Default display callchain from report if recorded with -g

[Frederic Weisbecker]

If we recorded with -g option to record the callchain, let's display
the callchain by default from perf report.

The user can override this default using "-g none" option from
perf report.

Reported-by: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
---
 tools/perf/builtin-report.c |   16 +++++++++++++++-
 tools/perf/util/callchain.h |    1 +
 2 files changed, 16 insertions(+), 1 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index a5e2f8d..c4a8e10 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -68,7 +68,7 @@ static int		callchain;
 
 static
 struct callchain_param	callchain_param = {
-	.mode	= CHAIN_GRAPH_ABS,
+	.mode	= CHAIN_GRAPH_REL,
 	.min_percent = 0.5
 };
 
@@ -1836,6 +1836,13 @@ static int __cmd_report(void)
 					" -g?\n");
 			exit(-1);
 		}
+	} else if (callchain_param.mode != CHAIN_NONE && !callchain) {
+			callchain = 1;
+			if (register_callchain_param(&callchain_param) < 0) {
+				fprintf(stderr, "Can't register callchain"
+						" params\n");
+				exit(-1);
+			}
 	}
 
 	if (load_kernel() < 0) {
@@ -1974,6 +1981,13 @@ parse_callchain_opt(const struct option *opt __used, const char *arg,
 	else if (!strncmp(tok, "fractal", strlen(arg)))
 		callchain_param.mode = CHAIN_GRAPH_REL;
 
+	else if (!strncmp(tok, "none", strlen(arg))) {
+		callchain_param.mode = CHAIN_NONE;
+		callchain = 0;
+
+		return 0;
+	}
+
 	else
 		return -1;
 
diff --git a/tools/perf/util/callchain.h b/tools/perf/util/callchain.h
index b2d128e..a926ae4 100644
--- a/tools/perf/util/callchain.h
+++ b/tools/perf/util/callchain.h
@@ -7,6 +7,7 @@
 #include "symbol.h"
 
 enum chain_mode {
+	CHAIN_NONE,
 	CHAIN_FLAT,
 	CHAIN_GRAPH_ABS,
 	CHAIN_GRAPH_REL
-- 
1.6.2.3

[PATCH 3/3] perfcounter: Align ftrace events raw samples to 8 bytes

[Frederic Weisbecker]

Align the ftrace events raw samples to 8 bytes so that they meet the
perf output event alignements requirements.

Reported-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
---
 include/trace/ftrace.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index 80e5f6c..7b0834c 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -687,7 +687,7 @@ static void ftrace_profile_##call(proto)				\
 	pc = preempt_count();						\
 									\
 	__data_size = ftrace_get_offsets_##call(&__data_offsets, args); \
-	__entry_size = __data_size + sizeof(*entry);			\
+	__entry_size = ALIGN(__data_size + sizeof(*entry), sizeof(u64));\
 									\
 	do {								\
 		char raw_data[__entry_size];				\
-- 
1.6.2.3

[tip:perfcounters/core] perf_counter: Fix ftrace events raw samples to be aligned to 8 bytes

[tip-bot for Frederic Weisbecker]

Commit-ID:  eba67b1fd06180d57b94bf97557d89873b0ce61e
Gitweb:     http://git.kernel.org/tip/eba67b1fd06180d57b94bf97557d89873b0ce61e
Author:     Frederic Weisbecker <fweisbec@gmail.com>
AuthorDate: Sat, 8 Aug 2009 04:26:39 +0200
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Sat, 8 Aug 2009 13:49:23 +0200

perf_counter: Fix ftrace events raw samples to be aligned to 8 bytes

Align the ftrace events raw samples to 8 bytes so that they
meet the perf output event alignment requirements.

Reported-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
LKML-Reference: <1249698400-5441-6-git-send-email-fweisbec@gmail.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 include/trace/ftrace.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index bbf9042..7fb16d9 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -685,7 +685,7 @@ static void ftrace_profile_##call(proto)				\
 	pc = preempt_count();						\
 									\
 	__data_size = ftrace_get_offsets_##call(&__data_offsets, args); \
-	__entry_size = __data_size + sizeof(*entry);			\
+	__entry_size = ALIGN(__data_size + sizeof(*entry), sizeof(u64));\
 									\
 	do {								\
 		char raw_data[__entry_size];				\

Re: [PATCH 3/3] perfcounter: Align ftrace events raw samples to 8 bytes

[Peter Zijlstra]

On Sat, 2009-08-08 at 04:26 +0200, Frederic Weisbecker wrote:
> Align the ftrace events raw samples to 8 bytes so that they meet the
> perf output event alignements requirements.
> 
> Reported-by: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Mike Galbraith <efault@gmx.de>
> Cc: Paul Mackerras <paulus@samba.org>
> ---
>  include/trace/ftrace.h |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
> index 80e5f6c..7b0834c 100644
> --- a/include/trace/ftrace.h
> +++ b/include/trace/ftrace.h
> @@ -687,7 +687,7 @@ static void ftrace_profile_##call(proto)				\
>  	pc = preempt_count();						\
>  									\
>  	__data_size = ftrace_get_offsets_##call(&__data_offsets, args); \
> -	__entry_size = __data_size + sizeof(*entry);			\
> +	__entry_size = ALIGN(__data_size + sizeof(*entry), sizeof(u64));\
>  									\
>  	do {								\
>  		char raw_data[__entry_size];				\

Make sure to clear the trailing few bytes, otherwise we've got a stack
data leak here.

Also, tracepoints using __string() could still easily overflow the stack
here and cause great havoc, esp since the tracepoint could already be
deep into a call-chain.

Re: [PATCH 3/3] perfcounter: Align ftrace events raw samples to 8 bytes

[Frederic Weisbecker]

On Mon, Aug 10, 2009 at 09:13:57AM +0200, Peter Zijlstra wrote:
> On Sat, 2009-08-08 at 04:26 +0200, Frederic Weisbecker wrote:
> > Align the ftrace events raw samples to 8 bytes so that they meet the
> > perf output event alignements requirements.
> > 
> > Reported-by: Peter Zijlstra <peterz@infradead.org>
> > Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> > Cc: Mike Galbraith <efault@gmx.de>
> > Cc: Paul Mackerras <paulus@samba.org>
> > ---
> >  include/trace/ftrace.h |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
> > index 80e5f6c..7b0834c 100644
> > --- a/include/trace/ftrace.h
> > +++ b/include/trace/ftrace.h
> > @@ -687,7 +687,7 @@ static void ftrace_profile_##call(proto)				\
> >  	pc = preempt_count();						\
> >  									\
> >  	__data_size = ftrace_get_offsets_##call(&__data_offsets, args); \
> > -	__entry_size = __data_size + sizeof(*entry);			\
> > +	__entry_size = ALIGN(__data_size + sizeof(*entry), sizeof(u64));\
> >  									\
> >  	do {								\
> >  		char raw_data[__entry_size];				\
> 
> Make sure to clear the trailing few bytes, otherwise we've got a stack
> data leak here.


Oh, right.


 
> Also, tracepoints using __string() could still easily overflow the stack
> here and cause great havoc, esp since the tracepoint could already be
> deep into a call-chain.


I know yeah, I'll think about a way to store the string parts as pointers
and copy their content on perf_output only.

[PATCH 7/3] perfcounter: Zeroe dead bytes from ftrace raw samples size alignment

[Frederic Weisbecker]

After aligning the ftrace raw samples, there are dead bytes storing
random data from the stack. We don't want to leak these to userspace,
then zeroe these out.

Before:

	0x2de88 [0x50]: event: 9
	.
	. ... raw event: size 80 bytes
	.  0000:  09 00 00 00 01 00 50 00 d0 c7 00 81 ff ff ff ff  ......P........
	.  0010:  68 01 00 00 68 01 00 00 2c 00 00 00 00 00 00 00  h...h...,......
	.  0020:  2c 00 00 00 2b 00 01 02 68 01 00 00 68 01 00 00  ,...+...h...h..
	.  0030:  6b 6f 6e 64 65 6d 61 6e 64 2f 30 00 00 00 00 00  kondemand/0....
	.  0040:  68 01 00 00 40 7f 46 81 ff ff ff ff 00 10 1b 7f  h...@.F........
                                                      ^  ^  ^  ^
                                                         Leak

After:

	0x2d318 [0x50]: event: 9
	.
	. ... raw event: size 80 bytes
	.  0000:  09 00 00 00 01 00 50 00 d0 c7 00 81 ff ff ff ff  ......P........
	.  0010:  68 01 00 00 68 01 00 00 68 14 00 00 00 00 00 00  h...h...h......
	.  0020:  2c 00 00 00 2b 00 01 02 68 01 00 00 68 01 00 00  ,...+...h...h..
	.  0030:  6b 6f 6e 64 65 6d 61 6e 64 2f 30 00 00 00 00 00  kondemand/0....
	.  0040:  68 01 00 00 a0 80 46 81 ff ff ff ff 00 00 00 00  h.....F........
                                                      ^  ^  ^  ^
							 Fixed

Reported-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
---
 include/trace/ftrace.h |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index f98ff56..f2646e0 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -650,6 +650,8 @@ __attribute__((section("_ftrace_events"))) event_##call = {		\
  *		char raw_data[__entry_size]; <- allocate our sample in the stack
  *		struct trace_entry *ent;
  *
+ *		// zeroe dead bytes from alignment to avoid stack leak to userspace
+ *		*(u64 *)(&raw_data[__entry_size - sizeof(u64)]) = 0ULL;
  *		entry = (struct ftrace_raw_<call> *)raw_data;
  *		ent = &entry->ent;
  *		tracing_generic_entry_update(ent, irq_flags, pc);
@@ -700,6 +702,7 @@ static void ftrace_profile_##call(proto)				\
 		char raw_data[__entry_size];				\
 		struct trace_entry *ent;				\
 									\
+		*(u64 *)(&raw_data[__entry_size - sizeof(u64)]) = 0ULL;	\
 		entry = (struct ftrace_raw_##call *)raw_data;		\
 		ent = &entry->ent;					\
 		tracing_generic_entry_update(ent, irq_flags, pc);	\
-- 
1.6.2.3

[tip:perfcounters/urgent] perf_counter: Zero dead bytes from ftrace raw samples size alignment

[tip-bot for Frederic Weisbecker]

Commit-ID:  1853db0e02ae4088f102b0d8e59e83dc98f93f03
Gitweb:     http://git.kernel.org/tip/1853db0e02ae4088f102b0d8e59e83dc98f93f03
Author:     Frederic Weisbecker <fweisbec@gmail.com>
AuthorDate: Mon, 10 Aug 2009 16:38:36 +0200
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Mon, 10 Aug 2009 16:51:19 +0200

perf_counter: Zero dead bytes from ftrace raw samples size alignment

After aligning the ftrace raw samples, there are dead bytes storing
random data from the stack. We don't want to leak these to userspace,
then zero these out.

Before:

	0x2de88 [0x50]: event: 9
	.
	. ... raw event: size 80 bytes
	.  0000:  09 00 00 00 01 00 50 00 d0 c7 00 81 ff ff ff ff  ......P........
	.  0010:  68 01 00 00 68 01 00 00 2c 00 00 00 00 00 00 00  h...h...,......
	.  0020:  2c 00 00 00 2b 00 01 02 68 01 00 00 68 01 00 00  ,...+...h...h..
	.  0030:  6b 6f 6e 64 65 6d 61 6e 64 2f 30 00 00 00 00 00  kondemand/0....
	.  0040:  68 01 00 00 40 7f 46 81 ff ff ff ff 00 10 1b 7f  h...@.F........
                                                      ^  ^  ^  ^
                                                         Leak

After:

	0x2d318 [0x50]: event: 9
	.
	. ... raw event: size 80 bytes
	.  0000:  09 00 00 00 01 00 50 00 d0 c7 00 81 ff ff ff ff  ......P........
	.  0010:  68 01 00 00 68 01 00 00 68 14 00 00 00 00 00 00  h...h...h......
	.  0020:  2c 00 00 00 2b 00 01 02 68 01 00 00 68 01 00 00  ,...+...h...h..
	.  0030:  6b 6f 6e 64 65 6d 61 6e 64 2f 30 00 00 00 00 00  kondemand/0....
	.  0040:  68 01 00 00 a0 80 46 81 ff ff ff ff 00 00 00 00  h.....F........
                                                      ^  ^  ^  ^
							 Fixed

Reported-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
LKML-Reference: <1249915116-5210-1-git-send-email-fweisbec@gmail.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>


---
 include/trace/ftrace.h |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index a05524f..f64fbaa 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -648,6 +648,9 @@ __attribute__((section("_ftrace_events"))) event_##call = {		\
  *		char raw_data[__entry_size]; <- allocate our sample in the stack
  *		struct trace_entry *ent;
  *
+ *		zero dead bytes from alignment to avoid stack leak to userspace:
+ *
+ *		*(u64 *)(&raw_data[__entry_size - sizeof(u64)]) = 0ULL;
  *		entry = (struct ftrace_raw_<call> *)raw_data;
  *		ent = &entry->ent;
  *		tracing_generic_entry_update(ent, irq_flags, pc);
@@ -698,6 +701,7 @@ static void ftrace_profile_##call(proto)				\
 		char raw_data[__entry_size];				\
 		struct trace_entry *ent;				\
 									\
+		*(u64 *)(&raw_data[__entry_size - sizeof(u64)]) = 0ULL;	\
 		entry = (struct ftrace_raw_##call *)raw_data;		\
 		ent = &entry->ent;					\
 		tracing_generic_entry_update(ent, irq_flags, pc);	\

[PATCH 4/4] perf tools: callchain: Display amount of ignored chains in fractal mode

[Frederic Weisbecker]

When we filter the callchains below a given percentage, we ignore them
and the end result only shows entries that have an upper percentage
than the filter threshold. It seems then that we have an imbalance in
the percentage, as if the sum inside a profiled branch doesn't reach
100%.

Fix this by displaying the remaining hits that have been filtered but
without more detail than their amount in each branches.
Example while filtering below 50%:

7.73%  [k] delay_tsc
                |
                |--98.22%-- __const_udelay
                |          |
                |          |--86.37%-- ath5k_hw_register_timeout
                |          |          ath5k_hw_noise_floor_calibration
                |          |          ath5k_hw_reset
                |          |          ath5k_reset
                |          |          ath5k_config
                |          |          ieee80211_hw_config
                |          |          |
                |          |          |--88.53%-- ieee80211_scan_work
                |          |          |          worker_thread
                |          |          |          kthread
                |          |          |          child_rip
                |          |           --11.47%-- [...]
                |           --13.63%-- [...]
                 --1.78%-- [...]

Reported-by: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
---
 tools/perf/builtin-report.c |   47 ++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 44 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index c4a8e10..99274ce 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -891,6 +891,21 @@ ipchain__fprintf_graph(FILE *fp, struct callchain_list *chain, int depth,
 	return ret;
 }
 
+static struct symbol *rem_sq_bracket;
+static struct callchain_list rem_hits;
+
+static void init_rem_hits(void)
+{
+	rem_sq_bracket = malloc(sizeof(*rem_sq_bracket) + 6);
+	if (!rem_sq_bracket) {
+		fprintf(stderr, "Not enough memory to display remaining hits\n");
+		return;
+	}
+
+	strcpy(rem_sq_bracket->name, "[...]");
+	rem_hits.sym = rem_sq_bracket;
+}
+
 static size_t
 callchain__fprintf_graph(FILE *fp, struct callchain_node *self,
 			u64 total_samples, int depth, int depth_mask)
@@ -900,6 +915,7 @@ callchain__fprintf_graph(FILE *fp, struct callchain_node *self,
 	struct callchain_list *chain;
 	int new_depth_mask = depth_mask;
 	u64 new_total;
+	u64 remaining;
 	size_t ret = 0;
 	int i;
 
@@ -908,17 +924,25 @@ callchain__fprintf_graph(FILE *fp, struct callchain_node *self,
 	else
 		new_total = total_samples;
 
+	remaining = new_total;
+
 	node = rb_first(&self->rb_root);
 	while (node) {
+		u64 cumul;
+
 		child = rb_entry(node, struct callchain_node, rb_node);
+		cumul = cumul_hits(child);
+		remaining -= cumul;
 
 		/*
 		 * The depth mask manages the output of pipes that show
 		 * the depth. We don't want to keep the pipes of the current
-		 * level for the last child of this depth
+		 * level for the last child of this depth.
+		 * Except if we have remaining filtered hits. They will
+		 * supersede the last child
 		 */
 		next = rb_next(node);
-		if (!next)
+		if (!next && (callchain_param.mode != CHAIN_GRAPH_REL || !remaining))
 			new_depth_mask &= ~(1 << (depth - 1));
 
 		/*
@@ -933,7 +957,7 @@ callchain__fprintf_graph(FILE *fp, struct callchain_node *self,
 			ret += ipchain__fprintf_graph(fp, chain, depth,
 						      new_depth_mask, i++,
 						      new_total,
-						      cumul_hits(child));
+						      cumul);
 		}
 		ret += callchain__fprintf_graph(fp, child, new_total,
 						depth + 1,
@@ -941,6 +965,19 @@ callchain__fprintf_graph(FILE *fp, struct callchain_node *self,
 		node = next;
 	}
 
+	if (callchain_param.mode == CHAIN_GRAPH_REL &&
+		remaining && remaining != new_total) {
+
+		if (!rem_sq_bracket)
+			return ret;
+
+		new_depth_mask &= ~(1 << (depth - 1));
+
+		ret += ipchain__fprintf_graph(fp, &rem_hits, depth,
+					      new_depth_mask, 0, new_total,
+					      remaining);
+	}
+
 	return ret;
 }
 
@@ -1361,6 +1398,8 @@ static size_t output__fprintf(FILE *fp, u64 total_samples)
 	unsigned int width;
 	char *col_width = col_width_list_str;
 
+	init_rem_hits();
+
 	fprintf(fp, "# Samples: %Ld\n", (u64)total_samples);
 	fprintf(fp, "#\n");
 
@@ -1432,6 +1471,8 @@ print_entries:
 	}
 	fprintf(fp, "\n");
 
+	free(rem_sq_bracket);
+
 	return ret;
 }
 
-- 
1.6.2.3