perf tools: callchain: Warn only once in empty node detection
Subsystems (3)
PATCH 1/4 perf tools: callchain: Warn only once in empty node detection
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).
--- 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(-)
tools/perf/util/callchain.c +2 -1
@@ -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
tools/perf/util/util.h +3 -0
@@ -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
tools/perf/util/wrapper.c +16 -0
@@ -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; +}
Signature · 1 line
1.6.2.3
PATCH 1/3 perfcounter: Initialize tracepoint record before any use
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.
--- kernel/perf_counter.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-)
kernel/perf_counter.c +4 -3
@@ -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);
Signature · 1 line
1.6.2.3
Re: [PATCH 1/3] perfcounter: Initialize tracepoint record before any use
Quoted 10 lines · Frederic Weisbecker
Despite that the tracepoint record is always present when the
PERF_SAMPLE_TP_RECORDflag 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 functionThen, initialize it to NULL and always check if it's not NULL before dereference it.
Quoted 7 lines
Reported-by Ingo Molnar <[email protected]>Signed-off-by Frederic Weisbecker <[email protected]>Cc Peter Zijlstra <[email protected]>Cc Arnaldo Carvalho de Melo <[email protected]>Cc Mike Galbraith <[email protected]>Cc Paul Mackerras <[email protected]>---
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.
Quoted 38 lines
kernel/perf_counter.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-)kernel/perf_counter.c +4
-3@@ -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);Signature · 1 line
1.6.2.3
Peter Zijlstra
maintainer[PATCH 4/3] perf_counter: Correct PERF_SAMPLE_RAW output
In reply to Frederic WeisbeckerSubject: perf_counter: Correct PERF_SAMPLE_RAW output
From: Peter Zijlstra <[email protected]>
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.
--- include/linux/perf_counter.h | 2 ++ include/trace/ftrace.h | 3 ++- kernel/perf_counter.c | 30 ++++++++++++++++++++++++------ 3 files changed, 28 insertions(+), 7 deletions(-)
include/linux/perf_counter.h +2 -0
@@ -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,
include/trace/ftrace.h +2 -1
@@ -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]; \
kernel/perf_counter.c +24 -6
@@ -2647,7 +2647,6 @@ void perf_counter_output(struct perf_couu64 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
Commit-ID: a044560c3a1f0ad75ce685c1ed7604820b9ed319 Gitweb: http://git.kernel.org/tip/a044560c3a1f0ad75ce685c1ed7604820b9ed319 Author: Peter Zijlstra <[email protected]> AuthorDate: Mon, 10 Aug 2009 11:16:52 +0200 Committer: Ingo Molnar <[email protected]> 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.
LKML-Reference: <1249896447.17467.74.camel@twins>
--- include/linux/perf_counter.h | 2 ++ include/trace/ftrace.h | 3 ++- kernel/perf_counter.c | 30 ++++++++++++++++++++++++------ 3 files changed, 28 insertions(+), 7 deletions(-)
include/linux/perf_counter.h +2 -0
@@ -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,
include/trace/ftrace.h +2 -1
@@ -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]; \
kernel/perf_counter.c +24 -6
@@ -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); }
Frederic Weisbecker
Acked-byRe: [PATCH 4/3] perf_counter: Correct PERF_SAMPLE_RAW output
In reply to Peter ZijlstraQuoted 9 lines · Peter Zijlstra
Subject:
perf_counter: CorrectPERF_SAMPLE_RAWoutput
From: Peter Zijlstra <[email protected]>
Date: Mon Aug 10 11:16:52 CEST 2009PERF_SAMPLE_* output switches should unconditionally output the correct format, as they are the only way to unambiguously parse the
PERF_EVENT_SAMPLEdata.Signed-off-by Peter Zijlstra <[email protected]>
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.
Quoted 89 lines
--- 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
===================================================================include/linux/perf_counter.h +2
-0@@ -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
===================================================================include/trace/ftrace.h +2
-1@@ -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
===================================================================kernel/perf_counter.c +24
-6@@ -2647,7 +2647,6 @@ void perf_counter_output(struct perf_couu64 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
In reply to Peter ZijlstraQuoted 33 lines · Peter Zijlstra
Subject:
perf_counter: CorrectPERF_SAMPLE_RAWoutput
From: Peter Zijlstra <[email protected]>
Date: Mon Aug 10 11:16:52 CEST 2009PERF_SAMPLE_* output switches should unconditionally output the correct format, as they are the only way to unambiguously parse the
PERF_EVENT_SAMPLEdata.Signed-off-by Peter Zijlstra <[email protected]>--- 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
===================================================================include/linux/perf_counter.h +2
-0@@ -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
===================================================================
include/trace/ftrace.h static void ftrace_profile_##call(proto) · line 691
Quoted 6 lines
include/trace/ftrace.h +2
-1@@ -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.
Quoted 16 lines
include/trace/ftrace.h +0
-0\ do { \ char raw_data[__entry_size]; \Index:
linux-2.6/kernel/perf_counter.c
===================================================================kernel/perf_counter.c +0
-1@@ -2647,7 +2647,6 @@ void perf_counter_output(struct perf_couu64 counter; } group_entry;struct perf_callchain_entry *callchain =NULL ; -struct perf_raw_record *raw =NULL ;int callchain_size =0 ;u64 time;struct {
kernel/perf_counter.c void perf_counter_output(struct perf_cou · line 2722
Quoted 10 lines
kernel/perf_counter.c +4
-3@@ -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?
Quoted 9 lines
kernel/perf_counter.c +5
-0+else + size +=sizeof (u32); + +WARN_ON_ONCE (size & (sizeof (u64)-1 )); + header.size += size; } ret =perf_output_begin (&handle, counter, header.size, nmi,1 );
kernel/perf_counter.c void perf_counter_output(struct perf_cou · line 2796
Quoted 9 lines
kernel/perf_counter.c +4
-2@@ -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.
Quoted 15 lines
kernel/perf_counter.c +11
-0+ }else { +struct { +u32 size; +u32 data; + } raw = { + .size =sizeof (u32), + .data =0 , + }; +perf_output_put (&handle, raw); + } + }perf_output_end (&handle); }
Peter Zijlstra
maintainerRe: [PATCH 4/3] perf_counter: Correct PERF_SAMPLE_RAW output
Quoted 7 lines · Frederic Weisbecker
Quoted 7 lines
Index:
linux-2.6/include/trace/ftrace.h
===================================================================include/trace/ftrace.h +0
-0@@ -687,7 +687,8 @@ static void ftrace_profile_##call(proto) pc =preempt_count (); \ \
Quoted 8 lines
include/trace/ftrace.h static void ftrace_profile_##call(proto) · line 691
Quoted 4 lines
include/trace/ftrace.h +2
-1__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.
Quoted 15 lines
include/trace/ftrace.h void perf_counter_output(struct perf_cou · line 2722
Quoted 11 lines
include/trace/ftrace.h +4
-3@@ -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.
Quoted 5 lines
include/trace/ftrace.h void perf_counter_output(struct perf_cou · line 2726
Quoted 5 lines
include/trace/ftrace.h +4
-0+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.
Quoted 38 lines
Quoted 5 lines
include/trace/ftrace.h +1
-0+ header.size += size; } ret =perf_output_begin (&handle, counter, header.size, nmi,1 );include/trace/ftrace.h void perf_counter_output(struct perf_cou · line 2796
Quoted 9 lines
include/trace/ftrace.h +4
-2@@ -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 structtrace_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........
Quoted 8 lines
.................................. ^ ^ ^ ^
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 :/
Quoted 9 lines
I first thought the
0x4cbytes was a padding from gcc becausesizeof(struct ftrace_raw_<call>)% 8 != 0
But no, it is equal to 0: It's between0x4cand0x24= 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
Quoted 4 lines · Peter Zijlstra
Quoted 2 lines
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 <[email protected]>
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.
--- include/trace/ftrace.h | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletions(-)
include/trace/ftrace.h +7 -1
@@ -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]; \
Signature · 1 line
1.6.2.3
Re: [PATCH 6/3] perfcounter: Substract the buffer size field from the event record size
Quoted 9 lines · Frederic Weisbecker
From: Frederic Weisbecker <[email protected]>
Date: Mon, 10 Aug 2009 15:56:46 +0200
Subject: [PATCH 6/3] perfcounter: Substract the buffer size field from the event record sizeWe 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.
Quoted 5 lines
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 <[email protected]>
Thanks!
Quoted 30 lines
--- include/trace/ftrace.h | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletions(-)include/trace/ftrace.h +7
-1@@ -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
In reply to Frederic WeisbeckerCommit-ID: 304703aba31a87903b8c0db8f5e6890cac2d596d Gitweb: http://git.kernel.org/tip/304703aba31a87903b8c0db8f5e6890cac2d596d Author: Frederic Weisbecker <[email protected]> AuthorDate: Mon, 10 Aug 2009 16:11:32 +0200 Committer: Ingo Molnar <[email protected]> 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.
LKML-Reference: <20090810141129.GA5124@nowhere>
--- include/trace/ftrace.h | 8 +++++++- 1 files changed, 7 insertions(+), 1 deletions(-)
include/trace/ftrace.h +7 -1
@@ -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]; \
Peter Zijlstra
maintainer[PATCH 5/3] perf_counter: Require CAP_SYS_ADMIN for raw tracepoint data
In reply to Frederic WeisbeckerSubject: perf_counter: Require CAP_SYS_ADMIN for raw tracepoint data
From: Peter Zijlstra <[email protected]>
Date: Mon Aug 10 11:20:12 CEST 2009
Raw tracepoint data is a severe data leak, restrict this to root only.
--- kernel/perf_counter.c | 8 ++++++++ 1 file changed, 8 insertions(+)
kernel/perf_counter.c +8 -0
@@ -3788,6 +3788,14 @@ static void tp_perf_counter_destroy(strustatic 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
Commit-ID: a4e95fc2cbb31d70a65beffeaf8773f881328c34 Gitweb: http://git.kernel.org/tip/a4e95fc2cbb31d70a65beffeaf8773f881328c34 Author: Peter Zijlstra <[email protected]> AuthorDate: Mon, 10 Aug 2009 11:20:12 +0200 Committer: Ingo Molnar <[email protected]> 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.
LKML-Reference: <1249896452.17467.75.camel@twins>
--- kernel/perf_counter.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-)
kernel/perf_counter.c +8 -0
@@ -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
In reply to Frederic WeisbeckerWhen 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.
--- tools/perf/util/callchain.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
tools/perf/util/callchain.c +2 -0
@@ -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 ); }
Signature · 1 line
1.6.2.3
PATCH 3/4 perf tools: callchain: Default display callchain from report if recorded with -g
In reply to Frederic WeisbeckerIf 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.
--- tools/perf/builtin-report.c | 16 +++++++++++++++- tools/perf/util/callchain.h | 1 + 2 files changed, 16 insertions(+), 1 deletions(-)
tools/perf/builtin-report.c +15 -1
@@ -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 ;
tools/perf/util/callchain.h +1 -0
@@ -7,6 +7,7 @@#include "symbol.h" enum chain_mode { +CHAIN_NONE ,CHAIN_FLAT ,CHAIN_GRAPH_ABS ,CHAIN_GRAPH_REL
Signature · 1 line
1.6.2.3
PATCH 2/3 perfcounter: Generalize the tracepoint sampling to generic sampling
In reply to Frederic WeisbeckerRename:
- 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 <[email protected]>
--- include/linux/perf_counter.h | 10 +++++----- kernel/perf_counter.c | 20 ++++++++++---------- 2 files changed, 15 insertions(+), 15 deletions(-)
include/linux/perf_counter.h +5 -5
@@ -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,
kernel/perf_counter.c +10 -10
@@ -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)
Signature · 1 line
1.6.2.3
[tip:perfcounters/core] perf_counter: Fix tracepoint sampling to be part of generic sampling
Commit-ID: 6455883ad50c575b0bc5f6d6ab45d42cd56b83c4 Gitweb: http://git.kernel.org/tip/6455883ad50c575b0bc5f6d6ab45d42cd56b83c4 Author: Frederic Weisbecker <[email protected]> AuthorDate: Sat, 8 Aug 2009 04:26:37 +0200 Committer: Ingo Molnar <[email protected]> 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 <[email protected]>
LKML-Reference: <[email protected]>
--- include/linux/perf_counter.h | 10 +++++----- kernel/perf_counter.c | 20 ++++++++++---------- 2 files changed, 15 insertions(+), 15 deletions(-)
include/linux/perf_counter.h +5 -5
@@ -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,
kernel/perf_counter.c +10 -10
@@ -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
In reply to Frederic WeisbeckerCommit-ID: 3a43ce68ae1758fa6a839386025ef45acb6baa22 Gitweb: http://git.kernel.org/tip/3a43ce68ae1758fa6a839386025ef45acb6baa22 Author: Frederic Weisbecker <[email protected]> AuthorDate: Sat, 8 Aug 2009 04:26:37 +0200 Committer: Ingo Molnar <[email protected]> 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 <[email protected]>
LKML-Reference: <[email protected]>
--- include/linux/perf_counter.h | 10 +++++----- kernel/perf_counter.c | 20 ++++++++++---------- 2 files changed, 15 insertions(+), 15 deletions(-)
include/linux/perf_counter.h +5 -5
@@ -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,
kernel/perf_counter.c +10 -10
@@ -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 4/4 perf tools: callchain: Display amount of ignored chains in fractal mode
In reply to Frederic WeisbeckerWhen 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%-- [...]--- tools/perf/builtin-report.c | 47 ++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 44 insertions(+), 3 deletions(-)
tools/perf/builtin-report.c +44 -3
@@ -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; }
Signature · 1 line
1.6.2.3
PATCH 3/3 perfcounter: Align ftrace events raw samples to 8 bytes
In reply to Frederic WeisbeckerAlign the ftrace events raw samples to 8 bytes so that they meet the perf output event alignements requirements.
--- include/trace/ftrace.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
include/trace/ftrace.h +1 -1
@@ -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]; \
Signature · 1 line
1.6.2.3
[tip:perfcounters/core] perf_counter: Fix ftrace events raw samples to be aligned to 8 bytes
Commit-ID: eba67b1fd06180d57b94bf97557d89873b0ce61e Gitweb: http://git.kernel.org/tip/eba67b1fd06180d57b94bf97557d89873b0ce61e Author: Frederic Weisbecker <[email protected]> AuthorDate: Sat, 8 Aug 2009 04:26:39 +0200 Committer: Ingo Molnar <[email protected]> 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.
LKML-Reference: <[email protected]>
--- include/trace/ftrace.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
include/trace/ftrace.h +1 -1
@@ -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]; \
Peter Zijlstra
maintainerRe: [PATCH 3/3] perfcounter: Align ftrace events raw samples to 8 bytes
In reply to Frederic WeisbeckerQuoted 18 lines · 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 <[email protected]>Signed-off-by Frederic Weisbecker <[email protected]>Cc Peter Zijlstra <[email protected]>Cc Arnaldo Carvalho de Melo <[email protected]>Cc Mike Galbraith <[email protected]>Cc Paul Mackerras <[email protected]>--- include/trace/ftrace.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)include/trace/ftrace.h +0
-0
include/trace/ftrace.h static void ftrace_profile_##call(proto) \ · line 693
Quoted 8 lines
include/trace/ftrace.h +1
-1@@ -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
Quoted 14 lines · Peter Zijlstra
Quoted 13 lines · 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 <[email protected]>Signed-off-by Frederic Weisbecker <[email protected]>Cc Peter Zijlstra <[email protected]>Cc Arnaldo Carvalho de Melo <[email protected]>Cc Mike Galbraith <[email protected]>Cc Paul Mackerras <[email protected]>--- include/trace/ftrace.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
Quoted 16 lines
include/trace/ftrace.h static void ftrace_profile_##call(proto) \ · line 693
Quoted 13 lines
include/trace/ftrace.h +1
-1@@ -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.
Quoted 3 lines
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
In reply to Peter ZijlstraAfter 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 [email protected]........ ^ ^ ^ ^ 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--- include/trace/ftrace.h | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
include/trace/ftrace.h +3 -0
@@ -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); \
Signature · 1 line
1.6.2.3
[tip:perfcounters/urgent] perf_counter: Zero dead bytes from ftrace raw samples size alignment
Commit-ID: 1853db0e02ae4088f102b0d8e59e83dc98f93f03 Gitweb: http://git.kernel.org/tip/1853db0e02ae4088f102b0d8e59e83dc98f93f03 Author: Frederic Weisbecker <[email protected]> AuthorDate: Mon, 10 Aug 2009 16:38:36 +0200 Committer: Ingo Molnar <[email protected]> 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 [email protected]........ ^ ^ ^ ^ 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........
^ ^ ^ ^
FixedLKML-Reference: <[email protected]>
--- include/trace/ftrace.h | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)
include/trace/ftrace.h +4 -0
@@ -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); \