Re: [PATCH v6 4/4] tools/perf: Allow inherit + PERF_SAMPLE_READ when opening events

[Namhyung Kim]

Hello,

On Wed, May 29, 2024 at 10:48 AM Ben Gainey <Ben.Gainey@xxxxxxx> wrote:
>
> On Mon, 2024-05-27 at 18:47 +0100, Ben Gainey wrote:
> > On Thu, 2024-05-23 at 18:41 -0700, Namhyung Kim wrote:
> > > On Tue, May 21, 2024 at 6:30 AM Ben Gainey <ben.gainey@xxxxxxx>
> > > wrote:
> > > >
> > > > The tool will now default to this new mode if the user specifies
> > > > a
> > > > sampling group when not in system-wide mode, and when --no-
> > > > inherit
> > > > is not specified.
> > > >
> > > > This change updates evsel to allow the combination of inherit
> > > > and PERF_SAMPLE_READ.
> > > >
> > > > A fallback is implemented for kernel versions where this feature
> > > > is
> > > > not
> > > > supported.
> > >
> > > But I'm afraid the test would fail on old kernels.  Maybe we need
> > > to
> > > put it in the selftests.
> > >
> >
> > Sorry, not clear what you mean?
> >
> > Is the issue that the fallback on older kernels fails, or that the
> > "perf test" tests fail?
> >
> > Thanks
> > Ben
>
> Just to follow up, I've rechecked the fallback on an unmodified 6.9.1
> kernel with the following:
>
>     perf record -vv -e '{cycles,instructions}:S' ls
>
> With an unpatched version of perf running on an unpatched kernel, the
> cycles event is opened as:
>
>     ------------------------------------------------------------
>     perf_event_attr:
>       type                             0 (PERF_TYPE_HARDWARE)
>       size                             136
>       config                           0 (PERF_COUNT_HW_CPU_CYCLES)
>       { sample_period, sample_freq }   4000
>       sample_type                      IP|TID|TIME|READ|ID|PERIOD
>       read_format                      ID|GROUP|LOST
>       disabled                         1
>       exclude_kernel                   1
>       exclude_hv                       1
>       mmap                             1
>       comm                             1
>       freq                             1
>       enable_on_exec                   1
>       task                             1
>       sample_id_all                    1
>       exclude_guest                    1
>       mmap2                            1
>       comm_exec                        1
>       ksymbol                          1
>       bpf_event                        1
>     ------------------------------------------------------------
>
> whereas with these patches applied to perf, on an unpatched kernel, the
> output is as follows
>
>     ------------------------------------------------------------
>     perf_event_attr:
>       type                             0 (PERF_TYPE_HARDWARE)
>       size                             136
>       config                           0 (PERF_COUNT_HW_CPU_CYCLES)
>       { sample_period, sample_freq }   4000
>       sample_type                      IP|TID|TIME|READ|ID|PERIOD
>       read_format                      ID|GROUP|LOST
>       disabled                         1
>       inherit                          1
>       exclude_kernel                   1
>       exclude_hv                       1
>       mmap                             1
>       comm                             1
>       freq                             1
>       enable_on_exec                   1
>       task                             1
>       sample_id_all                    1
>       exclude_guest                    1
>       mmap2                            1
>       comm_exec                        1
>       ksymbol                          1
>       bpf_event                        1
>     ------------------------------------------------------------
>     sys_perf_event_open: pid 3442954  cpu 0  group_fd -1  flags 0x8
>     sys_perf_event_open failed, error -22
>     Using PERF_SAMPLE_READ / :S modifier is not compatible with
> inherit, falling back to no-inherit.
>     ------------------------------------------------------------
>     perf_event_attr:
>       type                             0 (PERF_TYPE_HARDWARE)
>       size                             136
>       config                           0 (PERF_COUNT_HW_CPU_CYCLES)
>       { sample_period, sample_freq }   4000
>       sample_type                      IP|TID|TIME|READ|ID|PERIOD
>       read_format                      ID|GROUP|LOST
>       disabled                         1
>       exclude_kernel                   1
>       exclude_hv                       1
>       mmap                             1
>       comm                             1
>       freq                             1
>       enable_on_exec                   1
>       task                             1
>       sample_id_all                    1
>       exclude_guest                    1
>       mmap2                            1
>       comm_exec                        1
>       ksymbol                          1
>       bpf_event                        1
>     ------------------------------------------------------------
>
> The command falls back to the same configuration as was previously
> used. The same is true for the instructions event.
>
> `perf test` fails on an unpatched kernel in "15: Setup struct
> perf_event_attr" for the test "test-record-group-sampling1" but that is
> surely expected for unpatched kernels?
>
> Is there some very-old kernel version where this would be expected to
> succeed by accident?

I don't think so but I don't want the test to fail depending on the
kernel version.  Maybe we can check the allowed combination
first and skip the test if perf_event_open() fails.  And then it can
verify if the kernel rejects the unsupported combinations.  Not
sure if it's easy to do that in the current attr test framework.

Thanks,
Namhyung