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] [thread-next>] [day] [month] [year] [list]

Message-ID: <b97a8aa8-37b1-4d8f-9de0-f294fcd07adb@intel.com>
Date: Fri, 8 Nov 2024 20:01:02 +0200
From: Adrian Hunter <adrian.hunter@...el.com>
To: Namhyung Kim <namhyung@...nel.org>, Leo Yan <leo.yan@....com>
Cc: 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 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.

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.