| From foo@baz Mon Apr 9 17:09:24 CEST 2018 |
| From: Peter Zijlstra <peterz@infradead.org> |
| Date: Tue, 30 May 2017 11:45:12 +0200 |
| Subject: perf/core: Correct event creation with PERF_FORMAT_GROUP |
| |
| From: Peter Zijlstra <peterz@infradead.org> |
| |
| |
| [ Upstream commit ba5213ae6b88fb170c4771fef6553f759c7d8cdd ] |
| |
| Andi was asking about PERF_FORMAT_GROUP vs inherited events, which led |
| to the discovery of a bug from commit: |
| |
| 3dab77fb1bf8 ("perf: Rework/fix the whole read vs group stuff") |
| |
| - PERF_SAMPLE_GROUP = 1U << 4, |
| + PERF_SAMPLE_READ = 1U << 4, |
| |
| - if (attr->inherit && (attr->sample_type & PERF_SAMPLE_GROUP)) |
| + if (attr->inherit && (attr->read_format & PERF_FORMAT_GROUP)) |
| |
| is a clear fail :/ |
| |
| While this changes user visible behaviour; it was previously possible |
| to create an inherited event with PERF_SAMPLE_READ; this is deemed |
| acceptible because its results were always incorrect. |
| |
| Reported-by: Andi Kleen <ak@linux.intel.com> |
| Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> |
| Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> |
| Cc: Arnaldo Carvalho de Melo <acme@kernel.org> |
| Cc: Jiri Olsa <jolsa@kernel.org> |
| Cc: Linus Torvalds <torvalds@linux-foundation.org> |
| Cc: Peter Zijlstra <peterz@infradead.org> |
| Cc: Stephane Eranian <eranian@google.com> |
| Cc: Thomas Gleixner <tglx@linutronix.de> |
| Cc: Vince Weaver <vince@deater.net> |
| Fixes: 3dab77fb1bf8 ("perf: Rework/fix the whole read vs group stuff") |
| Link: http://lkml.kernel.org/r/20170530094512.dy2nljns2uq7qa3j@hirez.programming.kicks-ass.net |
| Signed-off-by: Ingo Molnar <mingo@kernel.org> |
| Signed-off-by: Sasha Levin <alexander.levin@microsoft.com> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| kernel/events/core.c | 15 ++++++++++----- |
| 1 file changed, 10 insertions(+), 5 deletions(-) |
| |
| --- a/kernel/events/core.c |
| +++ b/kernel/events/core.c |
| @@ -5669,9 +5669,6 @@ static void perf_output_read_one(struct |
| __output_copy(handle, values, n * sizeof(u64)); |
| } |
| |
| -/* |
| - * XXX PERF_FORMAT_GROUP vs inherited events seems difficult. |
| - */ |
| static void perf_output_read_group(struct perf_output_handle *handle, |
| struct perf_event *event, |
| u64 enabled, u64 running) |
| @@ -5716,6 +5713,13 @@ static void perf_output_read_group(struc |
| #define PERF_FORMAT_TOTAL_TIMES (PERF_FORMAT_TOTAL_TIME_ENABLED|\ |
| PERF_FORMAT_TOTAL_TIME_RUNNING) |
| |
| +/* |
| + * XXX PERF_SAMPLE_READ vs inherited events seems difficult. |
| + * |
| + * The problem is that its both hard and excessively expensive to iterate the |
| + * child list, not to mention that its impossible to IPI the children running |
| + * on another CPU, from interrupt/NMI context. |
| + */ |
| static void perf_output_read(struct perf_output_handle *handle, |
| struct perf_event *event) |
| { |
| @@ -9259,9 +9263,10 @@ perf_event_alloc(struct perf_event_attr |
| local64_set(&hwc->period_left, hwc->sample_period); |
| |
| /* |
| - * we currently do not support PERF_FORMAT_GROUP on inherited events |
| + * We currently do not support PERF_SAMPLE_READ on inherited events. |
| + * See perf_output_read(). |
| */ |
| - if (attr->inherit && (attr->read_format & PERF_FORMAT_GROUP)) |
| + if (attr->inherit && (attr->sample_type & PERF_SAMPLE_READ)) |
| goto err_ns; |
| |
| if (!has_branch_stack(event)) |