Skip to content

Commit

Permalink
perf/core: Fix exclusive events' grouping
Browse files Browse the repository at this point in the history
So far, we tried to disallow grouping exclusive events for the fear of
complications they would cause with moving between contexts. Specifically,
moving a software group to a hardware context would violate the exclusivity
rules if both groups contain matching exclusive events.

This attempt was, however, unsuccessful: the check that we have in the
perf_event_open() syscall is both wrong (looks at wrong PMU) and
insufficient (group leader may still be exclusive), as can be illustrated
by running:

  $ perf record -e '{intel_pt//,cycles}' uname
  $ perf record -e '{cycles,intel_pt//}' uname

ultimately successfully.

Furthermore, we are completely free to trigger the exclusivity violation
by:

   perf -e '{cycles,intel_pt//}' -e '{intel_pt//,instructions}'

even though the helpful perf record will not allow that, the ABI will.

The warning later in the perf_event_open() path will also not trigger, because
it's also wrong.

Fix all this by validating the original group before moving, getting rid
of broken safeguards and placing a useful one to perf_install_in_context().

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: <stable@vger.kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
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 <vincent.weaver@maine.edu>
Cc: mathieu.poirier@linaro.org
Cc: will.deacon@arm.com
Fixes: bed5b25 ("perf: Add a pmu capability for "exclusive" events")
Link: https://lkml.kernel.org/r/20190701110755.24646-1-alexander.shishkin@linux.intel.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
  • Loading branch information
Alexander Shishkin authored and Ingo Molnar committed Jul 13, 2019
1 parent 2f217d5 commit 8a58dda
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 12 deletions.
5 changes: 5 additions & 0 deletions include/linux/perf_event.h
Original file line number Diff line number Diff line change
Expand Up @@ -1055,6 +1055,11 @@ static inline int in_software_context(struct perf_event *event)
return event->ctx->pmu->task_ctx_nr == perf_sw_context;
}

static inline int is_exclusive_pmu(struct pmu *pmu)
{
return pmu->capabilities & PERF_PMU_CAP_EXCLUSIVE;
}

extern struct static_key perf_swevent_enabled[PERF_COUNT_SW_MAX];

extern void ___perf_sw_event(u32, u64, struct pt_regs *, u64);
Expand Down
34 changes: 22 additions & 12 deletions kernel/events/core.c
Original file line number Diff line number Diff line change
Expand Up @@ -2553,6 +2553,9 @@ static int __perf_install_in_context(void *info)
return ret;
}

static bool exclusive_event_installable(struct perf_event *event,
struct perf_event_context *ctx);

/*
* Attach a performance event to a context.
*
Expand All @@ -2567,6 +2570,8 @@ perf_install_in_context(struct perf_event_context *ctx,

lockdep_assert_held(&ctx->mutex);

WARN_ON_ONCE(!exclusive_event_installable(event, ctx));

if (event->cpu != -1)
event->cpu = cpu;

Expand Down Expand Up @@ -4360,7 +4365,7 @@ static int exclusive_event_init(struct perf_event *event)
{
struct pmu *pmu = event->pmu;

if (!(pmu->capabilities & PERF_PMU_CAP_EXCLUSIVE))
if (!is_exclusive_pmu(pmu))
return 0;

/*
Expand Down Expand Up @@ -4391,7 +4396,7 @@ static void exclusive_event_destroy(struct perf_event *event)
{
struct pmu *pmu = event->pmu;

if (!(pmu->capabilities & PERF_PMU_CAP_EXCLUSIVE))
if (!is_exclusive_pmu(pmu))
return;

/* see comment in exclusive_event_init() */
Expand All @@ -4411,14 +4416,15 @@ static bool exclusive_event_match(struct perf_event *e1, struct perf_event *e2)
return false;
}

/* Called under the same ctx::mutex as perf_install_in_context() */
static bool exclusive_event_installable(struct perf_event *event,
struct perf_event_context *ctx)
{
struct perf_event *iter_event;
struct pmu *pmu = event->pmu;

if (!(pmu->capabilities & PERF_PMU_CAP_EXCLUSIVE))
lockdep_assert_held(&ctx->mutex);

if (!is_exclusive_pmu(pmu))
return true;

list_for_each_entry(iter_event, &ctx->event_list, event_entry) {
Expand Down Expand Up @@ -10947,11 +10953,6 @@ SYSCALL_DEFINE5(perf_event_open,
goto err_alloc;
}

if ((pmu->capabilities & PERF_PMU_CAP_EXCLUSIVE) && group_leader) {
err = -EBUSY;
goto err_context;
}

/*
* Look up the group leader (we will attach this event to it):
*/
Expand Down Expand Up @@ -11039,6 +11040,18 @@ SYSCALL_DEFINE5(perf_event_open,
move_group = 0;
}
}

/*
* Failure to create exclusive events returns -EBUSY.
*/
err = -EBUSY;
if (!exclusive_event_installable(group_leader, ctx))
goto err_locked;

for_each_sibling_event(sibling, group_leader) {
if (!exclusive_event_installable(sibling, ctx))
goto err_locked;
}
} else {
mutex_lock(&ctx->mutex);
}
Expand Down Expand Up @@ -11075,9 +11088,6 @@ SYSCALL_DEFINE5(perf_event_open,
* because we need to serialize with concurrent event creation.
*/
if (!exclusive_event_installable(event, ctx)) {
/* exclusive and group stuff are assumed mutually exclusive */
WARN_ON_ONCE(move_group);

err = -EBUSY;
goto err_locked;
}
Expand Down

0 comments on commit 8a58dda

Please sign in to comment.