[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5c3e30ac-7fe3-47fb-923e-3def7902a772@intel.com>
Date: Thu, 23 Oct 2025 15:11:07 -0700
From: "Chen, Zide" <zide.chen@...el.com>
To: Ian Rogers <irogers@...gle.com>
Cc: 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>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
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 10/23/2025 9:14 AM, 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>
>
> 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 :-)
OK. Maybe a new test "PMU event open fallback tests".
Powered by blists - more mailing lists