Re: [PATCH V14 08/11] perf tools: Add missing_features for aux_start_paused, aux_pause, aux_resume

[Namhyung Kim <namhyung@kernel.org>]

On Fri, Nov 08, 2024 at 08:01:02PM +0200, Adrian Hunter wrote:
> On 8/11/24 19:39, Namhyung Kim wrote:
> > Hello,
> > 
> > On Fri, Nov 08, 2024 at 03:41:52PM +0000, Leo Yan wrote:
> >> Hi Adrian,
> >>
> >> On Tue, Oct 22, 2024 at 06:59:14PM +0300, Adrian Hunter wrote:
> >>>
> >>> Display "feature is not supported" error message if aux_start_paused,
> >>> aux_pause or aux_resume result in a perf_event_open() error.
> >>>
> >>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> >>> Acked-by: Ian Rogers <irogers@google.com>
> >>> Reviewed-by: Andi Kleen <ak@linux.intel.com>
> >>> ---
> >>>
> >>>
> >>> Changes in V13:
> >>>         Add error message also in EOPNOTSUPP case (Leo)
> >>>
> >>>
> >>>  tools/perf/util/evsel.c | 12 ++++++++++++
> >>>  tools/perf/util/evsel.h |  1 +
> >>>  2 files changed, 13 insertions(+)
> >>>
> >>> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> >>> index 95593b55d9a7..88b31a005ac6 100644
> >>> --- a/tools/perf/util/evsel.c
> >>> +++ b/tools/perf/util/evsel.c
> >>> @@ -2102,6 +2102,12 @@ bool evsel__detect_missing_features(struct evsel *evsel)
> >>>                 perf_missing_features.inherit_sample_read = true;
> >>>                 pr_debug2("Using PERF_SAMPLE_READ / :S modifier is not compatible with inherit, falling back to no-inherit.\n");
> >>>                 return true;
> >>> +       } else if (!perf_missing_features.aux_pause_resume &&
> >>> +           (evsel->core.attr.aux_pause || evsel->core.attr.aux_resume ||
> >>> +            evsel->core.attr.aux_start_paused)) {
> >>> +               perf_missing_features.aux_pause_resume = true;
> >>> +               pr_debug2_peo("Kernel has no aux_pause/aux_resume support, bailing out\n");
> >>> +               return false;
> >>
> >> This patch fails to apply on the latest perf-tools-next branch due to
> >> conflict:
> >>
> >>   https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git
> >>   branch: perf-tools-next
> >>
> >> You might need to rebase it on the latest code base.
> > 
> > Yep, please do for the tooling patches.
> 
> I'd noticed that, but there is more work to sort it out.
> The probing simply won't work when there are dependencies
> on other events, or other attr members.  But there is no
> point in trying to "fallback" in that case either.  It is
> just a failure, but what *is* still needed is a sensible
> error message.

Do you need to probe with two or more events?  I guess we can extend
evsel__detect_missing_pmu_features to check more complicated setups.
For now, it only checks exclude_guest attribute.

> 
> The other patches, including the ones that come after,
> still apply by the way, or they did the other day, so
> they could be applied anyway.

Do you want me to process the series without this patch?  I think
this is needed to support old kernels and I'd like to have it together.
Would be hard for you to update this change in the current form and work
on improving it later?

Thanks,
Namhyung