linux-kernel - Re: [PATCH V14 08/11] perf tools: Add missing_features for aux_start_paused, aux_pause, aux

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <Zy5ZHHgO69gQwZRo@google.com>
Date: Fri, 8 Nov 2024 10:31:56 -0800
From: Namhyung Kim <namhyung@...nel.org>
To: Adrian Hunter <adrian.hunter@...el.com>
Cc: Leo Yan <leo.yan@....com>, Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...hat.com>, Mark Rutland <mark.rutland@....com>,
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
	Heiko Carstens <hca@...ux.ibm.com>,
	Thomas Richter <tmricht@...ux.ibm.com>,
	Hendrik Brueckner <brueckner@...ux.ibm.com>,
	Suzuki K Poulose <suzuki.poulose@....com>,
	Mike Leach <mike.leach@...aro.org>,
	James Clark <james.clark@....com>, coresight@...ts.linaro.org,
	linux-arm-kernel@...ts.infradead.org,
	Yicong Yang <yangyicong@...ilicon.com>,
	Jonathan Cameron <jonathan.cameron@...wei.com>,
	Will Deacon <will@...nel.org>,
	Arnaldo Carvalho de Melo <acme@...nel.org>,
	Jiri Olsa <jolsa@...nel.org>, Ian Rogers <irogers@...gle.com>,
	Andi Kleen <ak@...ux.intel.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Borislav Petkov <bp@...en8.de>,
	Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
	H Peter Anvin <hpa@...or.com>,
	Kan Liang <kan.liang@...ux.intel.com>, linux-kernel@...r.kernel.org,
	linux-perf-users@...r.kernel.org
Subject: Re: [PATCH V14 08/11] perf tools: Add missing_features for
 aux_start_paused, aux_pause, aux_resume

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@...el.com>
> >>> Acked-by: Ian Rogers <irogers@...gle.com>
> >>> Reviewed-by: Andi Kleen <ak@...ux.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