Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit ba5213ae authored by Peter Zijlstra's avatar Peter Zijlstra Committed by Ingo Molnar
Browse files

perf/core: Correct event creation with PERF_FORMAT_GROUP 



Andi was asking about PERF_FORMAT_GROUP vs inherited events, which led
to the discovery of a bug from commit:

  3dab77fb ("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: default avatarAndi Kleen <ak@linux.intel.com>
Signed-off-by: default avatarPeter 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:  3dab77fb ("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: default avatarIngo Molnar <mingo@kernel.org>
parent a5506c46

kernel/events/core.c

+10 −5
Original line number Diff line number Diff line
@@ -5729,9 +5729,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)
@@ -5776,6 +5773,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)

{
@@ -9462,9 +9466,10 @@ perf_event_alloc(struct perf_event_attr *attr, int cpu, 
	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))