* [PATCH] perf script: fix crash when processing recorded stat data
@ 2019-01-20 19:14 Tony Jones
2019-01-20 22:01 ` Jiri Olsa
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Tony Jones @ 2019-01-20 19:14 UTC (permalink / raw)
To: linux-perf-users; +Cc: Tony Jones, acme, Jin Yao, jolsa, ak, linux-kernel
While updating Perf to work with Python3 and Python2 I noticed that the
stat-cpi script was dumping core.
$ perf stat -e cycles,instructions record -o /tmp/perf.data /bin/false
Performance counter stats for '/bin/false':
802,148 cycles
604,622 instructions 802,148 cycles
604,622 instructions
0.001445842 seconds time elapsed
$ perf script -i /tmp/perf.data -s scripts/python/stat-cpi.py
Segmentation fault (core dumped)
...
...
rblist=rblist@entry=0xb2a200 <rt_stat>,
new_entry=new_entry@entry=0x7ffcb755c310) at util/rblist.c:33
ctx=<optimized out>, type=<optimized out>, create=<optimized out>,
cpu=<optimized out>, evsel=<optimized out>) at util/stat-shadow.c:118
ctx=<optimized out>, type=<optimized out>, st=<optimized out>)
at util/stat-shadow.c:196
count=count@entry=727442, cpu=cpu@entry=0, st=0xb2a200 <rt_stat>)
at util/stat-shadow.c:239
config=config@entry=0xafeb40 <stat_config>,
counter=counter@entry=0x133c6e0) at util/stat.c:372
...
...
The issue is that since 1fcd03946b52 perf_stat__update_shadow_stats now calls
update_runtime_stat passing rt_stat rather than calling update_stats but
perf_stat__init_shadow_stats has never been called to initialize rt_stat in
the script path processing recorded stat data.
Since I can't see any reason why perf_stat__init_shadow_stats() is presently
initialized like it is in builtin-script.c::perf_sample__fprint_metric()
[4bd1bef8bba2f] I'm proposing it instead be initialized once in __cmd_script
Fixes: 1fcd03946b52 ("perf stat: Update per-thread shadow stats")
Signed-off-by: Tony Jones <tonyj@suse.de>
---
tools/perf/builtin-script.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index d079f36d342d..9a6dd86e606f 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -1681,13 +1681,8 @@ static void perf_sample__fprint_metric(struct perf_script *script,
.force_header = false,
};
struct perf_evsel *ev2;
- static bool init;
u64 val;
- if (!init) {
- perf_stat__init_shadow_stats();
- init = true;
- }
if (!evsel->stats)
perf_evlist__alloc_stats(script->session->evlist, false);
if (evsel_script(evsel->leader)->gnum++ == 0)
@@ -2359,6 +2354,8 @@ static int __cmd_script(struct perf_script *script)
signal(SIGINT, sig_handler);
+ perf_stat__init_shadow_stats();
+
/* override event processing functions */
if (script->show_task_events) {
script->tool.comm = process_comm_event;
--
2.18.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] perf script: fix crash when processing recorded stat data
2019-01-20 19:14 [PATCH] perf script: fix crash when processing recorded stat data Tony Jones
@ 2019-01-20 22:01 ` Jiri Olsa
2019-01-21 5:46 ` Ravi Bangoria
2019-01-22 11:37 ` [tip:perf/urgent] perf script: Fix " tip-bot for Tony Jones
2 siblings, 0 replies; 5+ messages in thread
From: Jiri Olsa @ 2019-01-20 22:01 UTC (permalink / raw)
To: Tony Jones; +Cc: linux-perf-users, acme, Jin Yao, ak, linux-kernel
On Sun, Jan 20, 2019 at 11:14:14AM -0800, Tony Jones wrote:
> While updating Perf to work with Python3 and Python2 I noticed that the
> stat-cpi script was dumping core.
>
> $ perf stat -e cycles,instructions record -o /tmp/perf.data /bin/false
>
> Performance counter stats for '/bin/false':
>
> 802,148 cycles
>
> 604,622 instructions 802,148 cycles
> 604,622 instructions
>
> 0.001445842 seconds time elapsed
>
> $ perf script -i /tmp/perf.data -s scripts/python/stat-cpi.py
> Segmentation fault (core dumped)
> ...
> ...
> rblist=rblist@entry=0xb2a200 <rt_stat>,
> new_entry=new_entry@entry=0x7ffcb755c310) at util/rblist.c:33
> ctx=<optimized out>, type=<optimized out>, create=<optimized out>,
> cpu=<optimized out>, evsel=<optimized out>) at util/stat-shadow.c:118
> ctx=<optimized out>, type=<optimized out>, st=<optimized out>)
> at util/stat-shadow.c:196
> count=count@entry=727442, cpu=cpu@entry=0, st=0xb2a200 <rt_stat>)
> at util/stat-shadow.c:239
> config=config@entry=0xafeb40 <stat_config>,
> counter=counter@entry=0x133c6e0) at util/stat.c:372
> ...
> ...
>
> The issue is that since 1fcd03946b52 perf_stat__update_shadow_stats now calls
> update_runtime_stat passing rt_stat rather than calling update_stats but
> perf_stat__init_shadow_stats has never been called to initialize rt_stat in
> the script path processing recorded stat data.
>
> Since I can't see any reason why perf_stat__init_shadow_stats() is presently
> initialized like it is in builtin-script.c::perf_sample__fprint_metric()
> [4bd1bef8bba2f] I'm proposing it instead be initialized once in __cmd_script
>
> Fixes: 1fcd03946b52 ("perf stat: Update per-thread shadow stats")
> Signed-off-by: Tony Jones <tonyj@suse.de>
> ---
> tools/perf/builtin-script.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index d079f36d342d..9a6dd86e606f 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -1681,13 +1681,8 @@ static void perf_sample__fprint_metric(struct perf_script *script,
> .force_header = false,
> };
> struct perf_evsel *ev2;
> - static bool init;
> u64 val;
>
> - if (!init) {
> - perf_stat__init_shadow_stats();
> - init = true;
> - }
well, there's no need to use shadow stats until stat
data is processed.. but it's actually just a static
initialization, so there's no need for late init
Reviewed-by: Jiri Olsa <jolsa@kernel.org>
thanks,
jirka
> if (!evsel->stats)
> perf_evlist__alloc_stats(script->session->evlist, false);
> if (evsel_script(evsel->leader)->gnum++ == 0)
> @@ -2359,6 +2354,8 @@ static int __cmd_script(struct perf_script *script)
>
> signal(SIGINT, sig_handler);
>
> + perf_stat__init_shadow_stats();
> +
> /* override event processing functions */
> if (script->show_task_events) {
> script->tool.comm = process_comm_event;
> --
> 2.18.0
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] perf script: fix crash when processing recorded stat data
2019-01-20 19:14 [PATCH] perf script: fix crash when processing recorded stat data Tony Jones
2019-01-20 22:01 ` Jiri Olsa
@ 2019-01-21 5:46 ` Ravi Bangoria
2019-01-21 14:15 ` Arnaldo Carvalho de Melo
2019-01-22 11:37 ` [tip:perf/urgent] perf script: Fix " tip-bot for Tony Jones
2 siblings, 1 reply; 5+ messages in thread
From: Ravi Bangoria @ 2019-01-21 5:46 UTC (permalink / raw)
To: Tony Jones, acme
Cc: linux-perf-users, Jin Yao, jolsa, ak, linux-kernel, Ravi Bangoria
On 1/21/19 12:44 AM, Tony Jones wrote:
> While updating Perf to work with Python3 and Python2 I noticed that the
> stat-cpi script was dumping core.
[...]
> Fixes: 1fcd03946b52 ("perf stat: Update per-thread shadow stats")
> Signed-off-by: Tony Jones <tonyj@suse.de>
Tested-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] perf script: fix crash when processing recorded stat data
2019-01-21 5:46 ` Ravi Bangoria
@ 2019-01-21 14:15 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 5+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-01-21 14:15 UTC (permalink / raw)
To: Ravi Bangoria
Cc: Tony Jones, linux-perf-users, Jin Yao, jolsa, ak, linux-kernel
Em Mon, Jan 21, 2019 at 11:16:26AM +0530, Ravi Bangoria escreveu:
>
>
> On 1/21/19 12:44 AM, Tony Jones wrote:
> > While updating Perf to work with Python3 and Python2 I noticed that the
> > stat-cpi script was dumping core.
>
> [...]
> > Fixes: 1fcd03946b52 ("perf stat: Update per-thread shadow stats")
> > Signed-off-by: Tony Jones <tonyj@suse.de>
>
> Tested-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Thanks, applied. Added your tby and Jiri's rby.
- Arnaldo
^ permalink raw reply [flat|nested] 5+ messages in thread
* [tip:perf/urgent] perf script: Fix crash when processing recorded stat data
2019-01-20 19:14 [PATCH] perf script: fix crash when processing recorded stat data Tony Jones
2019-01-20 22:01 ` Jiri Olsa
2019-01-21 5:46 ` Ravi Bangoria
@ 2019-01-22 11:37 ` tip-bot for Tony Jones
2 siblings, 0 replies; 5+ messages in thread
From: tip-bot for Tony Jones @ 2019-01-22 11:37 UTC (permalink / raw)
To: linux-tip-commits
Cc: jolsa, ak, tglx, linux-kernel, ravi.bangoria, hpa, yao.jin,
tonyj, acme, mingo
Commit-ID: 8bf8c6da53c2265aea365a1de6038f118f522113
Gitweb: https://git.kernel.org/tip/8bf8c6da53c2265aea365a1de6038f118f522113
Author: Tony Jones <tonyj@suse.de>
AuthorDate: Sun, 20 Jan 2019 11:14:14 -0800
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 21 Jan 2019 11:29:07 -0300
perf script: Fix crash when processing recorded stat data
While updating perf to work with Python3 and Python2 I noticed that the
stat-cpi script was dumping core.
$ perf stat -e cycles,instructions record -o /tmp/perf.data /bin/false
Performance counter stats for '/bin/false':
802,148 cycles
604,622 instructions 802,148 cycles
604,622 instructions
0.001445842 seconds time elapsed
$ perf script -i /tmp/perf.data -s scripts/python/stat-cpi.py
Segmentation fault (core dumped)
...
...
rblist=rblist@entry=0xb2a200 <rt_stat>,
new_entry=new_entry@entry=0x7ffcb755c310) at util/rblist.c:33
ctx=<optimized out>, type=<optimized out>, create=<optimized out>,
cpu=<optimized out>, evsel=<optimized out>) at util/stat-shadow.c:118
ctx=<optimized out>, type=<optimized out>, st=<optimized out>)
at util/stat-shadow.c:196
count=count@entry=727442, cpu=cpu@entry=0, st=0xb2a200 <rt_stat>)
at util/stat-shadow.c:239
config=config@entry=0xafeb40 <stat_config>,
counter=counter@entry=0x133c6e0) at util/stat.c:372
...
...
The issue is that since 1fcd03946b52 perf_stat__update_shadow_stats now calls
update_runtime_stat passing rt_stat rather than calling update_stats but
perf_stat__init_shadow_stats has never been called to initialize rt_stat in
the script path processing recorded stat data.
Since I can't see any reason why perf_stat__init_shadow_stats() is presently
initialized like it is in builtin-script.c::perf_sample__fprint_metric()
[4bd1bef8bba2f] I'm proposing it instead be initialized once in __cmd_script
Committer testing:
After applying the patch:
# perf script -i /tmp/perf.data -s tools/perf/scripts/python/stat-cpi.py
0.001970: cpu -1, thread -1 -> cpi 1.709079 (1075684/629394)
#
No segfault.
Signed-off-by: Tony Jones <tonyj@suse.de>
Reviewed-by: Jiri Olsa <jolsa@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Tested-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Jin Yao <yao.jin@linux.intel.com>
Fixes: 1fcd03946b52 ("perf stat: Update per-thread shadow stats")
Link: http://lkml.kernel.org/r/20190120191414.12925-1-tonyj@suse.de
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/builtin-script.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 357906ed1898..ac221f137ed2 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -1681,13 +1681,8 @@ static void perf_sample__fprint_metric(struct perf_script *script,
.force_header = false,
};
struct perf_evsel *ev2;
- static bool init;
u64 val;
- if (!init) {
- perf_stat__init_shadow_stats();
- init = true;
- }
if (!evsel->stats)
perf_evlist__alloc_stats(script->session->evlist, false);
if (evsel_script(evsel->leader)->gnum++ == 0)
@@ -2359,6 +2354,8 @@ static int __cmd_script(struct perf_script *script)
signal(SIGINT, sig_handler);
+ perf_stat__init_shadow_stats();
+
/* override event processing functions */
if (script->show_task_events) {
script->tool.comm = process_comm_event;
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-01-22 11:38 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-20 19:14 [PATCH] perf script: fix crash when processing recorded stat data Tony Jones
2019-01-20 22:01 ` Jiri Olsa
2019-01-21 5:46 ` Ravi Bangoria
2019-01-21 14:15 ` Arnaldo Carvalho de Melo
2019-01-22 11:37 ` [tip:perf/urgent] perf script: Fix " tip-bot for Tony Jones
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).