Re: [PATCH v2 0/4] perf: Support PERF_SAMPLE_READ with inherit_stat

[Ben Gainey]

On Thu, 2024-02-08 at 13:10 +0000, Ben Gainey wrote:
> This change allows events to use PERF_SAMPLE READ with inherit so
> long
> as both inherit_stat and PERF_SAMPLE_TID are set.
>
> Currently it is not possible to use PERF_SAMPLE_READ with inherit.
> This
> restriction assumes the user is interested in collecting aggregate
> statistics as per `perf stat`. It prevents a user from collecting
> per-thread samples using counter groups from a multi-threaded or
> multi-process application, as with `perf record -e '{....}:S'`.
> Instead
> users must use system-wide mode, or forgo the ability to sample
> counter
> groups. System-wide mode is often problematic as it requires specific
> permissions (no CAP_PERFMON / root access), or may lead to capture of
> significant amounts of extra data from other processes running on the
> system.
>
> Perf already supports the ability to collect per-thread counts with
> `inherit` via the `inherit_stat` flag. This patch changes
> `perf_event_alloc` relaxing the restriction to combine `inherit` with
> `PERF_SAMPLE_READ` so that the combination will be allowed so long as
> `inherit_stat` and `PERF_SAMPLE_TID` are enabled.
>
> In this configuration stream ids (such as may appear in the
> read_format
> field of a PERF_RECORD_SAMPLE) are no longer globally unique, rather
> the pair of (stream id, tid) uniquely identify each event. Tools that
> rely on this, for example to calculate a delta between samples, would
> need updating to take this into account. Previously valid event
> configurations (system-wide, no-inherit and so on) where each stream
> id
> is the identifier are unaffected.
>
>
> Changes since v1:
>  - Rebase on v6.8-rc1
>  - Fixed value written into sample after child exists.
>  - Modified handling of switch-out so that context with these events
> take the
>    slow path, so that the per-event/per-thread PMU state is correctly
> switched.
>  - Modified perf tools to support this mode of operation.
>
>
> Ben Gainey (4):
>   perf: Support PERF_SAMPLE_READ with inherit_stat
>   tools/perf: Track where perf_sample_ids need per-thread periods
>   tools/perf: Correctly calculate sample period for inherited
>     SAMPLE_READ values
>   tools/perf: Allow inherit + inherit_stat + PERF_SAMPLE_READ when
>     opening events
>
>  include/linux/perf_event.h              |  1 +
>  kernel/events/core.c                    | 53 +++++++++++++++++------
> --
>  tools/lib/perf/evlist.c                 |  1 +
>  tools/lib/perf/evsel.c                  | 48 ++++++++++++++++++++++
>  tools/lib/perf/include/internal/evsel.h | 48 +++++++++++++++++++++-
>  tools/perf/util/evsel.c                 | 15 ++++++-
>  tools/perf/util/evsel.h                 |  1 +
>  tools/perf/util/session.c               | 11 +++--
>  8 files changed, 154 insertions(+), 24 deletions(-)
>



Hello all, appreciate everyone is busy. Is there any feedback on these?
I expect they will need rebasing, but before I do that, are you happy
with the approach?

Cheers
Ben
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.