linux-kernel - Re: [PATCH] perf tools: Refactor precise

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: <aYZl2evOTebsTIxZ@x1>
Date: Fri, 6 Feb 2026 19:06:17 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Ian Rogers <irogers@...gle.com>
Cc: Zide Chen <zide.chen@...el.com>, linux-kernel@...r.kernel.org,
	linux-perf-users@...r.kernel.org,
	Namhyung Kim <namhyung@...nel.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Adrian Hunter <adrian.hunter@...el.com>,
	Ingo Molnar <mingo@...hat.com>, Jiri Olsa <jolsa@...nel.org>,
	Mark Rutland <mark.rutland@....com>,
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
	thomas.falcon@...el.com, dapeng1.mi@...ux.intel.com,
	xudong.hao@...el.com
Subject: Re: [PATCH] perf tools: Refactor precise_ip fallback logic

On Thu, Oct 23, 2025 at 09:14:17AM -0700, Ian Rogers wrote:
> On Wed, Oct 22, 2025 at 3:14 PM Zide Chen <zide.chen@...el.com> wrote:
> >
> > Commit c33aea446bf555ab ("perf tools: Fix precise_ip fallback logic")
> > unconditionally called the precise_ip fallback and moved it after the
> > missing-feature checks so that it could handle EINVAL as well.
> >
> > However, this introduced an issue: after disabling missing features,
> > the event could fail to open, which makes the subsequent precise_ip
> > fallback useless since it will always fail.
> >
> > For example, run the following command on Intel SPR:
> >
> > $ perf record -e '{cpu/mem-loads-aux/S,cpu/mem-loads,ldlat=3/PS}' -- ls
> >
> > Opening the event "cpu/mem-loads,ldlat=3/PS" returns EINVAL when
> > precise_ip == 3. It then sets attr.inherit = false, which triggers a
> > kernel check failure since it doesn't match the group leader's inherit
> > attribute. As a result, it continues to fail even after precise_ip is
> > reduced.
> >
> > By moving the precise_ip fallback earlier, this issue is resolved, as
> > well as the kernel test robot report mentioned in commit
> > c33aea446bf555ab.
> >
> > No negative side effects are expected, because the precise_ip level is
> > restored by evsel__precise_ip_fallback() if the fallback does not help.
> >
> > This also aligns with commit 2b70702917337a8d ("perf tools: Remove
> > evsel__handle_error_quirks()").
> >
> > Fixes: af954f76eea56453 ("perf tools: Check fallback error and order")
> > Fixes: c33aea446bf555ab ("perf tools: Fix precise_ip fallback logic")
> > Reviewed-by: Dapeng Mi <dapeng1.mi@...ux.intel.com>
> > Signed-off-by: Zide Chen <zide.chen@...el.com>
> 
> Acked-by: Ian Rogers <irogers@...gle.com>

Fell thru the cracks... Still applies cleanly.

Thanks, applied to perf-tools-next,

- Arnaldo
 
> Any chance you could help with a test case that covers this? The
> fallback logic is spread out and easy to introduce subtle bugs into.
> Just having a test case that does ` perf record -e
> '{cpu/mem-loads-aux/S,cpu/mem-loads,ldlat=3/PS}' -- ls` and checks the
> output for EINVAL when the events are present would be useful, as then
> we can make sure this doesn't regress on SPR and later. Something with
> more generic events would of course be better :-)
> 
> Thanks,
> Ian
> 
> > ---
> >  tools/perf/util/evsel.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> > index ca74514c8707..6ce32533a213 100644
> > --- a/tools/perf/util/evsel.c
> > +++ b/tools/perf/util/evsel.c
> > @@ -2714,12 +2714,12 @@ static int evsel__open_cpu(struct evsel *evsel, struct perf_cpu_map *cpus,
> >         if (err == -EMFILE && rlimit__increase_nofile(&set_rlimit))
> >                 goto retry_open;
> >
> > +       if (evsel__precise_ip_fallback(evsel))
> > +               goto retry_open;
> > +
> >         if (err == -EINVAL && evsel__detect_missing_features(evsel, cpu))
> >                 goto fallback_missing_features;
> >
> > -       if (evsel__precise_ip_fallback(evsel))
> > -               goto retry_open;
> > -
> >  out_close:
> >         if (err)
> >                 threads->err_thread = thread;
> > --
> > 2.51.0
> >