[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5f84fe4f-90ef-42d6-8a3a-c1f515a7832a@intel.com>
Date: Thu, 6 Nov 2025 17:23:09 -0800
From: "Chen, Zide" <zide.chen@...el.com>
To: Namhyung Kim <namhyung@...nel.org>
Cc: linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.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>, Ian Rogers <irogers@...gle.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 11/6/2025 10:52 AM, Namhyung Kim wrote:
> On Tue, Nov 04, 2025 at 11:10:44AM -0800, Chen, Zide wrote:
>>
>>
>> On 11/3/2025 7:48 PM, Namhyung Kim wrote:
>>> Hello,
>>>
>>> Sorry for the delay.
>>>
>>> On Mon, Oct 27, 2025 at 11:56:52AM -0700, Chen, Zide wrote:
>>>>
>>>>
>>>> On 10/25/2025 5:42 PM, Namhyung Kim wrote:
>>>>> On Fri, Oct 24, 2025 at 11:03:17AM -0700, Chen, Zide wrote:
>>>>>>
>>>>>>
>>>>>> On 10/23/2025 7:30 PM, Namhyung Kim wrote:
>>>>>>> Hello,
>>>>>>>
>>>>>>> On Wed, Oct 22, 2025 at 03:08:02PM -0700, Zide Chen 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
>>>>>>>
>>>>>>> I'm curious about this part. Why the kernel set 'inherit = false'? IOW
>>>>>>> how did the leader event (mem-loads-aux) succeed with inherit = true
>>>>>>> then?
>>>>>>
>>>>>> Initially, the inherit = true for both the group leader
>>>>>> (cpu/mem-loads-aux/S) and the event in question (cpu/mem-loads,ldlat=3/PS).
>>>>>>
>>>>>> When the second event fails with EINVAL, the current logic calls
>>>>>> evsel__detect_missing_features() first. Since this is a PERF_SAMPLE_READ
>>>>>> event, the inherit attribute falls back to false, according to the
>>>>>> fallback order implemented in evsel__detect_missing_features().
>>>>>
>>>>> Right, that means the kernel doesn't support PERF_SAMPLE_READ with
>>>>> inherit = true. How did the first event succeed to open then?
>>>>
>>>> The perf tool sets PERF_SAMPLE_TID for Inherit + PERF_SAMPLE_READ
>>>> events, as implemented in commit 90035d3cd876 ("tools/perf: Allow
>>>> inherit + PERF_SAMPLE_READ when opening event").
>>>>
>>>> Meanwhile, commit 7e8b255650fc ("perf: Support PERF_SAMPLE_READ with
>>>> inherit") rejects a perf event if has_inherit_and_sample_read(attr) is
>>>> true and PERF_SAMPLE_TID is not set in attr->sample_type.
>>>>
>>>> Therefore, the first event succeeded, while the one opened in
>>>> evsel__detect_missing_features() which doesn't have PERF_SAMPLE_TID failed.
>>>
>>> Why does the first succeed and the second fail? Don't they have the
>>> same SAMPLE_READ and SAMPLE_TID + inherit flags?
>>
>> Sorry, my previous reply wasn’t entirely accurate. The first event
>> (cpu/mem-loads-aux/S) succeeds because it’s not a precise event
>> (precise_ip == 0).
>
> I'm not sure how it matters. I've tested the same command line on SPR
> and got this message. It says it failed to open because of inherit and
> SAMPE_READ. It didn't have precise_ip too.
>
> $ perf record -e cpu/mem-loads-aux/S -vv true |& less
> ...
> ------------------------------------------------------------
> perf_event_attr:
> type 4 (cpu)
> size 136
> config 0x8203 (mem-loads-aux)
> { sample_period, sample_freq } 4000
> sample_type IP|TID|TIME|READ|ID|PERIOD
> read_format ID|LOST
> disabled 1
> inherit 1
> mmap 1
> comm 1
> freq 1
> enable_on_exec 1
> task 1
> sample_id_all 1
> mmap2 1
> comm_exec 1
> ksymbol 1
> bpf_event 1
> ------------------------------------------------------------
> sys_perf_event_open: pid 1161023 cpu 0 group_fd -1 flags 0x8
> sys_perf_event_open failed, error -22
> Using PERF_SAMPLE_READ / :S modifier is not compatible with inherit, falling back to no-inherit.
> ...
>
> And it fell back to no-inherit and succeeded.
On my SPR, with either kernel 6.18.0-rc4 or the older 6.17.0-rc6, my
test results are different from yours — I didn’t see any EINVAL, and
there was no fallback. :)
It’s strange, but even so, since there’s no group leader in this case, I
assume that when it falls back to non-inherit, it should pass the
following check.
if (task && group_leader &&
group_leader->attr.inherit != attr.inherit) {
err = -EINVAL;
goto err_task;
}
> I've also found that it
> worked even with precise_ip = 3.
>
> $ perf record -e cpu/mem-loads-aux/PS -vv true |& less
> ...
> sys_perf_event_open: pid 1172834 cpu 0 group_fd -1 flags 0x8
> sys_perf_event_open failed, error -22
> Using PERF_SAMPLE_READ / :S modifier is not compatible with inherit, falling back to no-inherit.
> ------------------------------------------------------------
> perf_event_attr:
> type 4 (cpu)
> size 136
> config 0x8203 (mem-loads-aux)
> { sample_period, sample_freq } 4000
> sample_type IP|TID|TIME|READ|ID|PERIOD
> read_format ID|LOST
> disabled 1
> mmap 1
> comm 1
> freq 1
> enable_on_exec 1
> task 1
> precise_ip 3 <<<---- here
> sample_id_all 1
> mmap2 1
> comm_exec 1
> ksymbol 1
> bpf_event 1
> ------------------------------------------------------------
> sys_perf_event_open: pid 1172834 cpu 0 group_fd -1 flags 0x8 = 4
> ...
Again, on my machine, I didn’t see EINVAL, and no fallback to
non-inherit. In my test, glc_get_event_constraints() successfully forces
this event (config == 0x8203) to fixed counter 0, so there’s no issue here.
> And it works fine on my machine.
>
> $ perf record -e '{cpu/mem-loads-aux/S,cpu/mem-loads/PS}' ls
> ...
> [ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 0.033 MB perf.data (6 samples) ]
I don't know why it works for you, but in my tests, this event:
Opening: cpu/mem-loads/PS
------------------------------------------------------------
perf_event_attr:
type 4 (cpu)
size 248
config 0x1cd
(mem_trans_retired.load_latency_gt_1024)
{ sample_period, sample_freq } 4000
sample_type IP|TID|TIME|READ|ID|PERIOD
read_format ID|GROUP|LOST
inherit 1
freq 1
precise_ip 3
sample_id_all 1
{ bp_addr, config1 } 0x3
------------------------------------------------------------
It gets emptyconstraint, then it can't schedule the event on any counter
and x86_schedule_events() returns -EINVAL.
glc_get_event_constraints()
{
struct event_constraint *c;
// It gets the constraint INTEL_PLD_CONSTRAINT(0x1cd, 0xfe)
// from intel_pebs_constraints(),
c = icl_get_event_constraints(cpuc, idx, event);
// When it tries to force :ppp event to fixed counter 0
if ((event->attr.precise_ip == 3) &&
!constraint_match(&fixed0_constraint, event->hw.config)) {
// It happens the constrain doesn't mask fixed counter 0
if (c->idxmsk64 & BIT_ULL(0)) {
return &counter0_constraint;
// It gets here.
return &emptyconstraint;
}
return c;
}
After that, it falls back to non-inherit, and it fails again because the
inherit attribute differs from the group leader’s. This carries over to
the precise_ip fallback path in the current code.
>>
>> The second event fails with -EINVAL because, on some platforms, events
>> with precise_ip = 3 must be scheduled on fixed counter 0, and it fails
>> if it happens that this counter is unavailable.
>>
>> In the current code, the first fallback attempt (inherit = 0) also fails
>> because the inherit attribute differs from that of the group leader
>> (first event).
>
> So I don't understand this. Either the first event failed due to
> inherit set or the second event should succeed with inherit. Maybe
> there's an unknown bug or something.
>
> Thanks,
> namhyung
>
Powered by blists - more mailing lists