Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1278004 > unrolled thread
| Started by | Namhyung Kim <namhyung@kernel.org> |
|---|---|
| First post | 2015-11-26 08:10 +0100 |
| Last post | 2015-11-27 08:50 +0100 |
| Articles | 17 — 6 participants |
Back to article view | Back to linux.kernel
[PATCH 1/3] perf top: Fix freeze on --call-graph flat/folded Namhyung Kim <namhyung@kernel.org> - 2015-11-26 08:10 +0100
[PATCH 2/3] perf callchain: Stop resolving callchains after invalid address Namhyung Kim <namhyung@kernel.org> - 2015-11-26 08:10 +0100
Re: [PATCH 2/3] perf callchain: Stop resolving callchains after invalid address Ingo Molnar <mingo@kernel.org> - 2015-11-26 08:50 +0100
Re: [PATCH 2/3] perf callchain: Stop resolving callchains after invalid address Namhyung Kim <namhyung@kernel.org> - 2015-11-26 15:20 +0100
Re: [PATCH 2/3] perf callchain: Stop resolving callchains after invalid address Ingo Molnar <mingo@kernel.org> - 2015-11-27 08:50 +0100
Re: [PATCH 2/3] perf callchain: Stop resolving callchains after invalid address David Ahern <dsahern@gmail.com> - 2015-11-26 14:20 +0100
Re: [PATCH 2/3] perf callchain: Stop resolving callchains after invalid address Namhyung Kim <namhyung@kernel.org> - 2015-11-26 16:10 +0100
Re: [PATCH 2/3] perf callchain: Stop resolving callchains after invalid address David Ahern <dsahern@gmail.com> - 2015-11-26 16:50 +0100
Re: [PATCH 2/3] perf callchain: Stop resolving callchains after invalid address Jiri Olsa <jolsa@redhat.com> - 2015-11-26 17:00 +0100
Re: [PATCH 2/3] perf callchain: Stop resolving callchains after invalid address Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-11-26 17:20 +0100
Re: [PATCH 1/3] perf top: Fix freeze on --call-graph flat/folded Jiri Olsa <jolsa@redhat.com> - 2015-11-26 09:40 +0100
Re: [PATCH 1/3] perf top: Fix freeze on --call-graph flat/folded Namhyung Kim <namhyung@kernel.org> - 2015-11-26 15:00 +0100
Re: [PATCH 1/3] perf top: Fix freeze on --call-graph flat/folded Jiri Olsa <jolsa@redhat.com> - 2015-11-26 15:10 +0100
Re: [PATCH 1/3] perf top: Fix freeze on --call-graph flat/folded Namhyung Kim <namhyung@kernel.org> - 2015-11-26 16:20 +0100
Re: [PATCH 1/3] perf top: Fix freeze on --call-graph flat/folded Jiri Olsa <jolsa@redhat.com> - 2015-11-26 16:30 +0100
Re: [PATCH 1/3] perf top: Fix freeze on --call-graph flat/folded Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-11-26 17:40 +0100
[tip:perf/core] perf top: Fix freeze on --call-graph flat/folded tip-bot for Namhyung Kim <tipbot@zytor.com> - 2015-11-27 08:50 +0100
| From | Namhyung Kim <namhyung@kernel.org> |
|---|---|
| Date | 2015-11-26 08:10 +0100 |
| Subject | [PATCH 1/3] perf top: Fix freeze on --call-graph flat/folded |
| Message-ID | <qyYQO-1zw-5@gated-at.bofh.it> |
The callchain rbtree is rebuilt periodically, so it needs to
reinitialize the root everytime. Otherwise it can be stuck in the
rbtree insertion with stale pointers.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/util/callchain.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index fc3b1e0d09ee..564377d2bebf 100644
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -290,6 +290,7 @@ static void
sort_chain_flat(struct rb_root *rb_root, struct callchain_root *root,
u64 min_hit, struct callchain_param *param __maybe_unused)
{
+ *rb_root = RB_ROOT;
__sort_chain_flat(rb_root, &root->node, min_hit);
}
--
2.6.2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [next] | [standalone]
| From | Namhyung Kim <namhyung@kernel.org> |
|---|---|
| Date | 2015-11-26 08:10 +0100 |
| Subject | [PATCH 2/3] perf callchain: Stop resolving callchains after invalid address |
| Message-ID | <qyYQO-1zw-21@gated-at.bofh.it> |
| In reply to | #1278004 |
Unwinding optimized binaries using frame pointer gives garbage. Check
callchain address and stop if it's under vm.mmap_min_addr sysctl value.
Before:
$ perf report --stdio --no-children -g callee
...
1.37% perf [kernel.vmlinux] [k] smp_call_function_single
|
---smp_call_function_single
_perf_event_enable
perf_event_for_each_child
perf_ioctl
do_vfs_ioctl
sys_ioctl
entry_SYSCALL_64_fastpath
__GI___ioctl
0
0
0x1c5aa70
0x1c5b910
0x1c5aa70
0x1c5b910
0x1c5aa70
0x1c5b910
0x1c5aa70
0x1c5b910
0x1c5aa70
0x1c5b910
...
After:
$ perf report --stdio --no-children -g callee
...
1.37% perf [kernel.vmlinux] [k] smp_call_function_single
|
---smp_call_function_single
_perf_event_enable
perf_event_for_each_child
perf_ioctl
do_vfs_ioctl
sys_ioctl
entry_SYSCALL_64_fastpath
__GI___ioctl
$ perf report --stdio --no-children -g caller
...
1.37% perf [kernel.vmlinux] [k] smp_call_function_single
|
---__GI__ioctl
entry_SYSCALL_64_fastpath
sys_ioctl
do_vfs_ioctl
perf_ioctl
perf_event_for_each_child
_perf_event_enable
smp_call_function_single
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/perf.c | 6 ++++++
tools/perf/util/machine.c | 13 +++++++++++++
tools/perf/util/util.c | 1 +
tools/perf/util/util.h | 1 +
4 files changed, 21 insertions(+)
diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index 4bee53c3f796..0075a6d38e3a 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -18,6 +18,7 @@
#include "util/bpf-loader.h"
#include "util/debug.h"
#include <api/fs/tracing_path.h>
+#include <api/fs/fs.h>
#include <pthread.h>
const char perf_usage_string[] =
@@ -528,11 +529,16 @@ int main(int argc, const char **argv)
{
const char *cmd;
char sbuf[STRERR_BUFSIZE];
+ int min_addr;
/* The page_size is placed in util object. */
page_size = sysconf(_SC_PAGE_SIZE);
cacheline_size = sysconf(_SC_LEVEL1_DCACHE_LINESIZE);
+ if (sysctl__read_int("vm/mmap_min_addr", &min_addr) < 0)
+ min_addr = page_size;
+ mmap_min_addr = min_addr;
+
cmd = perf_extract_argv0_path(argv[0]);
if (!cmd)
cmd = "perf-help";
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 7f5071a4d9aa..0a2f35e0d737 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -1860,6 +1860,19 @@ static int thread__resolve_callchain_sample(struct thread *thread,
#endif
ip = chain->ips[j];
+ /*
+ * Callchain value under mmap_min_addr means it's broken
+ * or the end of callchain. Stop.
+ */
+ if (ip < mmap_min_addr) {
+ if (callchain_param.order == ORDER_CALLEE)
+ break;
+
+ /* ignore current callchains for CALLER order */
+ callchain_cursor_reset(&callchain_cursor);
+ continue;
+ }
+
err = add_callchain_ip(thread, parent, root_al, &cpumode, ip);
if (err)
diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
index 75759aebc7b8..8e198acd9fa6 100644
--- a/tools/perf/util/util.c
+++ b/tools/perf/util/util.c
@@ -30,6 +30,7 @@ struct callchain_param callchain_param = {
*/
unsigned int page_size;
int cacheline_size;
+unsigned long mmap_min_addr;
bool test_attr__enabled;
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index dcc659017976..506d4dbb58be 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -281,6 +281,7 @@ void sighandler_dump_stack(int sig);
extern unsigned int page_size;
extern int cacheline_size;
+extern unsigned long mmap_min_addr;
void get_term_dimensions(struct winsize *ws);
void set_term_quiet_input(struct termios *old);
--
2.6.2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Ingo Molnar <mingo@kernel.org> |
|---|---|
| Date | 2015-11-26 08:50 +0100 |
| Subject | Re: [PATCH 2/3] perf callchain: Stop resolving callchains after invalid address |
| Message-ID | <qyZtv-1TC-17@gated-at.bofh.it> |
| In reply to | #1278009 |
* Namhyung Kim <namhyung@kernel.org> wrote:
> Unwinding optimized binaries using frame pointer gives garbage. Check
> callchain address and stop if it's under vm.mmap_min_addr sysctl value.
>
> Before:
> $ perf report --stdio --no-children -g callee
> ...
>
> 1.37% perf [kernel.vmlinux] [k] smp_call_function_single
> |
> ---smp_call_function_single
> _perf_event_enable
> perf_event_for_each_child
> perf_ioctl
> do_vfs_ioctl
> sys_ioctl
> entry_SYSCALL_64_fastpath
> __GI___ioctl
> 0
> 0
> 0x1c5aa70
> 0x1c5b910
> 0x1c5aa70
> 0x1c5b910
> 0x1c5aa70
> 0x1c5b910
> 0x1c5aa70
> 0x1c5b910
> 0x1c5aa70
> 0x1c5b910
> ...
>
> After:
> $ perf report --stdio --no-children -g callee
> ...
>
> 1.37% perf [kernel.vmlinux] [k] smp_call_function_single
> |
> ---smp_call_function_single
> _perf_event_enable
> perf_event_for_each_child
> perf_ioctl
> do_vfs_ioctl
> sys_ioctl
> entry_SYSCALL_64_fastpath
> __GI___ioctl
In addition to that, would it make sense to terminate the callchain with an
indicator that we found something anomalous? Such an extra line:
...
would not be intrusive, but would tell the informed reader that it's not a normal
ending of the call chain.
This assumes that we can tell apart 'normal end of call chain' from 'seems to end
with garbage poiner' cases - can do we that?
Thanks,
Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Namhyung Kim <namhyung@kernel.org> |
|---|---|
| Date | 2015-11-26 15:20 +0100 |
| Subject | Re: [PATCH 2/3] perf callchain: Stop resolving callchains after invalid address |
| Message-ID | <qz5yW-6iA-15@gated-at.bofh.it> |
| In reply to | #1278030 |
Hi Ingo, On Thu, Nov 26, 2015 at 08:43:35AM +0100, Ingo Molnar wrote: > > * Namhyung Kim <namhyung@kernel.org> wrote: > > > Unwinding optimized binaries using frame pointer gives garbage. Check > > callchain address and stop if it's under vm.mmap_min_addr sysctl value. > > > > Before: > > $ perf report --stdio --no-children -g callee > > ... > > > > 1.37% perf [kernel.vmlinux] [k] smp_call_function_single > > | > > ---smp_call_function_single > > _perf_event_enable > > perf_event_for_each_child > > perf_ioctl > > do_vfs_ioctl > > sys_ioctl > > entry_SYSCALL_64_fastpath > > __GI___ioctl > > 0 > > 0 > > 0x1c5aa70 > > 0x1c5b910 > > 0x1c5aa70 > > 0x1c5b910 > > 0x1c5aa70 > > 0x1c5b910 > > 0x1c5aa70 > > 0x1c5b910 > > 0x1c5aa70 > > 0x1c5b910 > > ... > > > > After: > > $ perf report --stdio --no-children -g callee > > ... > > > > 1.37% perf [kernel.vmlinux] [k] smp_call_function_single > > | > > ---smp_call_function_single > > _perf_event_enable > > perf_event_for_each_child > > perf_ioctl > > do_vfs_ioctl > > sys_ioctl > > entry_SYSCALL_64_fastpath > > __GI___ioctl > > In addition to that, would it make sense to terminate the callchain with an > indicator that we found something anomalous? Such an extra line: > > ... > > would not be intrusive, but would tell the informed reader that it's not a normal > ending of the call chain. > > This assumes that we can tell apart 'normal end of call chain' from 'seems to end > with garbage poiner' cases - can do we that? In case of fp unwind, I'm not sure we can determine whether it's normal end or not especially for optimized binaries. It seems kernel also can stop callchain anytime if it sees a broken frame. For dwarf unwind, I think it's also hard to tell since it can be stopped for various reasons like insufficient dump size or broken CFI, ... Thanks, Namhyung -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Ingo Molnar <mingo@kernel.org> |
|---|---|
| Date | 2015-11-27 08:50 +0100 |
| Subject | Re: [PATCH 2/3] perf callchain: Stop resolving callchains after invalid address |
| Message-ID | <qzlX4-8k4-37@gated-at.bofh.it> |
| In reply to | #1278256 |
* Namhyung Kim <namhyung@kernel.org> wrote:
> Hi Ingo,
>
> On Thu, Nov 26, 2015 at 08:43:35AM +0100, Ingo Molnar wrote:
> >
> > * Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > > Unwinding optimized binaries using frame pointer gives garbage. Check
> > > callchain address and stop if it's under vm.mmap_min_addr sysctl value.
> > >
> > > Before:
> > > $ perf report --stdio --no-children -g callee
> > > ...
> > >
> > > 1.37% perf [kernel.vmlinux] [k] smp_call_function_single
> > > |
> > > ---smp_call_function_single
> > > _perf_event_enable
> > > perf_event_for_each_child
> > > perf_ioctl
> > > do_vfs_ioctl
> > > sys_ioctl
> > > entry_SYSCALL_64_fastpath
> > > __GI___ioctl
> > > 0
> > > 0
> > > 0x1c5aa70
> > > 0x1c5b910
> > > 0x1c5aa70
> > > 0x1c5b910
> > > 0x1c5aa70
> > > 0x1c5b910
> > > 0x1c5aa70
> > > 0x1c5b910
> > > 0x1c5aa70
> > > 0x1c5b910
> > > ...
> > >
> > > After:
> > > $ perf report --stdio --no-children -g callee
> > > ...
> > >
> > > 1.37% perf [kernel.vmlinux] [k] smp_call_function_single
> > > |
> > > ---smp_call_function_single
> > > _perf_event_enable
> > > perf_event_for_each_child
> > > perf_ioctl
> > > do_vfs_ioctl
> > > sys_ioctl
> > > entry_SYSCALL_64_fastpath
> > > __GI___ioctl
> >
> > In addition to that, would it make sense to terminate the callchain with an
> > indicator that we found something anomalous? Such an extra line:
> >
> > ...
> >
> > would not be intrusive, but would tell the informed reader that it's not a normal
> > ending of the call chain.
> >
> > This assumes that we can tell apart 'normal end of call chain' from 'seems to end
> > with garbage poiner' cases - can do we that?
>
> In case of fp unwind, I'm not sure we can determine whether it's
> normal end or not especially for optimized binaries. It seems kernel
> also can stop callchain anytime if it sees a broken frame.
>
> For dwarf unwind, I think it's also hard to tell since it can be
> stopped for various reasons like insufficient dump size or broken CFI,
But but. Doesn't your patch 'detect' an anomaly to begin with?
+ /*
+ * Callchain value under mmap_min_addr means it's broken
+ * or the end of callchain. Stop.
+ */
+ if (ip < mmap_min_addr) {
+ if (callchain_param.order == ORDER_CALLEE)
+ break;
all I'm asking for is to indicate it in some low-key visual fashion when we
encounter such a 'broken' call-chain.
I presume the 'old' way of ending the call-chain was that 'ip' was zero, right? We
should not print the indicator in that case.
Also, in the dwarf case I'd also see value in indicating if any of these events
occured:
> For dwarf unwind, I think it's also hard to tell since it can be stopped for
> various reasons like insufficient dump size or broken CFI,
even if we cannot catch all anomalies. Performance analysis must stand firm on a
hard rock of reliability and dependability, and we should always propagate
information about possible profiling data corruption/unreliability. That's why we
print the 'IO overload' messages during perf record for example.
Even if the problem is not caused by perf, but by external factors such as the
compiler/linker.
Thanks,
Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | David Ahern <dsahern@gmail.com> |
|---|---|
| Date | 2015-11-26 14:20 +0100 |
| Subject | Re: [PATCH 2/3] perf callchain: Stop resolving callchains after invalid address |
| Message-ID | <qz4CR-5DC-9@gated-at.bofh.it> |
| In reply to | #1278009 |
On 11/26/15 12:08 AM, Namhyung Kim wrote:
> @@ -528,11 +529,16 @@ int main(int argc, const char **argv)
> {
> const char *cmd;
> char sbuf[STRERR_BUFSIZE];
> + int min_addr;
>
> /* The page_size is placed in util object. */
> page_size = sysconf(_SC_PAGE_SIZE);
> cacheline_size = sysconf(_SC_LEVEL1_DCACHE_LINESIZE);
>
> + if (sysctl__read_int("vm/mmap_min_addr", &min_addr) < 0)
This assumes the record and analysis are done on the same system.
David
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Namhyung Kim <namhyung@kernel.org> |
|---|---|
| Date | 2015-11-26 16:10 +0100 |
| Subject | Re: [PATCH 2/3] perf callchain: Stop resolving callchains after invalid address |
| Message-ID | <qz6lj-6Q1-1@gated-at.bofh.it> |
| In reply to | #1278206 |
Hi David,
On Thu, Nov 26, 2015 at 06:14:57AM -0700, David Ahern wrote:
> On 11/26/15 12:08 AM, Namhyung Kim wrote:
> >@@ -528,11 +529,16 @@ int main(int argc, const char **argv)
> > {
> > const char *cmd;
> > char sbuf[STRERR_BUFSIZE];
> >+ int min_addr;
> >
> > /* The page_size is placed in util object. */
> > page_size = sysconf(_SC_PAGE_SIZE);
> > cacheline_size = sysconf(_SC_LEVEL1_DCACHE_LINESIZE);
> >
> >+ if (sysctl__read_int("vm/mmap_min_addr", &min_addr) < 0)
>
> This assumes the record and analysis are done on the same system.
Right. Maybe we can just use minimal size (or page size?) or save and
pass it through somewhere in the feature bit?
Thanks,
Namhyung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | David Ahern <dsahern@gmail.com> |
|---|---|
| Date | 2015-11-26 16:50 +0100 |
| Subject | Re: [PATCH 2/3] perf callchain: Stop resolving callchains after invalid address |
| Message-ID | <qz6Y1-76F-3@gated-at.bofh.it> |
| In reply to | #1278274 |
On 11/26/15 8:00 AM, Namhyung Kim wrote:
> Hi David,
>
> On Thu, Nov 26, 2015 at 06:14:57AM -0700, David Ahern wrote:
>> On 11/26/15 12:08 AM, Namhyung Kim wrote:
>>> @@ -528,11 +529,16 @@ int main(int argc, const char **argv)
>>> {
>>> const char *cmd;
>>> char sbuf[STRERR_BUFSIZE];
>>> + int min_addr;
>>>
>>> /* The page_size is placed in util object. */
>>> page_size = sysconf(_SC_PAGE_SIZE);
>>> cacheline_size = sysconf(_SC_LEVEL1_DCACHE_LINESIZE);
>>>
>>> + if (sysctl__read_int("vm/mmap_min_addr", &min_addr) < 0)
>>
>> This assumes the record and analysis are done on the same system.
>
> Right. Maybe we can just use minimal size (or page size?) or save and
> pass it through somewhere in the feature bit?
no preference, but it should work with cross arch analysis as well
(e.g., record on arm/ppc and analysis on x86)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Jiri Olsa <jolsa@redhat.com> |
|---|---|
| Date | 2015-11-26 17:00 +0100 |
| Subject | Re: [PATCH 2/3] perf callchain: Stop resolving callchains after invalid address |
| Message-ID | <qz77I-7ak-7@gated-at.bofh.it> |
| In reply to | #1278305 |
On Thu, Nov 26, 2015 at 08:48:17AM -0700, David Ahern wrote:
> On 11/26/15 8:00 AM, Namhyung Kim wrote:
> >Hi David,
> >
> >On Thu, Nov 26, 2015 at 06:14:57AM -0700, David Ahern wrote:
> >>On 11/26/15 12:08 AM, Namhyung Kim wrote:
> >>>@@ -528,11 +529,16 @@ int main(int argc, const char **argv)
> >>> {
> >>> const char *cmd;
> >>> char sbuf[STRERR_BUFSIZE];
> >>>+ int min_addr;
> >>>
> >>> /* The page_size is placed in util object. */
> >>> page_size = sysconf(_SC_PAGE_SIZE);
> >>> cacheline_size = sysconf(_SC_LEVEL1_DCACHE_LINESIZE);
> >>>
> >>>+ if (sysctl__read_int("vm/mmap_min_addr", &min_addr) < 0)
> >>
> >>This assumes the record and analysis are done on the same system.
> >
> >Right. Maybe we can just use minimal size (or page size?) or save and
> >pass it through somewhere in the feature bit?
>
> no preference, but it should work with cross arch analysis as well (e.g.,
> record on arm/ppc and analysis on x86)
I think we should store it in perf.data in features, but seems
like a waste to spend one bit just for this number.
I remember commenting on new CPU related FEATURE data, that would contain
cpu specific data in extensible form like TAG,VALUE,TAG,VALUE..
but I think the design changed or something, because I cannot find it in now ;-)
maybe we could add something like that
jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Arnaldo Carvalho de Melo <acme@kernel.org> |
|---|---|
| Date | 2015-11-26 17:20 +0100 |
| Subject | Re: [PATCH 2/3] perf callchain: Stop resolving callchains after invalid address |
| Message-ID | <qz7r4-7xJ-3@gated-at.bofh.it> |
| In reply to | #1278311 |
Em Thu, Nov 26, 2015 at 04:58:57PM +0100, Jiri Olsa escreveu:
> On Thu, Nov 26, 2015 at 08:48:17AM -0700, David Ahern wrote:
> > On 11/26/15 8:00 AM, Namhyung Kim wrote:
> > >Hi David,
> > >
> > >On Thu, Nov 26, 2015 at 06:14:57AM -0700, David Ahern wrote:
> > >>On 11/26/15 12:08 AM, Namhyung Kim wrote:
> > >>>@@ -528,11 +529,16 @@ int main(int argc, const char **argv)
> > >>> {
> > >>> const char *cmd;
> > >>> char sbuf[STRERR_BUFSIZE];
> > >>>+ int min_addr;
> > >>>
> > >>> /* The page_size is placed in util object. */
> > >>> page_size = sysconf(_SC_PAGE_SIZE);
> > >>> cacheline_size = sysconf(_SC_LEVEL1_DCACHE_LINESIZE);
> > >>>
> > >>>+ if (sysctl__read_int("vm/mmap_min_addr", &min_addr) < 0)
Please put this in that symbol_conf kitchen sink :-)
I'm unsure though if there would be a reason for having both the local
min_addr and the one at perf record time, i.e. from perf.data :-\
> > >>This assumes the record and analysis are done on the same system.
> > >
> > >Right. Maybe we can just use minimal size (or page size?) or save and
> > >pass it through somewhere in the feature bit?
> >
> > no preference, but it should work with cross arch analysis as well (e.g.,
> > record on arm/ppc and analysis on x86)
>
> I think we should store it in perf.data in features, but seems
> like a waste to spend one bit just for this number.
>
> I remember commenting on new CPU related FEATURE data, that would contain
> cpu specific data in extensible form like TAG,VALUE,TAG,VALUE..
>
> but I think the design changed or something, because I cannot find it in now ;-)
>
> maybe we could add something like that
Right, isn't that one part of the perf/stat patchkit?
- Arnaldo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Jiri Olsa <jolsa@redhat.com> |
|---|---|
| Date | 2015-11-26 09:40 +0100 |
| Message-ID | <qz0fT-2yX-19@gated-at.bofh.it> |
| In reply to | #1278004 |
On Thu, Nov 26, 2015 at 04:08:18PM +0900, Namhyung Kim wrote:
> The callchain rbtree is rebuilt periodically, so it needs to
> reinitialize the root everytime. Otherwise it can be stuck in the
> rbtree insertion with stale pointers.
>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/perf/util/callchain.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
> index fc3b1e0d09ee..564377d2bebf 100644
> --- a/tools/perf/util/callchain.c
> +++ b/tools/perf/util/callchain.c
> @@ -290,6 +290,7 @@ static void
> sort_chain_flat(struct rb_root *rb_root, struct callchain_root *root,
> u64 min_hit, struct callchain_param *param __maybe_unused)
> {
> + *rb_root = RB_ROOT;
it seems ok, but I did not find how this could be called twice?
the only sort I can see is done within:
__hists__insert_output_entry
do we allow resorting of the callchains?
thanks,
jirka
> __sort_chain_flat(rb_root, &root->node, min_hit);
> }
>
> --
> 2.6.2
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Namhyung Kim <namhyung@kernel.org> |
|---|---|
| Date | 2015-11-26 15:00 +0100 |
| Message-ID | <qz5fC-5UD-37@gated-at.bofh.it> |
| In reply to | #1278082 |
Hi Jiri,
On Thu, Nov 26, 2015 at 09:38:53AM +0100, Jiri Olsa wrote:
> On Thu, Nov 26, 2015 at 04:08:18PM +0900, Namhyung Kim wrote:
> > The callchain rbtree is rebuilt periodically, so it needs to
> > reinitialize the root everytime. Otherwise it can be stuck in the
> > rbtree insertion with stale pointers.
> >
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> > tools/perf/util/callchain.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
> > index fc3b1e0d09ee..564377d2bebf 100644
> > --- a/tools/perf/util/callchain.c
> > +++ b/tools/perf/util/callchain.c
> > @@ -290,6 +290,7 @@ static void
> > sort_chain_flat(struct rb_root *rb_root, struct callchain_root *root,
> > u64 min_hit, struct callchain_param *param __maybe_unused)
> > {
> > + *rb_root = RB_ROOT;
>
> it seems ok, but I did not find how this could be called twice?
>
> the only sort I can see is done within:
> __hists__insert_output_entry
>
> do we allow resorting of the callchains?
No, but I think it's possible though.
It's called from perf top's display thread.
display_thread()
-> while (!done)
-> perf_top__print_sym_table()
-> hists__output_resort()
-> __hists__insert_output_entry()
Thanks,
Namhyung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Jiri Olsa <jolsa@redhat.com> |
|---|---|
| Date | 2015-11-26 15:10 +0100 |
| Message-ID | <qz5pg-6dE-7@gated-at.bofh.it> |
| In reply to | #1278243 |
On Thu, Nov 26, 2015 at 10:52:56PM +0900, Namhyung Kim wrote:
> Hi Jiri,
>
> On Thu, Nov 26, 2015 at 09:38:53AM +0100, Jiri Olsa wrote:
> > On Thu, Nov 26, 2015 at 04:08:18PM +0900, Namhyung Kim wrote:
> > > The callchain rbtree is rebuilt periodically, so it needs to
> > > reinitialize the root everytime. Otherwise it can be stuck in the
> > > rbtree insertion with stale pointers.
> > >
> > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > > ---
> > > tools/perf/util/callchain.c | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
> > > index fc3b1e0d09ee..564377d2bebf 100644
> > > --- a/tools/perf/util/callchain.c
> > > +++ b/tools/perf/util/callchain.c
> > > @@ -290,6 +290,7 @@ static void
> > > sort_chain_flat(struct rb_root *rb_root, struct callchain_root *root,
> > > u64 min_hit, struct callchain_param *param __maybe_unused)
> > > {
> > > + *rb_root = RB_ROOT;
> >
> > it seems ok, but I did not find how this could be called twice?
> >
> > the only sort I can see is done within:
> > __hists__insert_output_entry
> >
> > do we allow resorting of the callchains?
>
> No, but I think it's possible though.
>
> It's called from perf top's display thread.
ah right.. top ;-) ok
isn't there analogical issue with the other sorts?
graph_abs, graph_rel... I dont see that code doing this
thanks,
jirka
>
> display_thread()
> -> while (!done)
> -> perf_top__print_sym_table()
> -> hists__output_resort()
> -> __hists__insert_output_entry()
>
> Thanks,
> Namhyung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Namhyung Kim <namhyung@kernel.org> |
|---|---|
| Date | 2015-11-26 16:20 +0100 |
| Message-ID | <qz6v0-6U1-25@gated-at.bofh.it> |
| In reply to | #1278248 |
On Thu, Nov 26, 2015 at 03:00:34PM +0100, Jiri Olsa wrote:
> On Thu, Nov 26, 2015 at 10:52:56PM +0900, Namhyung Kim wrote:
> > Hi Jiri,
> >
> > On Thu, Nov 26, 2015 at 09:38:53AM +0100, Jiri Olsa wrote:
> > > On Thu, Nov 26, 2015 at 04:08:18PM +0900, Namhyung Kim wrote:
> > > > The callchain rbtree is rebuilt periodically, so it needs to
> > > > reinitialize the root everytime. Otherwise it can be stuck in the
> > > > rbtree insertion with stale pointers.
> > > >
> > > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > > > ---
> > > > tools/perf/util/callchain.c | 1 +
> > > > 1 file changed, 1 insertion(+)
> > > >
> > > > diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
> > > > index fc3b1e0d09ee..564377d2bebf 100644
> > > > --- a/tools/perf/util/callchain.c
> > > > +++ b/tools/perf/util/callchain.c
> > > > @@ -290,6 +290,7 @@ static void
> > > > sort_chain_flat(struct rb_root *rb_root, struct callchain_root *root,
> > > > u64 min_hit, struct callchain_param *param __maybe_unused)
> > > > {
> > > > + *rb_root = RB_ROOT;
> > >
> > > it seems ok, but I did not find how this could be called twice?
> > >
> > > the only sort I can see is done within:
> > > __hists__insert_output_entry
> > >
> > > do we allow resorting of the callchains?
> >
> > No, but I think it's possible though.
> >
> > It's called from perf top's display thread.
>
> ah right.. top ;-) ok
>
> isn't there analogical issue with the other sorts?
> graph_abs, graph_rel... I dont see that code doing this
The sort_chain_graph_abs/rel() already do this. Unlike flat/folded
callchains, they put child into each callchain_node's rbtree. So they
reinitialize node->rb_root in __sort_chain_graph_abs/rel() and reset
rb_root after finishing sort.
Thanks,
Namhyung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Jiri Olsa <jolsa@redhat.com> |
|---|---|
| Date | 2015-11-26 16:30 +0100 |
| Message-ID | <qz6EH-6XT-43@gated-at.bofh.it> |
| In reply to | #1278285 |
On Fri, Nov 27, 2015 at 12:10:31AM +0900, Namhyung Kim wrote:
> On Thu, Nov 26, 2015 at 03:00:34PM +0100, Jiri Olsa wrote:
> > On Thu, Nov 26, 2015 at 10:52:56PM +0900, Namhyung Kim wrote:
> > > Hi Jiri,
> > >
> > > On Thu, Nov 26, 2015 at 09:38:53AM +0100, Jiri Olsa wrote:
> > > > On Thu, Nov 26, 2015 at 04:08:18PM +0900, Namhyung Kim wrote:
> > > > > The callchain rbtree is rebuilt periodically, so it needs to
> > > > > reinitialize the root everytime. Otherwise it can be stuck in the
> > > > > rbtree insertion with stale pointers.
> > > > >
> > > > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > > > > ---
> > > > > tools/perf/util/callchain.c | 1 +
> > > > > 1 file changed, 1 insertion(+)
> > > > >
> > > > > diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
> > > > > index fc3b1e0d09ee..564377d2bebf 100644
> > > > > --- a/tools/perf/util/callchain.c
> > > > > +++ b/tools/perf/util/callchain.c
> > > > > @@ -290,6 +290,7 @@ static void
> > > > > sort_chain_flat(struct rb_root *rb_root, struct callchain_root *root,
> > > > > u64 min_hit, struct callchain_param *param __maybe_unused)
> > > > > {
> > > > > + *rb_root = RB_ROOT;
> > > >
> > > > it seems ok, but I did not find how this could be called twice?
> > > >
> > > > the only sort I can see is done within:
> > > > __hists__insert_output_entry
> > > >
> > > > do we allow resorting of the callchains?
> > >
> > > No, but I think it's possible though.
> > >
> > > It's called from perf top's display thread.
> >
> > ah right.. top ;-) ok
> >
> > isn't there analogical issue with the other sorts?
> > graph_abs, graph_rel... I dont see that code doing this
>
> The sort_chain_graph_abs/rel() already do this. Unlike flat/folded
> callchains, they put child into each callchain_node's rbtree. So they
> reinitialize node->rb_root in __sort_chain_graph_abs/rel() and reset
> rb_root after finishing sort.
ook, thanks for explanation
Acked-by: Jiri Olsa <jolsa@kernel.org>
jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Arnaldo Carvalho de Melo <acme@kernel.org> |
|---|---|
| Date | 2015-11-26 17:40 +0100 |
| Message-ID | <qz7Kq-7Fr-3@gated-at.bofh.it> |
| In reply to | #1278004 |
Em Thu, Nov 26, 2015 at 04:08:18PM +0900, Namhyung Kim escreveu: > The callchain rbtree is rebuilt periodically, so it needs to > reinitialize the root everytime. Otherwise it can be stuck in the > rbtree insertion with stale pointers. > > Signed-off-by: Namhyung Kim <namhyung@kernel.org> Applied. - Arnaldo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | tip-bot for Namhyung Kim <tipbot@zytor.com> |
|---|---|
| Date | 2015-11-27 08:50 +0100 |
| Subject | [tip:perf/core] perf top: Fix freeze on --call-graph flat/folded |
| Message-ID | <qzlX4-8k4-21@gated-at.bofh.it> |
| In reply to | #1278004 |
Commit-ID: 0356218a68551f051998f4fb5074a1eed7a346fe
Gitweb: http://git.kernel.org/tip/0356218a68551f051998f4fb5074a1eed7a346fe
Author: Namhyung Kim <namhyung@kernel.org>
AuthorDate: Thu, 26 Nov 2015 16:08:18 +0900
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 26 Nov 2015 13:32:08 -0300
perf top: Fix freeze on --call-graph flat/folded
The callchain rbtree is rebuilt periodically, so it needs to
reinitialize the root everytime. Otherwise it can be stuck in the
rbtree insertion with stale pointers.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1448521700-32062-1-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/callchain.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index fc3b1e0..564377d 100644
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -290,6 +290,7 @@ static void
sort_chain_flat(struct rb_root *rb_root, struct callchain_root *root,
u64 min_hit, struct callchain_param *param __maybe_unused)
{
+ *rb_root = RB_ROOT;
__sort_chain_flat(rb_root, &root->node, min_hit);
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web