* [PATCH 01/10] perf/x86: Add warning on proper cpu during event's update
2017-08-24 16:27 [PATCH 00/10] perf: inherit_stat related fixes Jiri Olsa
@ 2017-08-24 16:27 ` Jiri Olsa
2017-08-28 19:24 ` Peter Zijlstra
2017-08-24 16:27 ` [PATCH 02/10] perf: Fix leader for removed sibling event in perf_group_detach Jiri Olsa
` (8 subsequent siblings)
9 siblings, 1 reply; 24+ messages in thread
From: Jiri Olsa @ 2017-08-24 16:27 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Peter Zijlstra
Cc: lkml, Ingo Molnar, Alexander Shishkin, Namhyung Kim, David Ahern,
Andi Kleen, Mark Rutland
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.
Reported-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
arch/x86/events/core.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
index af12e294caed..b8e394d9f7f2 100644
--- a/arch/x86/events/core.c
+++ b/arch/x86/events/core.c
@@ -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;
--
2.9.5
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 01/10] perf/x86: Add warning on proper cpu during event's update
2017-08-24 16:27 ` [PATCH 01/10] perf/x86: Add warning on proper cpu during event's update Jiri Olsa
@ 2017-08-28 19:24 ` Peter Zijlstra
0 siblings, 0 replies; 24+ messages in thread
From: Peter Zijlstra @ 2017-08-28 19:24 UTC (permalink / raw)
To: Jiri Olsa
Cc: Arnaldo Carvalho de Melo, lkml, Ingo Molnar, Alexander Shishkin,
Namhyung Kim, David Ahern, Andi Kleen, Mark Rutland
On Thu, Aug 24, 2017 at 06:27:28PM +0200, Jiri Olsa wrote:
> 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.
>
> Reported-by: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Yeah, no objection to this one.
Ack
> ---
> arch/x86/events/core.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
> index af12e294caed..b8e394d9f7f2 100644
> --- a/arch/x86/events/core.c
> +++ b/arch/x86/events/core.c
> @@ -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;
>
> --
> 2.9.5
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 02/10] perf: Fix leader for removed sibling event in perf_group_detach
2017-08-24 16:27 [PATCH 00/10] perf: inherit_stat related fixes Jiri Olsa
2017-08-24 16:27 ` [PATCH 01/10] perf/x86: Add warning on proper cpu during event's update Jiri Olsa
@ 2017-08-24 16:27 ` Jiri Olsa
2017-08-28 19:23 ` Peter Zijlstra
2017-08-24 16:27 ` [PATCH 03/10] perf: Make sure we read only scheduled events Jiri Olsa
` (7 subsequent siblings)
9 siblings, 1 reply; 24+ messages in thread
From: Jiri Olsa @ 2017-08-24 16:27 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Peter Zijlstra
Cc: lkml, Ingo Molnar, Alexander Shishkin, Namhyung Kim, David Ahern,
Andi Kleen, Mark Rutland
Mark 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 <mark.rutland@arm.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
kernel/events/core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index d704e23914bf..30e30e94ea32 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -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;
}
--
2.9.5
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 02/10] perf: Fix leader for removed sibling event in perf_group_detach
2017-08-24 16:27 ` [PATCH 02/10] perf: Fix leader for removed sibling event in perf_group_detach Jiri Olsa
@ 2017-08-28 19:23 ` Peter Zijlstra
0 siblings, 0 replies; 24+ messages in thread
From: Peter Zijlstra @ 2017-08-28 19:23 UTC (permalink / raw)
To: Jiri Olsa
Cc: Arnaldo Carvalho de Melo, lkml, Ingo Molnar, Alexander Shishkin,
Namhyung Kim, David Ahern, Andi Kleen, Mark Rutland
On Thu, Aug 24, 2017 at 06:27:29PM +0200, Jiri Olsa wrote:
> Mark 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 <mark.rutland@arm.com>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> kernel/events/core.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index d704e23914bf..30e30e94ea32 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -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.
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 03/10] perf: Make sure we read only scheduled events
2017-08-24 16:27 [PATCH 00/10] perf: inherit_stat related fixes Jiri Olsa
2017-08-24 16:27 ` [PATCH 01/10] perf/x86: Add warning on proper cpu during event's update Jiri Olsa
2017-08-24 16:27 ` [PATCH 02/10] perf: Fix leader for removed sibling event in perf_group_detach Jiri Olsa
@ 2017-08-24 16:27 ` Jiri Olsa
2017-08-28 19:23 ` Peter Zijlstra
2017-08-24 16:27 ` [PATCH 04/10] perf record: Set read_format for inherit_stat Jiri Olsa
` (6 subsequent siblings)
9 siblings, 1 reply; 24+ messages in thread
From: Jiri Olsa @ 2017-08-24 16:27 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Peter Zijlstra
Cc: lkml, Ingo Molnar, Alexander Shishkin, Namhyung Kim, David Ahern,
Andi Kleen, Mark Rutland
Adding 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.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
kernel/events/core.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 30e30e94ea32..9a2791afe051 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -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);
--
2.9.5
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 03/10] perf: Make sure we read only scheduled events
2017-08-24 16:27 ` [PATCH 03/10] perf: Make sure we read only scheduled events Jiri Olsa
@ 2017-08-28 19:23 ` Peter Zijlstra
2017-09-01 7:21 ` Jiri Olsa
0 siblings, 1 reply; 24+ messages in thread
From: Peter Zijlstra @ 2017-08-28 19:23 UTC (permalink / raw)
To: Jiri Olsa
Cc: Arnaldo Carvalho de Melo, lkml, Ingo Molnar, Alexander Shishkin,
Namhyung Kim, David Ahern, Andi Kleen, Mark Rutland
On Thu, Aug 24, 2017 at 06:27:30PM +0200, Jiri Olsa wrote:
> Adding 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.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> kernel/events/core.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 30e30e94ea32..9a2791afe051 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -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);
I'm not seeing how this makes sense. Groups should either _all_ be
scheduled or not at all. Please explain.
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 03/10] perf: Make sure we read only scheduled events
2017-08-28 19:23 ` Peter Zijlstra
@ 2017-09-01 7:21 ` Jiri Olsa
0 siblings, 0 replies; 24+ messages in thread
From: Jiri Olsa @ 2017-09-01 7:21 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Jiri Olsa, Arnaldo Carvalho de Melo, lkml, Ingo Molnar,
Alexander Shishkin, Namhyung Kim, David Ahern, Andi Kleen,
Mark Rutland
On Mon, Aug 28, 2017 at 09:23:59PM +0200, Peter Zijlstra wrote:
> On Thu, Aug 24, 2017 at 06:27:30PM +0200, Jiri Olsa wrote:
> > Adding 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.
> >
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> > kernel/events/core.c | 10 +++++++---
> > 1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/kernel/events/core.c b/kernel/events/core.c
> > index 30e30e94ea32..9a2791afe051 100644
> > --- a/kernel/events/core.c
> > +++ b/kernel/events/core.c
> > @@ -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);
>
> 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
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 04/10] perf record: Set read_format for inherit_stat
2017-08-24 16:27 [PATCH 00/10] perf: inherit_stat related fixes Jiri Olsa
` (2 preceding siblings ...)
2017-08-24 16:27 ` [PATCH 03/10] perf: Make sure we read only scheduled events Jiri Olsa
@ 2017-08-24 16:27 ` Jiri Olsa
2017-08-29 21:20 ` [tip:perf/core] " tip-bot for Jiri Olsa
2017-08-24 16:27 ` [PATCH 05/10] perf report: Add dump_read function Jiri Olsa
` (5 subsequent siblings)
9 siblings, 1 reply; 24+ messages in thread
From: Jiri Olsa @ 2017-08-24 16:27 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Peter Zijlstra
Cc: lkml, Ingo Molnar, Alexander Shishkin, Namhyung Kim, David Ahern,
Andi Kleen, Mark Rutland
Set read_format for what we expect to get from read event
generated by perf_event_attr::inherit_stat.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/util/evsel.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index a5888c704e01..d9bd632ed7db 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -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);
--
2.9.5
^ permalink raw reply related [flat|nested] 24+ messages in thread* [tip:perf/core] perf record: Set read_format for inherit_stat
2017-08-24 16:27 ` [PATCH 04/10] perf record: Set read_format for inherit_stat Jiri Olsa
@ 2017-08-29 21:20 ` tip-bot for Jiri Olsa
0 siblings, 0 replies; 24+ messages in thread
From: tip-bot for Jiri Olsa @ 2017-08-29 21:20 UTC (permalink / raw)
To: linux-tip-commits
Cc: a.p.zijlstra, mark.rutland, dsahern, namhyung, andi, hpa, acme,
tglx, linux-kernel, mingo, alexander.shishkin, jolsa
Commit-ID: a17f06978769735ab5c7598c46881fa201e9b1a2
Gitweb: http://git.kernel.org/tip/a17f06978769735ab5c7598c46881fa201e9b1a2
Author: Jiri Olsa <jolsa@kernel.org>
AuthorDate: Thu, 24 Aug 2017 18:27:31 +0200
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
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.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/20170824162737.7813-5-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/evsel.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index a5888c7..d9bd632 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -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);
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 05/10] perf report: Add dump_read function
2017-08-24 16:27 [PATCH 00/10] perf: inherit_stat related fixes Jiri Olsa
` (3 preceding siblings ...)
2017-08-24 16:27 ` [PATCH 04/10] perf record: Set read_format for inherit_stat Jiri Olsa
@ 2017-08-24 16:27 ` Jiri Olsa
2017-08-29 21:20 ` [tip:perf/core] " tip-bot for Jiri Olsa
2017-08-24 16:27 ` [PATCH 06/10] perf values: Fix thread index bug Jiri Olsa
` (4 subsequent siblings)
9 siblings, 1 reply; 24+ messages in thread
From: Jiri Olsa @ 2017-08-24 16:27 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Peter Zijlstra
Cc: lkml, Ingo Molnar, Alexander Shishkin, Namhyung Kim, David Ahern,
Andi Kleen, Mark Rutland
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
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/builtin-report.c | 4 ----
tools/perf/util/session.c | 25 +++++++++++++++++++++++++
2 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index bace3429c030..9e4004b08f55 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -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;
}
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index dc453f84a14c..4a09604a66a0 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -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);
--
2.9.5
^ permalink raw reply related [flat|nested] 24+ messages in thread* [tip:perf/core] perf report: Add dump_read function
2017-08-24 16:27 ` [PATCH 05/10] perf report: Add dump_read function Jiri Olsa
@ 2017-08-29 21:20 ` tip-bot for Jiri Olsa
0 siblings, 0 replies; 24+ messages in thread
From: tip-bot for Jiri Olsa @ 2017-08-29 21:20 UTC (permalink / raw)
To: linux-tip-commits
Cc: namhyung, a.p.zijlstra, alexander.shishkin, andi, mingo,
linux-kernel, hpa, acme, mark.rutland, dsahern, jolsa, tglx
Commit-ID: dac7f6b7ed1c8601358357f60e9764a4c6a68d71
Gitweb: http://git.kernel.org/tip/dac7f6b7ed1c8601358357f60e9764a4c6a68d71
Author: Jiri Olsa <jolsa@kernel.org>
AuthorDate: Thu, 24 Aug 2017 18:27:32 +0200
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
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
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/20170824162737.7813-6-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/builtin-report.c | 4 ----
tools/perf/util/session.c | 25 +++++++++++++++++++++++++
2 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index bace342..9e4004b0 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -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;
}
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index dc453f8..ac86369 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -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);
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 06/10] perf values: Fix thread index bug
2017-08-24 16:27 [PATCH 00/10] perf: inherit_stat related fixes Jiri Olsa
` (4 preceding siblings ...)
2017-08-24 16:27 ` [PATCH 05/10] perf report: Add dump_read function Jiri Olsa
@ 2017-08-24 16:27 ` Jiri Olsa
2017-08-29 21:21 ` [tip:perf/core] " tip-bot for Jiri Olsa
2017-08-24 16:27 ` [PATCH 07/10] perf values: Fix allocation check Jiri Olsa
` (3 subsequent siblings)
9 siblings, 1 reply; 24+ messages in thread
From: Jiri Olsa @ 2017-08-24 16:27 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Peter Zijlstra
Cc: lkml, Ingo Molnar, Alexander Shishkin, Namhyung Kim, David Ahern,
Andi Kleen, Mark Rutland
We are taking wrong index (+1) for first thread, which
leaves thread with index 0 unused and uninitialized.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/util/values.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/values.c b/tools/perf/util/values.c
index 5de2e15e2eda..9ac36bf2c438 100644
--- a/tools/perf/util/values.c
+++ b/tools/perf/util/values.c
@@ -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;
}
--
2.9.5
^ permalink raw reply related [flat|nested] 24+ messages in thread* [tip:perf/core] perf values: Fix thread index bug
2017-08-24 16:27 ` [PATCH 06/10] perf values: Fix thread index bug Jiri Olsa
@ 2017-08-29 21:21 ` tip-bot for Jiri Olsa
0 siblings, 0 replies; 24+ messages in thread
From: tip-bot for Jiri Olsa @ 2017-08-29 21:21 UTC (permalink / raw)
To: linux-tip-commits
Cc: tglx, mingo, jolsa, andi, alexander.shishkin, linux-kernel,
a.p.zijlstra, namhyung, hpa, mark.rutland, acme, dsahern
Commit-ID: 64eed1deb6d87f4c0efe03297f50367a3689eb56
Gitweb: http://git.kernel.org/tip/64eed1deb6d87f4c0efe03297f50367a3689eb56
Author: Jiri Olsa <jolsa@kernel.org>
AuthorDate: Thu, 24 Aug 2017 18:27:33 +0200
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
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.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/20170824162737.7813-7-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/values.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/values.c b/tools/perf/util/values.c
index 5de2e15..9ac36bf 100644
--- a/tools/perf/util/values.c
+++ b/tools/perf/util/values.c
@@ -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;
}
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 07/10] perf values: Fix allocation check
2017-08-24 16:27 [PATCH 00/10] perf: inherit_stat related fixes Jiri Olsa
` (5 preceding siblings ...)
2017-08-24 16:27 ` [PATCH 06/10] perf values: Fix thread index bug Jiri Olsa
@ 2017-08-24 16:27 ` Jiri Olsa
2017-08-29 21:21 ` [tip:perf/core] " tip-bot for Jiri Olsa
2017-08-24 16:27 ` [PATCH 08/10] perf values: Zero value buffers Jiri Olsa
` (2 subsequent siblings)
9 siblings, 1 reply; 24+ messages in thread
From: Jiri Olsa @ 2017-08-24 16:27 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Peter Zijlstra
Cc: lkml, Ingo Molnar, Alexander Shishkin, Namhyung Kim, David Ahern,
Andi Kleen, Mark Rutland
Bailing out in case the allocation failed,
not the other way round.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/util/values.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/util/values.c b/tools/perf/util/values.c
index 9ac36bf2c438..2c4af02f08cd 100644
--- a/tools/perf/util/values.c
+++ b/tools/perf/util/values.c
@@ -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;
}
--
2.9.5
^ permalink raw reply related [flat|nested] 24+ messages in thread* [tip:perf/core] perf values: Fix allocation check
2017-08-24 16:27 ` [PATCH 07/10] perf values: Fix allocation check Jiri Olsa
@ 2017-08-29 21:21 ` tip-bot for Jiri Olsa
0 siblings, 0 replies; 24+ messages in thread
From: tip-bot for Jiri Olsa @ 2017-08-29 21:21 UTC (permalink / raw)
To: linux-tip-commits
Cc: a.p.zijlstra, andi, alexander.shishkin, dsahern, mark.rutland,
linux-kernel, hpa, jolsa, tglx, namhyung, mingo, acme
Commit-ID: f4ef3b7c184c4c269f953f226f7158347d007622
Gitweb: http://git.kernel.org/tip/f4ef3b7c184c4c269f953f226f7158347d007622
Author: Jiri Olsa <jolsa@kernel.org>
AuthorDate: Thu, 24 Aug 2017 18:27:34 +0200
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
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.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/20170824162737.7813-8-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/values.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/util/values.c b/tools/perf/util/values.c
index 9ac36bf..2c4af02 100644
--- a/tools/perf/util/values.c
+++ b/tools/perf/util/values.c
@@ -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;
}
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 08/10] perf values: Zero value buffers
2017-08-24 16:27 [PATCH 00/10] perf: inherit_stat related fixes Jiri Olsa
` (6 preceding siblings ...)
2017-08-24 16:27 ` [PATCH 07/10] perf values: Fix allocation check Jiri Olsa
@ 2017-08-24 16:27 ` Jiri Olsa
2017-08-29 21:21 ` [tip:perf/core] " tip-bot for Jiri Olsa
2017-08-24 16:27 ` [PATCH 09/10] perf report: Group stat values on global event id Jiri Olsa
2017-08-24 16:27 ` [PATCH 10/10] perf stat: Support inherit/no-inherit terms Jiri Olsa
9 siblings, 1 reply; 24+ messages in thread
From: Jiri Olsa @ 2017-08-24 16:27 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Peter Zijlstra
Cc: lkml, Ingo Molnar, Alexander Shishkin, Namhyung Kim, David Ahern,
Andi Kleen, Mark Rutland
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.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/util/values.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/values.c b/tools/perf/util/values.c
index 2c4af02f08cd..3b56aeaa8cbb 100644
--- a/tools/perf/util/values.c
+++ b/tools/perf/util/values.c
@@ -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;
}
--
2.9.5
^ permalink raw reply related [flat|nested] 24+ messages in thread* [tip:perf/core] perf values: Zero value buffers
2017-08-24 16:27 ` [PATCH 08/10] perf values: Zero value buffers Jiri Olsa
@ 2017-08-29 21:21 ` tip-bot for Jiri Olsa
0 siblings, 0 replies; 24+ messages in thread
From: tip-bot for Jiri Olsa @ 2017-08-29 21:21 UTC (permalink / raw)
To: linux-tip-commits
Cc: mark.rutland, mingo, a.p.zijlstra, andi, tglx, linux-kernel,
jolsa, dsahern, alexander.shishkin, namhyung, hpa, acme
Commit-ID: a1834fc938344dd3015a1df64ee7f2af70ded147
Gitweb: http://git.kernel.org/tip/a1834fc938344dd3015a1df64ee7f2af70ded147
Author: Jiri Olsa <jolsa@kernel.org>
AuthorDate: Thu, 24 Aug 2017 18:27:35 +0200
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
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.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/20170824162737.7813-9-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/values.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/values.c b/tools/perf/util/values.c
index 2c4af02..3b56aea 100644
--- a/tools/perf/util/values.c
+++ b/tools/perf/util/values.c
@@ -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;
}
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 09/10] perf report: Group stat values on global event id
2017-08-24 16:27 [PATCH 00/10] perf: inherit_stat related fixes Jiri Olsa
` (7 preceding siblings ...)
2017-08-24 16:27 ` [PATCH 08/10] perf values: Zero value buffers Jiri Olsa
@ 2017-08-24 16:27 ` Jiri Olsa
2017-08-29 21:22 ` [tip:perf/core] " tip-bot for Jiri Olsa
2017-08-24 16:27 ` [PATCH 10/10] perf stat: Support inherit/no-inherit terms Jiri Olsa
9 siblings, 1 reply; 24+ messages in thread
From: Jiri Olsa @ 2017-08-24 16:27 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Peter Zijlstra
Cc: lkml, Ingo Molnar, Alexander Shishkin, Namhyung Kim, David Ahern,
Andi Kleen, Mark Rutland
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 0
Now:
# PID TID cycles:u
3339 3339 19678
3340 3340 18744
3341 3341 17335
3342 3342 26414
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/builtin-report.c | 2 +-
tools/perf/util/values.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 9e4004b08f55..f9dff652dcbd 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -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);
diff --git a/tools/perf/util/values.c b/tools/perf/util/values.c
index 3b56aeaa8cbb..8a32bb0095e5 100644
--- a/tools/perf/util/values.c
+++ b/tools/perf/util/values.c
@@ -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;
}
--
2.9.5
^ permalink raw reply related [flat|nested] 24+ messages in thread* [tip:perf/core] perf report: Group stat values on global event id
2017-08-24 16:27 ` [PATCH 09/10] perf report: Group stat values on global event id Jiri Olsa
@ 2017-08-29 21:22 ` tip-bot for Jiri Olsa
0 siblings, 0 replies; 24+ messages in thread
From: tip-bot for Jiri Olsa @ 2017-08-29 21:22 UTC (permalink / raw)
To: linux-tip-commits
Cc: jolsa, hpa, a.p.zijlstra, mingo, mark.rutland, andi,
linux-kernel, acme, namhyung, tglx, alexander.shishkin, dsahern
Commit-ID: 9933183e365f7dd3a79507f1ffb4bcf9433a73ee
Gitweb: http://git.kernel.org/tip/9933183e365f7dd3a79507f1ffb4bcf9433a73ee
Author: Jiri Olsa <jolsa@kernel.org>
AuthorDate: Thu, 24 Aug 2017 18:27:36 +0200
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
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 0
Now:
# PID TID cycles:u
3339 3339 19678
3340 3340 18744
3341 3341 17335
3342 3342 26414
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/20170824162737.7813-10-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/builtin-report.c | 2 +-
tools/perf/util/values.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 9e4004b0..f9dff65 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -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);
diff --git a/tools/perf/util/values.c b/tools/perf/util/values.c
index 3b56aea..8a32bb0 100644
--- a/tools/perf/util/values.c
+++ b/tools/perf/util/values.c
@@ -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;
}
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 10/10] perf stat: Support inherit/no-inherit terms
2017-08-24 16:27 [PATCH 00/10] perf: inherit_stat related fixes Jiri Olsa
` (8 preceding siblings ...)
2017-08-24 16:27 ` [PATCH 09/10] perf report: Group stat values on global event id Jiri Olsa
@ 2017-08-24 16:27 ` Jiri Olsa
2017-08-24 16:30 ` Andi Kleen
9 siblings, 1 reply; 24+ messages in thread
From: Jiri Olsa @ 2017-08-24 16:27 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Peter Zijlstra
Cc: lkml, Ingo Molnar, Alexander Shishkin, Namhyung Kim, David Ahern,
Andi Kleen, Mark Rutland
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 ...
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/builtin-stat.c | 19 +++++++++++++++++++
tools/perf/util/parse-events.c | 2 ++
2 files changed, 21 insertions(+)
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 866da7aa54bf..92053b93e452 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -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));
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index f44aeba51d1f..b2b448ff0a36 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -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)
--
2.9.5
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 10/10] perf stat: Support inherit/no-inherit terms
2017-08-24 16:27 ` [PATCH 10/10] perf stat: Support inherit/no-inherit terms Jiri Olsa
@ 2017-08-24 16:30 ` Andi Kleen
2017-08-24 16:45 ` Jiri Olsa
0 siblings, 1 reply; 24+ messages in thread
From: Andi Kleen @ 2017-08-24 16:30 UTC (permalink / raw)
To: Jiri Olsa
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, lkml, Ingo Molnar,
Alexander Shishkin, Namhyung Kim, David Ahern, Andi Kleen,
Mark Rutland
On Thu, Aug 24, 2017 at 06:27:37PM +0200, Jiri Olsa wrote:
> 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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 10/10] perf stat: Support inherit/no-inherit terms
2017-08-24 16:30 ` Andi Kleen
@ 2017-08-24 16:45 ` Jiri Olsa
2017-08-25 18:57 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 24+ messages in thread
From: Jiri Olsa @ 2017-08-24 16:45 UTC (permalink / raw)
To: Andi Kleen
Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Peter Zijlstra, lkml,
Ingo Molnar, Alexander Shishkin, Namhyung Kim, David Ahern,
Mark Rutland
On Thu, Aug 24, 2017 at 09:30:34AM -0700, Andi Kleen wrote:
> On Thu, Aug 24, 2017 at 06:27:37PM +0200, Jiri Olsa wrote:
> > 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.
right, will add
thanks,
jirka
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 10/10] perf stat: Support inherit/no-inherit terms
2017-08-24 16:45 ` Jiri Olsa
@ 2017-08-25 18:57 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 24+ messages in thread
From: Arnaldo Carvalho de Melo @ 2017-08-25 18:57 UTC (permalink / raw)
To: Jiri Olsa
Cc: Andi Kleen, Jiri Olsa, Peter Zijlstra, lkml, Ingo Molnar,
Alexander Shishkin, Namhyung Kim, David Ahern, Mark Rutland
Em Thu, Aug 24, 2017 at 06:45:27PM +0200, Jiri Olsa escreveu:
> On Thu, Aug 24, 2017 at 09:30:34AM -0700, Andi Kleen wrote:
> > On Thu, Aug 24, 2017 at 06:27:37PM +0200, Jiri Olsa wrote:
> > > 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.
>
> right, will add
Ok, applied 4-9, Peter, are you ok with the kernel parts?
- Arnaldo
^ permalink raw reply [flat|nested] 24+ messages in thread