lkml.org 
[lkml]   [2019]   [Jul]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH v2] perf: Fix exclusive events' grouping
    Date
    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 -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>
    Fixes: bed5b25ad9c8a ("perf: Add a pmu capability for "exclusive" events")
    ---
    include/linux/perf_event.h | 5 +++++
    kernel/events/core.c | 34 ++++++++++++++++++++++------------
    2 files changed, 27 insertions(+), 12 deletions(-)

    diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
    index 2ddae518dce6..201cc93cec32 100644
    --- a/include/linux/perf_event.h
    +++ b/include/linux/perf_event.h
    @@ -1054,6 +1054,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);
    diff --git a/kernel/events/core.c b/kernel/events/core.c
    index 23efe6792abc..8cfb721bb284 100644
    --- a/kernel/events/core.c
    +++ b/kernel/events/core.c
    @@ -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.
    *
    @@ -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;

    @@ -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;

    /*
    @@ -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() */
    @@ -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) {
    @@ -10917,11 +10923,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):
    */
    @@ -11009,6 +11010,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);
    }
    @@ -11045,9 +11058,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;
    }
    --
    2.20.1
    \
     
     \ /
      Last update: 2019-07-01 13:08    [W:3.830 / U:0.436 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site