[PATCH AUTOSEL for 3.18 003/101] perf/core: Correct event creation with PERF_FORMAT_GROUP

[Sasha Levin]

From: Peter Zijlstra 

[ 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 
Signed-off-by: Peter Zijlstra (Intel) 
Cc: Alexander Shishkin 
Cc: Arnaldo Carvalho de Melo 
Cc: Jiri Olsa 
Cc: Linus Torvalds 
Cc: Peter Zijlstra 
Cc: Stephane Eranian 
Cc: Thomas Gleixner 
Cc: Vince Weaver 
Fixes:  3dab77fb1bf8 ("perf: Rework/fix the whole read vs group stuff")
Link: 
http://lkml.kernel.org/r/20170530094512.dy2nljns2uq7q...@hirez.programming.kicks-ass.net
Signed-off-by: Ingo Molnar 
Signed-off-by: Sasha Levin 
---
 kernel/events/core.c | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index de3303aab7d6..9eb7710914fb 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -4861,9 +4861,6 @@ static void perf_output_read_one(struct 
perf_output_handle *handle,
__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)
@@ -4908,6 +4905,13 @@ static void perf_output_read_group(struct 
perf_output_handle *handle,
 #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)
 {
@@ -7194,9 +7198,10 @@ perf_event_alloc(struct perf_event_attr *attr, int cpu,
local64_set(>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;
 
pmu = perf_init_event(event);
-- 
2.15.1

[PATCH AUTOSEL for 3.18 003/101] perf/core: Correct event creation with PERF_FORMAT_GROUP

[Sasha Levin]

From: Peter Zijlstra 

[ 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 
Signed-off-by: Peter Zijlstra (Intel) 
Cc: Alexander Shishkin 
Cc: Arnaldo Carvalho de Melo 
Cc: Jiri Olsa 
Cc: Linus Torvalds 
Cc: Peter Zijlstra 
Cc: Stephane Eranian 
Cc: Thomas Gleixner 
Cc: Vince Weaver 
Fixes:  3dab77fb1bf8 ("perf: Rework/fix the whole read vs group stuff")
Link: 
http://lkml.kernel.org/r/20170530094512.dy2nljns2uq7q...@hirez.programming.kicks-ass.net
Signed-off-by: Ingo Molnar 
Signed-off-by: Sasha Levin 
---
 kernel/events/core.c | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index de3303aab7d6..9eb7710914fb 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -4861,9 +4861,6 @@ static void perf_output_read_one(struct 
perf_output_handle *handle,
__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)
@@ -4908,6 +4905,13 @@ static void perf_output_read_group(struct 
perf_output_handle *handle,
 #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)
 {
@@ -7194,9 +7198,10 @@ perf_event_alloc(struct perf_event_attr *attr, int cpu,
local64_set(>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;
 
pmu = perf_init_event(event);
-- 
2.15.1