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

Ben Gainey posted 4 patches 2 years ago
There is a newer version of this series
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(-)
[PATCH v2 0/4] perf: Support PERF_SAMPLE_READ with inherit_stat
Posted by Ben Gainey 2 years ago
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(-)

-- 
2.43.0
Re: [PATCH v2 0/4] perf: Support PERF_SAMPLE_READ with inherit_stat
Posted by Ben Gainey 1 year, 11 months ago
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.