perf: inherit_stat related fixes
Subsystems (2)
Jiri Olsa
reviewercover perf: inherit_stat related fixes
hi,
Mark reported an issue with inherit_stat code
and posted a fix [1].
I found we have some issues with that in perf
tool as well, so sending his fix together with
perf tool fixes.
The patchset is also available in:
git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
perf/values
thanks,
jirka
[1] https://marc.info/?l=linux-kernel&m=150238662928203&w=2
---
Jiri Olsa (10):
perf/x86: Add warning on proper cpu during event's update
perf: Fix leader for removed sibling event in perf_group_detach
perf: Make sure we read only scheduled events
perf record: Set read_format for inherit_stat
perf report: Add dump_read function
perf values: Fix thread index bug
perf values: Fix allocation check
perf values: Zero value buffers
perf report: Group stat values on global event id perf stat: Support inherit/no-inherit terms
arch/x86/events/core.c | 2 ++ kernel/events/core.c | 11 ++++++++--- tools/perf/builtin-report.c | 6 +----- tools/perf/builtin-stat.c | 19 +++++++++++++++++++ tools/perf/util/evsel.c | 7 ++++++- tools/perf/util/parse-events.c | 2 ++ tools/perf/util/session.c | 25 +++++++++++++++++++++++++ tools/perf/util/values.c | 17 +++++++++++------ 8 files changed, 74 insertions(+), 15 deletions(-)
Jiri Olsa
reviewerPATCH 1/10 perf/x86: Add warning on proper cpu during event's update
Mark reported that we could actually call pmu->read on unscheduled event. I think it's good idea to keep a warning here to see if we've get it wrong again in future.
--- arch/x86/events/core.c | 2 ++ 1 file changed, 2 insertions(+)
arch/x86/events/core.c +2 -0
@@ -72,6 +72,8 @@ u64 x86_perf_event_update(struct perf_event *event)int idx = hwc->idx;u64 delta; +WARN_ON_ONCE (event->oncpu !=smp_processor_id ()); +if (idx ==INTEL_PMC_IDX_FIXED_BTS )return 0 ;
Signature · 1 line
2.9.5
Peter Zijlstra
maintainerRe: [PATCH 01/10] perf/x86: Add warning on proper cpu during event's update
Quoted 7 lines · Jiri Olsa
Mark reported that we could actually call
pmu->readon unscheduled event. I think it's good idea to keep a warning here to see if we've get it wrong again in future.Reported-by Mark Rutland <[email protected]>Signed-off-by Jiri Olsa <[email protected]>
Yeah, no objection to this one.
Ack
Quoted 20 lines
--- arch/x86/events/core.c | 2 ++ 1 file changed, 2 insertions(+)arch/x86/events/core.c +2
-0@@ -72,6 +72,8 @@ u64 x86_perf_event_update(struct perf_event *event)int idx = hwc->idx;u64 delta; +WARN_ON_ONCE (event->oncpu !=smp_processor_id ()); +if (idx ==INTEL_PMC_IDX_FIXED_BTS )return 0 ;Signature · 1 line
2.9.5
Jiri Olsa
reviewerPATCH 2/10 perf: Fix leader for removed sibling event in perf_group_detach
In reply to Jiri OlsaMark reported warning in x86_perf_event_update caused by reading unscheduled leader of an event that was already removed from the group.
As he pointed out we don't properly reset event's leader once it's been detached from the group and he posted the attached fix.
[1] https://marc.info/?l=linux-kernel&m=150238662928203&w=2
Originally-From: Mark Rutland <[email protected]>
--- kernel/events/core.c | 1 + 1 file changed, 1 insertion(+)
kernel/events/core.c +1 -0
@@ -1727,6 +1727,7 @@ static void perf_group_detach(struct perf_event *event)if (event->group_leader != event) {list_del_init (&event->group_entry); event->group_leader->nr_siblings--; + event->group_leader = event; goto out; }
Signature · 1 line
2.9.5
Peter Zijlstra
maintainerRe: [PATCH 02/10] perf: Fix leader for removed sibling event in perf_group_detach
Quoted 21 lines · Jiri Olsa
Mark reported warning in
x86_perf_event_updatecaused by reading unscheduled leader of an event that was already removed from the group.As he pointed out we don't properly reset event's leader once it's been detached from the group and he posted the attached fix.
[1] https://marc.info/?l=linux-kernel&m=150238662928203&w=2
Originally-From: Mark Rutland <[email protected]>
Signed-off-by Jiri Olsa <[email protected]>--- kernel/events/core.c | 1 + 1 file changed, 1 insertion(+)kernel/events/core.c +0
-0
kernel/events/core.c static void perf_group_detach(struct perf_event *event) · line 1732
Quoted 6 lines
kernel/events/core.c +1
-0@@ -1727,6 +1727,7 @@ static void perf_group_detach(struct perf_event *event)if (event->group_leader != event) {list_del_init (&event->group_entry); event->group_leader->nr_siblings--; + event->group_leader = event; goto out; }
No, I think this breaks other things... but I again forgot what. The original issue he reported is fixed by patch:
2bcb88eb04d3 ("perf/core: fix group {cpu,task} validation")
Also by Mark.
Jiri Olsa
reviewerPATCH 3/10 perf: Make sure we read only scheduled events
In reply to Jiri OlsaAdding leader's state check into perf_output_read_group to ensure we read only leader, which is scheduled in.
Similar check is already there for siblings.
--- kernel/events/core.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
kernel/events/core.c +7 -3
@@ -5760,6 +5760,11 @@ void perf_event__output_id_sample(struct perf_event *event,__perf_event__output_id_sample (handle, sample); } +static bool can_read (struct perf_event *event) +{ +return event->state ==PERF_EVENT_STATE_ACTIVE ; +} +static void perf_output_read_one (struct perf_output_handle *handle,struct perf_event *event,u64 enabled,u64 running)
@@ -5800,7 +5805,7 @@ static void perf_output_read_group(struct perf_output_handle *handle,if (read_format &PERF_FORMAT_TOTAL_TIME_RUNNING ) values[n++] = running; -if (leader != event) +if ((leader != event) &&can_read (leader)) leader->pmu->read (leader); values[n++] =perf_event_count (leader);
@@ -5812,8 +5817,7 @@ static void perf_output_read_group(struct perf_output_handle *handle,list_for_each_entry (sub, &leader->sibling_list, group_entry) { n =0 ; -if ((sub != event) && - (sub->state ==PERF_EVENT_STATE_ACTIVE )) +if ((sub != event) &&can_read (sub)) sub->pmu->read (sub); values[n++] =perf_event_count (sub);
Signature · 1 line
2.9.5
Peter Zijlstra
maintainerRe: [PATCH 03/10] perf: Make sure we read only scheduled events
Quoted 36 lines · Jiri Olsa
Adding leader's state check into
perf_output_read_groupto ensure we read only leader, which is scheduled in.Similar check is already there for siblings.
Signed-off-by Jiri Olsa <[email protected]>--- kernel/events/core.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)kernel/events/core.c +6
-1@@ -5760,6 +5760,11 @@ void perf_event__output_id_sample(struct perf_event *event,__perf_event__output_id_sample (handle, sample); } +static bool can_read (struct perf_event *event) +{ +return event->state ==PERF_EVENT_STATE_ACTIVE ; +} +static void perf_output_read_one (struct perf_output_handle *handle,struct perf_event *event,u64 enabled,u64 running)@@ -5800,7 +5805,7 @@ static void perf_output_read_group(struct perf_output_handle *handle,if (read_format &PERF_FORMAT_TOTAL_TIME_RUNNING ) values[n++] = running; -if (leader != event) +if ((leader != event) &&can_read (leader)) leader->pmu->read (leader); values[n++] =perf_event_count (leader);
kernel/events/core.c static void perf_output_read_group(struct perf_output_handle *handle, · line 5823
Quoted 9 lines
kernel/events/core.c +1
-2@@ -5812,8 +5817,7 @@ static void perf_output_read_group(struct perf_output_handle *handle,list_for_each_entry (sub, &leader->sibling_list, group_entry) { n =0 ; -if ((sub != event) && - (sub->state ==PERF_EVENT_STATE_ACTIVE )) +if ((sub != event) &&can_read (sub)) sub->pmu->read (sub); values[n++] =perf_event_count (sub);
I'm not seeing how this makes sense. Groups should either _all_ be scheduled or not at all. Please explain.
Re: [PATCH 03/10] perf: Make sure we read only scheduled events
Quoted 45 lines · Peter Zijlstra
Quoted 44 lines · Jiri Olsa
Adding leader's state check into
perf_output_read_groupto ensure we read only leader, which is scheduled in.Similar check is already there for siblings.
Signed-off-by Jiri Olsa <[email protected]>--- kernel/events/core.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)kernel/events/core.c +7
-3@@ -5760,6 +5760,11 @@ void perf_event__output_id_sample(struct perf_event *event,__perf_event__output_id_sample (handle, sample); } +static bool can_read (struct perf_event *event) +{ +return event->state ==PERF_EVENT_STATE_ACTIVE ; +} +static void perf_output_read_one (struct perf_output_handle *handle,struct perf_event *event,u64 enabled,u64 running)@@ -5800,7 +5805,7 @@ static void perf_output_read_group(struct perf_output_handle *handle,if (read_format &PERF_FORMAT_TOTAL_TIME_RUNNING ) values[n++] = running; -if (leader != event) +if ((leader != event) &&can_read (leader)) leader->pmu->read (leader); values[n++] =perf_event_count (leader);@@ -5812,8 +5817,7 @@ static void perf_output_read_group(struct perf_output_handle *handle,list_for_each_entry (sub, &leader->sibling_list, group_entry) { n =0 ; -if ((sub != event) && - (sub->state ==PERF_EVENT_STATE_ACTIVE )) +if ((sub != event) &&can_read (sub)) sub->pmu->read (sub);
Quoted 4 lines
kernel/events/core.c static void perf_output_read_group(struct perf_output_handle *handle, · line 5823
Quoted 1 line
kernel/events/core.c +0
-0values[n++] =perf_event_count (sub);I'm not seeing how this makes sense. Groups should either _all_ be scheduled or not at all. Please explain.
so this could be called for event which is already scheduled out:
perf_event_exit_task_context
task_ctx_sched_out <- unschedules event
perf_event_exit_event
sync_child_event
perf_event_read_event
perf_output_read
if leader != events (which is, if you don't have Mark's fix), we'll call leader->pmu->read(leader) even if it's not scheduled in
jirka
Jiri Olsa
reviewerPATCH 4/10 perf record: Set read_format for inherit_stat
In reply to Jiri OlsaSet read_format for what we expect to get from read event generated by perf_event_attr::inherit_stat.
--- tools/perf/util/evsel.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
tools/perf/util/evsel.c +6 -1
@@ -902,8 +902,13 @@ void perf_evsel__config(struct perf_evsel *evsel, struct record_opts *opts,if (opts->no_samples) attr->sample_freq =0 ; -if (opts->inherit_stat) +if (opts->inherit_stat) { + evsel->attr.read_format |= +PERF_FORMAT_TOTAL_TIME_ENABLED | +PERF_FORMAT_TOTAL_TIME_RUNNING | +PERF_FORMAT_ID ; attr->inherit_stat =1 ; + }if (opts->sample_address) {perf_evsel__set_sample_bit (evsel,ADDR );
Signature · 1 line
2.9.5
tip-bot for Jiri Olsa
merge bot[tip:perf/core] perf record: Set read_format for inherit_stat
perf record: Set read_format for inherit_stat
Commit-ID: a17f06978769735ab5c7598c46881fa201e9b1a2 Gitweb: http://git.kernel.org/tip/a17f06978769735ab5c7598c46881fa201e9b1a2 Author: Jiri Olsa <[email protected]> AuthorDate: Thu, 24 Aug 2017 18:27:31 +0200 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Mon, 28 Aug 2017 11:05:10 -0300
perf record: Set read_format for inherit_stat
Set read_format for what we expect to get from read event generated by perf_event_attr::inherit_stat.
--- tools/perf/util/evsel.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
tools/perf/util/evsel.c +6 -1
@@ -902,8 +902,13 @@ void perf_evsel__config(struct perf_evsel *evsel, struct record_opts *opts,if (opts->no_samples) attr->sample_freq =0 ; -if (opts->inherit_stat) +if (opts->inherit_stat) { + evsel->attr.read_format |= +PERF_FORMAT_TOTAL_TIME_ENABLED | +PERF_FORMAT_TOTAL_TIME_RUNNING | +PERF_FORMAT_ID ; attr->inherit_stat =1 ; + }if (opts->sample_address) {perf_evsel__set_sample_bit (evsel,ADDR );
Jiri Olsa
reviewerPATCH 5/10 perf report: Add dump_read function
In reply to Jiri OlsaAdding dump_read function to gather all the dump output of read function. Adding output of enabled and running times and id if enabled (3 new lines with '...' prefix below).
$ perf record -s ... $ perf report -D
958358311769 0x91f8 [0x40]: PERF_RECORD_READ: 3339 3339 cycles:u 0 ... time enabled : 958358313731
... time running : 958358313731
... id : 80
--- tools/perf/builtin-report.c | 4 ---- tools/perf/util/session.c | 25 +++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 4 deletions(-)
tools/perf/builtin-report.c +0 -4
@@ -249,10 +249,6 @@ static int process_read_event(struct perf_tool *tool,return err; } -dump_printf (": %d %d %s %" PRIu64"\n" , event->read.pid, event->read.tid, - evsel ?perf_evsel__name (evsel) :"FAIL" , - event->read.value); -return 0 ; }
tools/perf/util/session.c +25 -0
@@ -1127,6 +1127,30 @@ static void dump_sample(struct perf_evsel *evsel, union perf_event *event,sample_read__printf (sample, evsel->attr.read_format); } +static void dump_read (struct perf_evsel *evsel,union perf_event *event) +{ +struct read_event *read = &event->read; +u64 read_format; + +if (!dump_trace) +return ; + +printf (": %d %d %s %" PRIu64"\n" , event->read.pid, event->read.tid, + evsel ?perf_evsel__name (evsel) :"FAIL" , + event->read.value); + + read_format = evsel->attr.read_format; + +if (read_format &PERF_FORMAT_TOTAL_TIME_ENABLED ) +printf ("... time enabled : %" PRIu64"\n" , read->time_enabled); + +if (read_format &PERF_FORMAT_TOTAL_TIME_RUNNING ) +printf ("... time running : %" PRIu64"\n" , read->time_running); + +if (read_format &PERF_FORMAT_ID ) +printf ("... id : %" PRIu64"\n" , read->id); +} +static struct machine *machines__find_for_cpumode (struct machines *machines,union perf_event *event,struct perf_sample *sample)
@@ -1271,6 +1295,7 @@ static int machines__deliver_event(struct machines *machines, evlist->stats.total_lost_samples += event->lost_samples.lost;return tool->lost_samples (tool, event, sample, machine);case PERF_RECORD_READ : +dump_read (evsel, event);return tool->read (tool, event, sample, evsel, machine);case PERF_RECORD_THROTTLE :return tool->throttle (tool, event, sample, machine);
Signature · 1 line
2.9.5
tip-bot for Jiri Olsa
merge bot[tip:perf/core] perf report: Add dump_read function
perf report: Add dump_read function
Commit-ID: dac7f6b7ed1c8601358357f60e9764a4c6a68d71 Gitweb: http://git.kernel.org/tip/dac7f6b7ed1c8601358357f60e9764a4c6a68d71 Author: Jiri Olsa <[email protected]> AuthorDate: Thu, 24 Aug 2017 18:27:32 +0200 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Mon, 28 Aug 2017 16:43:50 -0300
perf report: Add dump_read function
Adding dump_read function to gather all the dump output of read function. Adding output of enabled and running times and id if enabled (3 new lines with '...' prefix below).
$ perf record -s ... $ perf report -D
958358311769 0x91f8 [0x40]: PERF_RECORD_READ: 3339 3339 cycles:u 0 ... time enabled : 958358313731
... time running : 958358313731
... id : 80
Committer note:
Do not use 'read' as a variable name as it breaks the build on older systems, such as RHEL6:
CC /tmp/build/perf/util/session.o
cc1: warnings being treated as errors
util/session.c: In function 'dump_read':
util/session.c:1132: error: declaration of 'read' shadows a global declaration /usr/include/bits/unistd.h:35: error: shadowed declaration is here
mv: cannot stat `/tmp/build/perf/util/.session.o.tmp': No such file or directory
--- tools/perf/builtin-report.c | 4 ---- tools/perf/util/session.c | 25 +++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 4 deletions(-)
tools/perf/builtin-report.c +0 -4
@@ -249,10 +249,6 @@ static int process_read_event(struct perf_tool *tool,return err; } -dump_printf (": %d %d %s %" PRIu64"\n" , event->read.pid, event->read.tid, - evsel ?perf_evsel__name (evsel) :"FAIL" , - event->read.value); -return 0 ; }
tools/perf/util/session.c +25 -0
@@ -1127,6 +1127,30 @@ static void dump_sample(struct perf_evsel *evsel, union perf_event *event,sample_read__printf (sample, evsel->attr.read_format); } +static void dump_read (struct perf_evsel *evsel,union perf_event *event) +{ +struct read_event *read_event = &event->read; +u64 read_format; + +if (!dump_trace) +return ; + +printf (": %d %d %s %" PRIu64"\n" , event->read.pid, event->read.tid, + evsel ?perf_evsel__name (evsel) :"FAIL" , + event->read.value); + + read_format = evsel->attr.read_format; + +if (read_format &PERF_FORMAT_TOTAL_TIME_ENABLED ) +printf ("... time enabled : %" PRIu64"\n" , read_event->time_enabled); + +if (read_format &PERF_FORMAT_TOTAL_TIME_RUNNING ) +printf ("... time running : %" PRIu64"\n" , read_event->time_running); + +if (read_format &PERF_FORMAT_ID ) +printf ("... id : %" PRIu64"\n" , read_event->id); +} +static struct machine *machines__find_for_cpumode (struct machines *machines,union perf_event *event,struct perf_sample *sample)
@@ -1271,6 +1295,7 @@ static int machines__deliver_event(struct machines *machines, evlist->stats.total_lost_samples += event->lost_samples.lost;return tool->lost_samples (tool, event, sample, machine);case PERF_RECORD_READ : +dump_read (evsel, event);return tool->read (tool, event, sample, evsel, machine);case PERF_RECORD_THROTTLE :return tool->throttle (tool, event, sample, machine);
Jiri Olsa
reviewerPATCH 6/10 perf values: Fix thread index bug
In reply to Jiri OlsaWe are taking wrong index (+1) for first thread, which leaves thread with index 0 unused and uninitialized.
--- tools/perf/util/values.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
tools/perf/util/values.c +2 -2
@@ -98,7 +98,7 @@ static int perf_read_values__findnew_thread(struct perf_read_values *values,return i; } - i = values->threads +1 ; + i = values->threads; values->value[i] =malloc (values->counters_max *sizeof (**values->value));if (!values->value[i]) {pr_debug ("failed to allocate read_values counters array" );
@@ -106,7 +106,7 @@ static int perf_read_values__findnew_thread(struct perf_read_values *values, } values->pid[i] = pid; values->tid[i] = tid; - values->threads = i; + values->threads = i +1 ;return i; }
Signature · 1 line
2.9.5
tip-bot for Jiri Olsa
merge bot[tip:perf/core] perf values: Fix thread index bug
perf values: Fix thread index bug
Commit-ID: 64eed1deb6d87f4c0efe03297f50367a3689eb56 Gitweb: http://git.kernel.org/tip/64eed1deb6d87f4c0efe03297f50367a3689eb56 Author: Jiri Olsa <[email protected]> AuthorDate: Thu, 24 Aug 2017 18:27:33 +0200 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Mon, 28 Aug 2017 16:44:42 -0300
perf values: Fix thread index bug
We are taking wrong index (+1) for first thread, which leaves thread with index 0 unused and uninitialized.
--- tools/perf/util/values.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
tools/perf/util/values.c +2 -2
@@ -98,7 +98,7 @@ static int perf_read_values__findnew_thread(struct perf_read_values *values,return i; } - i = values->threads +1 ; + i = values->threads; values->value[i] =malloc (values->counters_max *sizeof (**values->value));if (!values->value[i]) {pr_debug ("failed to allocate read_values counters array" );
@@ -106,7 +106,7 @@ static int perf_read_values__findnew_thread(struct perf_read_values *values, } values->pid[i] = pid; values->tid[i] = tid; - values->threads = i; + values->threads = i +1 ;return i; }
Jiri Olsa
reviewerPATCH 7/10 perf values: Fix allocation check
In reply to Jiri OlsaBailing out in case the allocation failed,
not the other way round.
--- tools/perf/util/values.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
tools/perf/util/values.c +1 -1
@@ -131,7 +131,7 @@ static int perf_read_values__enlarge_counters(struct perf_read_values *values)for (i =0 ; i < values->threads; i++) {u64 *value =realloc (values->value[i], counters_max *sizeof (**values->value)); -if (value) { +if (!value) {pr_debug ("failed to enlarge read_values ->values array" ); goto out_free_name; }
Signature · 1 line
2.9.5
tip-bot for Jiri Olsa
merge bot[tip:perf/core] perf values: Fix allocation check
perf values: Fix allocation check
Commit-ID: f4ef3b7c184c4c269f953f226f7158347d007622 Gitweb: http://git.kernel.org/tip/f4ef3b7c184c4c269f953f226f7158347d007622 Author: Jiri Olsa <[email protected]> AuthorDate: Thu, 24 Aug 2017 18:27:34 +0200 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Mon, 28 Aug 2017 16:44:43 -0300
perf values: Fix allocation check
Bailing out in case the allocation failed, not the other way round.
--- tools/perf/util/values.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
tools/perf/util/values.c +1 -1
@@ -131,7 +131,7 @@ static int perf_read_values__enlarge_counters(struct perf_read_values *values)for (i =0 ; i < values->threads; i++) {u64 *value =realloc (values->value[i], counters_max *sizeof (**values->value)); -if (value) { +if (!value) {pr_debug ("failed to enlarge read_values ->values array" ); goto out_free_name; }
Jiri Olsa
reviewerPATCH 8/10 perf values: Zero value buffers
In reply to Jiri OlsaWe need to make sure the array of value pointers are zero initialized, because we use them in realloc later on and uninitialized non zero value will cause allocation error and aborted execution.
--- tools/perf/util/values.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
tools/perf/util/values.c +7 -2
@@ -12,7 +12,7 @@ int perf_read_values_init(struct perf_read_values *values) values->threads_max =16 ; values->pid =malloc (values->threads_max *sizeof (*values->pid)); values->tid =malloc (values->threads_max *sizeof (*values->tid)); - values->value =malloc (values->threads_max *sizeof (*values->value)); + values->value =zalloc (values->threads_max *sizeof (*values->value));if (!values->pid || !values->tid || !values->value) {pr_debug ("failed to allocate read_values threads arrays" ); goto out_free_pid;
@@ -99,7 +99,8 @@ static int perf_read_values__findnew_thread(struct perf_read_values *values, } i = values->threads; - values->value[i] =malloc (values->counters_max *sizeof (**values->value)); + + values->value[i] =zalloc (values->counters_max *sizeof (**values->value));if (!values->value[i]) {pr_debug ("failed to allocate read_values counters array" );return -ENOMEM ;
@@ -130,12 +131,16 @@ static int perf_read_values__enlarge_counters(struct perf_read_values *values)for (i =0 ; i < values->threads; i++) {u64 *value =realloc (values->value[i], counters_max *sizeof (**values->value)); +int j;if (!value) {pr_debug ("failed to enlarge read_values ->values array" ); goto out_free_name; } +for (j = values->counters_max; j < counters_max; j++) + value[j] =0 ; + values->value[i] = value; }
Signature · 1 line
2.9.5
tip-bot for Jiri Olsa
merge bot[tip:perf/core] perf values: Zero value buffers
perf values: Zero value buffers
Commit-ID: a1834fc938344dd3015a1df64ee7f2af70ded147 Gitweb: http://git.kernel.org/tip/a1834fc938344dd3015a1df64ee7f2af70ded147 Author: Jiri Olsa <[email protected]> AuthorDate: Thu, 24 Aug 2017 18:27:35 +0200 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Mon, 28 Aug 2017 16:44:43 -0300
perf values: Zero value buffers
We need to make sure the array of value pointers are zero initialized, because we use them in realloc later on and uninitialized non zero value will cause allocation error and aborted execution.
--- tools/perf/util/values.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
tools/perf/util/values.c +7 -2
@@ -12,7 +12,7 @@ int perf_read_values_init(struct perf_read_values *values) values->threads_max =16 ; values->pid =malloc (values->threads_max *sizeof (*values->pid)); values->tid =malloc (values->threads_max *sizeof (*values->tid)); - values->value =malloc (values->threads_max *sizeof (*values->value)); + values->value =zalloc (values->threads_max *sizeof (*values->value));if (!values->pid || !values->tid || !values->value) {pr_debug ("failed to allocate read_values threads arrays" ); goto out_free_pid;
@@ -99,7 +99,8 @@ static int perf_read_values__findnew_thread(struct perf_read_values *values, } i = values->threads; - values->value[i] =malloc (values->counters_max *sizeof (**values->value)); + + values->value[i] =zalloc (values->counters_max *sizeof (**values->value));if (!values->value[i]) {pr_debug ("failed to allocate read_values counters array" );return -ENOMEM ;
@@ -130,12 +131,16 @@ static int perf_read_values__enlarge_counters(struct perf_read_values *values)for (i =0 ; i < values->threads; i++) {u64 *value =realloc (values->value[i], counters_max *sizeof (**values->value)); +int j;if (!value) {pr_debug ("failed to enlarge read_values ->values array" ); goto out_free_name; } +for (j = values->counters_max; j < counters_max; j++) + value[j] =0 ; + values->value[i] = value; }
Jiri Olsa
reviewerPATCH 9/10 perf report: Group stat values on global event id
In reply to Jiri OlsaThere's no big value on displaying counts for every event ID, which is one per every CPU. Rather than that, displaying the whole sum for the event.
$ perf record -c 100000 -e cycles:u -s test $ perf report -T
Before:
# PID TID cycles:u cycles:u cycles:u cycles:u ... [20 more columns of 'cycles:u']
3339 3339 0 0 0 0
3340 3340 0 0 0 0
3341 3341 0 0 0 0
3342 3342 0 0 0 0Now:
# PID TID cycles:u
3339 3339 19678
3340 3340 18744
3341 3341 17335
3342 3342 26414--- tools/perf/builtin-report.c | 2 +- tools/perf/util/values.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
tools/perf/builtin-report.c +1 -1
@@ -241,7 +241,7 @@ static int process_read_event(struct perf_tool *tool,const char *name = evsel ?perf_evsel__name (evsel) :"unknown" ;int err =perf_read_values_add_value (&rep->show_threads_values, event->read.pid, event->read.tid, - event->read.id, + evsel->idx, name, event->read.value);
tools/perf/util/values.c +1 -1
@@ -192,7 +192,7 @@ int perf_read_values_add_value(struct perf_read_values *values,if (cindex <0 )return cindex; - values->value[tindex][cindex] = value; + values->value[tindex][cindex] += value;return 0 ; }
Signature · 1 line
2.9.5
tip-bot for Jiri Olsa
merge bot[tip:perf/core] perf report: Group stat values on global event id
perf report: Group stat values on global event id
Commit-ID: 9933183e365f7dd3a79507f1ffb4bcf9433a73ee Gitweb: http://git.kernel.org/tip/9933183e365f7dd3a79507f1ffb4bcf9433a73ee Author: Jiri Olsa <[email protected]> AuthorDate: Thu, 24 Aug 2017 18:27:36 +0200 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Mon, 28 Aug 2017 16:44:44 -0300
perf report: Group stat values on global event id
There's no big value on displaying counts for every event ID, which is one per every CPU. Rather than that, displaying the whole sum for the event.
$ perf record -c 100000 -e cycles:u -s test $ perf report -T
Before:
# PID TID cycles:u cycles:u cycles:u cycles:u ... [20 more columns of 'cycles:u']
3339 3339 0 0 0 0
3340 3340 0 0 0 0
3341 3341 0 0 0 0
3342 3342 0 0 0 0Now:
# PID TID cycles:u
3339 3339 19678
3340 3340 18744
3341 3341 17335
3342 3342 26414--- tools/perf/builtin-report.c | 2 +- tools/perf/util/values.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
tools/perf/builtin-report.c +1 -1
@@ -241,7 +241,7 @@ static int process_read_event(struct perf_tool *tool,const char *name = evsel ?perf_evsel__name (evsel) :"unknown" ;int err =perf_read_values_add_value (&rep->show_threads_values, event->read.pid, event->read.tid, - event->read.id, + evsel->idx, name, event->read.value);
tools/perf/util/values.c +1 -1
@@ -192,7 +192,7 @@ int perf_read_values_add_value(struct perf_read_values *values,if (cindex <0 )return cindex; - values->value[tindex][cindex] = value; + values->value[tindex][cindex] += value;return 0 ; }
Jiri Olsa
reviewerPATCH 10/10 perf stat: Support inherit/no-inherit terms
In reply to Jiri OlsaAdding support to use 'inherit' and 'no-inherit' terms in perf stat command, like:
To disable perf_event_attr::inherit (enabled by default):
$ perf stat -e cpu/cpu-cycles,no-inherit/u ...
Enable perf_event_attr::inherit (disabled by -i):
$ perf stat -i -e cpu/cpu-cycles,inherit/u ...
--- tools/perf/builtin-stat.c | 19 +++++++++++++++++++ tools/perf/util/parse-events.c | 2 ++ 2 files changed, 21 insertions(+)
tools/perf/builtin-stat.c +19 -0
@@ -210,6 +210,23 @@ static void perf_stat__reset_stats(void)perf_stat__reset_shadow_stats (); } +static void apply_stat_config_terms (struct perf_evsel *evsel) +{ +struct perf_evsel_config_term *term; +struct list_head *config_terms = &evsel->config_terms; +struct perf_event_attr *attr = &evsel->attr; + +list_for_each_entry (term, config_terms, list) { +switch (term->type) { +case PERF_EVSEL__CONFIG_TERM_INHERIT : + attr->inherit = term->val.inherit ?1 :0 ; +break ; +default : +break ; + } + } +} +static int create_perf_stat_counter (struct perf_evsel *evsel) {struct perf_event_attr *attr = &evsel->attr;
@@ -264,6 +281,8 @@ static int create_perf_stat_counter(struct perf_evsel *evsel) attr->enable_on_exec =1 ; } +apply_stat_config_terms (evsel); +if (target__has_cpu (&target))return perf_evsel__open_per_cpu (evsel,perf_evsel__cpus (evsel));
tools/perf/util/parse-events.c +2 -0
@@ -934,6 +934,8 @@ config_term_avail(int term_type, struct parse_events_error *err)case PARSE_EVENTS__TERM_TYPE_CONFIG2 :case PARSE_EVENTS__TERM_TYPE_NAME :case PARSE_EVENTS__TERM_TYPE_SAMPLE_PERIOD : +case PARSE_EVENTS__TERM_TYPE_INHERIT : +case PARSE_EVENTS__TERM_TYPE_NOINHERIT :return true;default :if (!err)
Signature · 1 line
2.9.5
Re: [PATCH 10/10] perf stat: Support inherit/no-inherit terms
Quoted 10 lines · Jiri Olsa
Adding support to use 'inherit' and 'no-inherit' terms in perf stat command, like:
To disable
perf_event_attr::inherit(enabled by default):$ perf stat -e cpu/cpu-cycles,no-inherit/u ...Enable
perf_event_attr::inherit(disabled by -i):$ perf stat -i -e cpu/cpu-cycles,inherit/u ...
User documentation is missing.
-Andi
Re: [PATCH 10/10] perf stat: Support inherit/no-inherit terms
Quoted 6 lines · Andi Kleen
Quoted 5 lines · Jiri Olsa
Adding support to use 'inherit' and 'no-inherit' terms in perf stat command, like:
To disable
perf_event_attr::inherit(enabled by default):
Quoted 7 lines
Quoted 5 lines
$ perf stat -e cpu/cpu-cycles,no-inherit/u ...Enable
perf_event_attr::inherit(disabled by -i):$ perf stat -i -e cpu/cpu-cycles,inherit/u ...User documentation is missing.
right, will add
thanks,
jirka
Arnaldo Carvalho de Melo
maintainerRe: [PATCH 10/10] perf stat: Support inherit/no-inherit terms
Em Thu, Aug 24, 2017 at 06:45:27PM +0200, Jiri Olsa escreveu:
Quoted 9 lines · Andi Kleen
Quoted 8 lines · Jiri Olsa
Quoted 7 lines
Adding support to use 'inherit' and 'no-inherit' terms in perf stat command, like:
To disable
perf_event_attr::inherit(enabled by default):$ perf stat -e cpu/cpu-cycles,no-inherit/u ...
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
Enable
perf_event_attr::inherit(disabled by -i):$ perf stat -i -e cpu/cpu-cycles,inherit/u ...User documentation is missing.
right, will add
Ok, applied 4-9, Peter, are you ok with the kernel parts?
- Arnaldo