LKML Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 00/10] perf: inherit_stat related fixes
@ 2017-08-24 16:27 Jiri Olsa
  2017-08-24 16:27 ` [PATCH 01/10] perf/x86: Add warning on proper cpu during event's update Jiri Olsa
                   ` (9 more replies)
  0 siblings, 10 replies; 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

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(-)

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [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	[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	[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	[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	[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	[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	[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	[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	[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	[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	[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

* 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

* 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 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

* [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	[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	[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	[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	[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	[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	[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

end of thread, back to index

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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-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
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
2017-08-28 19:23   ` Peter Zijlstra
2017-09-01  7:21     ` Jiri Olsa
2017-08-24 16:27 ` [PATCH 04/10] perf record: Set read_format for inherit_stat 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
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
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
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
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-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
2017-08-24 16:30   ` Andi Kleen
2017-08-24 16:45     ` Jiri Olsa
2017-08-25 18:57       ` Arnaldo Carvalho de Melo

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git
	git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git