[PATCH] perf tools: Refactor precise_ip fallback logic

[PATCH] perf tools: Refactor precise_ip fallback logic

[Zide Chen]

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@linux.intel.com>
Signed-off-by: Zide Chen <zide.chen@intel.com>
---
 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

Re: [PATCH] perf tools: Refactor precise_ip fallback logic

[Ian Rogers]

On Wed, Oct 22, 2025 at 3:14 PM Zide Chen <zide.chen@intel.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@linux.intel.com>
> Signed-off-by: Zide Chen <zide.chen@intel.com>

Acked-by: Ian Rogers <irogers@google.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 :-)

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
>

Re: [PATCH] perf tools: Refactor precise_ip fallback logic

[Chen, Zide]

On 10/23/2025 9:14 AM, Ian Rogers wrote:
> On Wed, Oct 22, 2025 at 3:14 PM Zide Chen <zide.chen@intel.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@linux.intel.com>
>> Signed-off-by: Zide Chen <zide.chen@intel.com>
> 
> Acked-by: Ian Rogers <irogers@google.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".

Re: [PATCH] perf tools: Refactor precise_ip fallback logic

[Namhyung Kim]

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?

> 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.

I'm not sure.. some events may need a different (i.e. lower) precise
level than the max.  I think checking the missing feature later will
use the max precise level always.

Thanks,
Namhyung

> 
> 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@linux.intel.com>
> Signed-off-by: Zide Chen <zide.chen@intel.com>
> ---
>  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
> 

Re: [PATCH] perf tools: Refactor precise_ip fallback logic

[Chen, Zide]

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().

> 
>> 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.
> 
> I'm not sure.. some events may need a different (i.e. lower) precise
> level than the max.  I think checking the missing feature later will
> use the max precise level always.

Yes, but seems the basic idea of the event open fallback logic is to
check whether it's lucky enough to open the event by falling back one
single attribute, not multiple attributes.

evsel__precise_ip_fallback() can restore the precise_ip level after a
failed attempt, while evsel__detect_missing_features() cannot recover
the event attributes from its failed try.

Therefore, falling back precise_ip first maintains the intended
“one-by-one” fallback logic. If it’s placed later, it may combine two
fallbacks, which can cause failures like the example above.  Of course,
in theory, there might be cases where an event can be opened if both
precise_ip and another feature are relaxed together, but I haven’t
exhaustively checked whether such cases actually exist.

> Thanks,
> Namhyung
> 
>>
>> 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@linux.intel.com>
>> Signed-off-by: Zide Chen <zide.chen@intel.com>
>> ---
>>  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
>>

Re: [PATCH] perf tools: Refactor precise_ip fallback logic

[Namhyung Kim]

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?

> 
> > 
> >> 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.
> > 
> > I'm not sure.. some events may need a different (i.e. lower) precise
> > level than the max.  I think checking the missing feature later will
> > use the max precise level always.
> 
> Yes, but seems the basic idea of the event open fallback logic is to
> check whether it's lucky enough to open the event by falling back one
> single attribute, not multiple attributes.
> 
> evsel__precise_ip_fallback() can restore the precise_ip level after a
> failed attempt, while evsel__detect_missing_features() cannot recover
> the event attributes from its failed try.

I think precise_ip_fallback() is just a trial and error for each possible
value.  While detect_missing_features() checks what the current kernel
supports.  Trying different precise_ip values with unsupported attributes
doesn't make sense.

Thanks,
Namhyung

> 
> Therefore, falling back precise_ip first maintains the intended
> “one-by-one” fallback logic. If it’s placed later, it may combine two
> fallbacks, which can cause failures like the example above.  Of course,
> in theory, there might be cases where an event can be opened if both
> precise_ip and another feature are relaxed together, but I haven’t
> exhaustively checked whether such cases actually exist.
> 
> > Thanks,
> > Namhyung
> > 
> >>
> >> 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@linux.intel.com>
> >> Signed-off-by: Zide Chen <zide.chen@intel.com>
> >> ---
> >>  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
> >>
> 

Re: [PATCH] perf tools: Refactor precise_ip fallback logic

[Chen, Zide]

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.


>>
>>>
>>>> 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.
>>>
>>> I'm not sure.. some events may need a different (i.e. lower) precise
>>> level than the max.  I think checking the missing feature later will
>>> use the max precise level always.
>>
>> Yes, but seems the basic idea of the event open fallback logic is to
>> check whether it's lucky enough to open the event by falling back one
>> single attribute, not multiple attributes.
>>
>> evsel__precise_ip_fallback() can restore the precise_ip level after a
>> failed attempt, while evsel__detect_missing_features() cannot recover
>> the event attributes from its failed try.
> 
> I think precise_ip_fallback() is just a trial and error for each possible
> value.  While detect_missing_features() checks what the current kernel
> supports.  Trying different precise_ip values with unsupported attributes
> doesn't make sense.

When it returns -EINVAL, the cause could be an unsupported precise_ip or
something else. We could either end up with "trying different precise_ip
values with unsupported attributes", or "trying attributes with
unsupported precise_ip".

The perf tool’s fallback logic is a “best effort” mechanism to fix only
one issue, not multiple ones. So, IMO, we should place
evsel__detect_missing_features() as the last fallback attempt, since it
does not restore the event attributes after a failed try.
> Thanks,
> Namhyung
> 
>>
>> Therefore, falling back precise_ip first maintains the intended
>> “one-by-one” fallback logic. If it’s placed later, it may combine two
>> fallbacks, which can cause failures like the example above.  Of course,
>> in theory, there might be cases where an event can be opened if both
>> precise_ip and another feature are relaxed together, but I haven’t
>> exhaustively checked whether such cases actually exist.
>>
>>> Thanks,
>>> Namhyung
>>>
>>>>
>>>> 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@linux.intel.com>
>>>> Signed-off-by: Zide Chen <zide.chen@intel.com>
>>>> ---
>>>>  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
>>>>
>>

Re: [PATCH] perf tools: Refactor precise_ip fallback logic

[Namhyung Kim]

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?

> 
> 
> >>
> >>>
> >>>> 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.
> >>>
> >>> I'm not sure.. some events may need a different (i.e. lower) precise
> >>> level than the max.  I think checking the missing feature later will
> >>> use the max precise level always.
> >>
> >> Yes, but seems the basic idea of the event open fallback logic is to
> >> check whether it's lucky enough to open the event by falling back one
> >> single attribute, not multiple attributes.
> >>
> >> evsel__precise_ip_fallback() can restore the precise_ip level after a
> >> failed attempt, while evsel__detect_missing_features() cannot recover
> >> the event attributes from its failed try.
> > 
> > I think precise_ip_fallback() is just a trial and error for each possible
> > value.  While detect_missing_features() checks what the current kernel
> > supports.  Trying different precise_ip values with unsupported attributes
> > doesn't make sense.
> 
> When it returns -EINVAL, the cause could be an unsupported precise_ip or
> something else. We could either end up with "trying different precise_ip
> values with unsupported attributes", or "trying attributes with
> unsupported precise_ip".
> 
> The perf tool’s fallback logic is a “best effort” mechanism to fix only
> one issue, not multiple ones. So, IMO, we should place
> evsel__detect_missing_features() as the last fallback attempt, since it
> does not restore the event attributes after a failed try.

The missing feature check is about the global kernel behavior so there's
no point to try if we know the kernel won't support those features.
While precise fallback is per-PMU (and per-event?) behavior so it'd be
natural to try that after removing must-fail attributes from the missing
feature tests.

Thanks,
Namhyung

> >>
> >> Therefore, falling back precise_ip first maintains the intended
> >> “one-by-one” fallback logic. If it’s placed later, it may combine two
> >> fallbacks, which can cause failures like the example above.  Of course,
> >> in theory, there might be cases where an event can be opened if both
> >> precise_ip and another feature are relaxed together, but I haven’t
> >> exhaustively checked whether such cases actually exist.
> >>
> >>> Thanks,
> >>> Namhyung
> >>>
> >>>>
> >>>> 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@linux.intel.com>
> >>>> Signed-off-by: Zide Chen <zide.chen@intel.com>
> >>>> ---
> >>>>  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
> >>>>
> >>
> 

Re: [PATCH] perf tools: Refactor precise_ip fallback logic

[Chen, Zide]

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).

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).


>>
>>>>
>>>>>
>>>>>> 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.
>>>>>
>>>>> I'm not sure.. some events may need a different (i.e. lower) precise
>>>>> level than the max.  I think checking the missing feature later will
>>>>> use the max precise level always.
>>>>
>>>> Yes, but seems the basic idea of the event open fallback logic is to
>>>> check whether it's lucky enough to open the event by falling back one
>>>> single attribute, not multiple attributes.
>>>>
>>>> evsel__precise_ip_fallback() can restore the precise_ip level after a
>>>> failed attempt, while evsel__detect_missing_features() cannot recover
>>>> the event attributes from its failed try.
>>>
>>> I think precise_ip_fallback() is just a trial and error for each possible
>>> value.  While detect_missing_features() checks what the current kernel
>>> supports.  Trying different precise_ip values with unsupported attributes
>>> doesn't make sense.
>>
>> When it returns -EINVAL, the cause could be an unsupported precise_ip or
>> something else. We could either end up with "trying different precise_ip
>> values with unsupported attributes", or "trying attributes with
>> unsupported precise_ip".
>>
>> The perf tool’s fallback logic is a “best effort” mechanism to fix only
>> one issue, not multiple ones. So, IMO, we should place
>> evsel__detect_missing_features() as the last fallback attempt, since it
>> does not restore the event attributes after a failed try.
> 
> The missing feature check is about the global kernel behavior so there's
> no point to try if we know the kernel won't support those features.
> While precise fallback is per-PMU (and per-event?) behavior so it'd be
> natural to try that after removing must-fail attributes from the missing
> feature tests.

But someone may argue that since presise_ip is per-event and it's less
intrusive, why not try it first?

If we want to keep this principle, we need to ensure that detect missing
features does not incorrectly remove valid features, and there’s no need
to restore the removed features.

After commit 3b193a57baf1 (“perf tools: Detect missing kernel features
properly”), it no longer checks attributes based on the previously
failed evsel. Instead, it checks against a dummy event. This  makes it
difficult to correctly detect features with complex dependencies — for
example, group events involves PERF_SAMPLE_READ, PERF_SAMPLE_TID, and
inherit.

Another argument is what if the original evsel fails because of multiple
invalid attributes? Seems it's hard to trust the missing feature
detection to find out "must-fail" attributes.

> 
> Thanks,
> Namhyung
> 
>>>>
>>>> Therefore, falling back precise_ip first maintains the intended
>>>> “one-by-one” fallback logic. If it’s placed later, it may combine two
>>>> fallbacks, which can cause failures like the example above.  Of course,
>>>> in theory, there might be cases where an event can be opened if both
>>>> precise_ip and another feature are relaxed together, but I haven’t
>>>> exhaustively checked whether such cases actually exist.
>>>>
>>>>> Thanks,
>>>>> Namhyung
>>>>>
>>>>>>
>>>>>> 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@linux.intel.com>
>>>>>> Signed-off-by: Zide Chen <zide.chen@intel.com>
>>>>>> ---
>>>>>>  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
>>>>>>
>>>>
>>

Re: [PATCH] perf tools: Refactor precise_ip fallback logic

[Namhyung Kim]

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.  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
  ...

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) ]

> 
> 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

Re: [PATCH] perf tools: Refactor precise_ip fallback logic

[Chen, Zide]

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
> 

Re: [PATCH] perf tools: Refactor precise_ip fallback logic

[Namhyung Kim]

On Thu, Nov 06, 2025 at 05:23:09PM -0800, Chen, Zide wrote:
> 
> 
> 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. :)

Yep, your kernel is recent and has the following commit.

7e8b255650fcfa1d0 ("perf: Support PERF_SAMPLE_READ with inherit")

My kernel is 6.6 and it rejects such a combination.  I'll test it on
newer kernels later.

> 
> 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.

That means your missing_features.inherit_sample_read should not be set.
It's strange you have that with the recent kernels.

Can you run these commands and show the output here?

  $ perf record -e task-clock:S  true
  $ perf evlist -v

Thanks,
Namhyung

> 
> > 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
> > 
> 

Re: [PATCH] perf tools: Refactor precise_ip fallback logic

[Chen, Zide]

On 11/7/2025 1:42 PM, Namhyung Kim wrote:
> On Thu, Nov 06, 2025 at 05:23:09PM -0800, Chen, Zide wrote:
>>
>>
>> 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. :)
> 
> Yep, your kernel is recent and has the following commit.
> 
> 7e8b255650fcfa1d0 ("perf: Support PERF_SAMPLE_READ with inherit")
> 
> My kernel is 6.6 and it rejects such a combination.  I'll test it on
> newer kernels later.
> 
>>
>> 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.
> 
> That means your missing_features.inherit_sample_read should not be set.
> It's strange you have that with the recent kernels.
> 
> Can you run these commands and show the output here?
> 
>   $ perf record -e task-clock:S  true
>   $ perf evlist -v

On 6.18.0-rc4:

$ perf record -e task-clock:S  true
[ perf record: Woken up 2 times to write data ]
[ perf record: Captured and wrote 0.006 MB perf.data ]

$ perf evlist -v
task-clock:Su: type: 1 (PERF_TYPE_SOFTWARE), size: 136, config: 0x1
(PERF_COUNT_SW_TASK_CLOCK), { sample_period, sample_freq }: 4000,
sample_type: IP|TID|TIME|READ|ID|PERIOD, read_format: ID|LOST, disabled:
1, inherit: 1, exclude_kernel: 1, exclude_hv: 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, build_id: 1


> Thanks,
> Namhyung
> 
>>
>>> 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
>>>
>>

Re: [PATCH] perf tools: Refactor precise_ip fallback logic

[Namhyung Kim]

On Fri, Nov 07, 2025 at 02:31:23PM -0800, Chen, Zide wrote:
> 
> 
> On 11/7/2025 1:42 PM, Namhyung Kim wrote:
> > On Thu, Nov 06, 2025 at 05:23:09PM -0800, Chen, Zide wrote:
> >>
> >>
> >> 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. :)
> > 
> > Yep, your kernel is recent and has the following commit.
> > 
> > 7e8b255650fcfa1d0 ("perf: Support PERF_SAMPLE_READ with inherit")
> > 
> > My kernel is 6.6 and it rejects such a combination.  I'll test it on
> > newer kernels later.
> > 
> >>
> >> 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.
> > 
> > That means your missing_features.inherit_sample_read should not be set.
> > It's strange you have that with the recent kernels.
> > 
> > Can you run these commands and show the output here?
> > 
> >   $ perf record -e task-clock:S  true
> >   $ perf evlist -v
> 
> On 6.18.0-rc4:
> 
> $ perf record -e task-clock:S  true
> [ perf record: Woken up 2 times to write data ]
> [ perf record: Captured and wrote 0.006 MB perf.data ]
> 
> $ perf evlist -v
> task-clock:Su: type: 1 (PERF_TYPE_SOFTWARE), size: 136, config: 0x1
> (PERF_COUNT_SW_TASK_CLOCK), { sample_period, sample_freq }: 4000,
> sample_type: IP|TID|TIME|READ|ID|PERIOD, read_format: ID|LOST, disabled:
> 1, inherit: 1, exclude_kernel: 1, exclude_hv: 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, build_id: 1

Thanks for sharing this.  Yep, it has the inherit bit.

I think there's a bug in the missing feature test.  Indeed, it should
also have PERF_SAMPLE_TID for the test according to the kernel comment.

	/*
	 * We do not support PERF_SAMPLE_READ on inherited events unless
	 * PERF_SAMPLE_TID is also selected, which allows inherited events to
	 * collect per-thread samples.
	 * See perf_output_read().
	 */
	if (has_inherit_and_sample_read(attr) && !(attr->sample_type & PERF_SAMPLE_TID))
		return ERR_PTR(-EINVAL);

I'll send a patch soon.

Thanks,
Namhyung

Re: [PATCH] perf tools: Refactor precise_ip fallback logic

[Chen, Zide]

On 11/10/2025 11:50 PM, Namhyung Kim wrote:
> On Fri, Nov 07, 2025 at 02:31:23PM -0800, Chen, Zide wrote:
>>
>>
>> On 11/7/2025 1:42 PM, Namhyung Kim wrote:
>>> On Thu, Nov 06, 2025 at 05:23:09PM -0800, Chen, Zide wrote:
>>>>
>>>>
>>>> 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. :)
>>>
>>> Yep, your kernel is recent and has the following commit.
>>>
>>> 7e8b255650fcfa1d0 ("perf: Support PERF_SAMPLE_READ with inherit")
>>>
>>> My kernel is 6.6 and it rejects such a combination.  I'll test it on
>>> newer kernels later.
>>>
>>>>
>>>> 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.
>>>
>>> That means your missing_features.inherit_sample_read should not be set.
>>> It's strange you have that with the recent kernels.
>>>
>>> Can you run these commands and show the output here?
>>>
>>>   $ perf record -e task-clock:S  true
>>>   $ perf evlist -v
>>
>> On 6.18.0-rc4:
>>
>> $ perf record -e task-clock:S  true
>> [ perf record: Woken up 2 times to write data ]
>> [ perf record: Captured and wrote 0.006 MB perf.data ]
>>
>> $ perf evlist -v
>> task-clock:Su: type: 1 (PERF_TYPE_SOFTWARE), size: 136, config: 0x1
>> (PERF_COUNT_SW_TASK_CLOCK), { sample_period, sample_freq }: 4000,
>> sample_type: IP|TID|TIME|READ|ID|PERIOD, read_format: ID|LOST, disabled:
>> 1, inherit: 1, exclude_kernel: 1, exclude_hv: 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, build_id: 1
> 
> Thanks for sharing this.  Yep, it has the inherit bit.
> 
> I think there's a bug in the missing feature test.  Indeed, it should
> also have PERF_SAMPLE_TID for the test according to the kernel comment.
> 
> 	/*
> 	 * We do not support PERF_SAMPLE_READ on inherited events unless
> 	 * PERF_SAMPLE_TID is also selected, which allows inherited events to
> 	 * collect per-thread samples.
> 	 * See perf_output_read().
> 	 */
> 	if (has_inherit_and_sample_read(attr) && !(attr->sample_type & PERF_SAMPLE_TID))
> 		return ERR_PTR(-EINVAL);

It seems that the purpose of the inherit_sample_read fallback is to
remove the inherit attribute when both PERF_SAMPLE_READ and inherit are
present, but PERF_SAMPLE_TID is not. The new change may not be able to
accomplish this?


> 
> I'll send a patch soon.
> 
> Thanks,
> Namhyung
> 

Re: [PATCH] perf tools: Refactor precise_ip fallback logic

[Namhyung Kim]

On Tue, Nov 11, 2025 at 11:11:45AM -0800, Chen, Zide wrote:
> 
> 
> On 11/10/2025 11:50 PM, Namhyung Kim wrote:
> > On Fri, Nov 07, 2025 at 02:31:23PM -0800, Chen, Zide wrote:
> >>
> >>
> >> On 11/7/2025 1:42 PM, Namhyung Kim wrote:
> >>> On Thu, Nov 06, 2025 at 05:23:09PM -0800, Chen, Zide wrote:
> >>>>
> >>>>
> >>>> On 11/6/2025 10:52 AM, Namhyung Kim wrote:

> >>> Can you run these commands and show the output here?
> >>>
> >>>   $ perf record -e task-clock:S  true
> >>>   $ perf evlist -v
> >>
> >> On 6.18.0-rc4:
> >>
> >> $ perf record -e task-clock:S  true
> >> [ perf record: Woken up 2 times to write data ]
> >> [ perf record: Captured and wrote 0.006 MB perf.data ]
> >>
> >> $ perf evlist -v
> >> task-clock:Su: type: 1 (PERF_TYPE_SOFTWARE), size: 136, config: 0x1
> >> (PERF_COUNT_SW_TASK_CLOCK), { sample_period, sample_freq }: 4000,
> >> sample_type: IP|TID|TIME|READ|ID|PERIOD, read_format: ID|LOST, disabled:
> >> 1, inherit: 1, exclude_kernel: 1, exclude_hv: 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, build_id: 1
> > 
> > Thanks for sharing this.  Yep, it has the inherit bit.
> > 
> > I think there's a bug in the missing feature test.  Indeed, it should
> > also have PERF_SAMPLE_TID for the test according to the kernel comment.
> > 
> > 	/*
> > 	 * We do not support PERF_SAMPLE_READ on inherited events unless
> > 	 * PERF_SAMPLE_TID is also selected, which allows inherited events to
> > 	 * collect per-thread samples.
> > 	 * See perf_output_read().
> > 	 */
> > 	if (has_inherit_and_sample_read(attr) && !(attr->sample_type & PERF_SAMPLE_TID))
> > 		return ERR_PTR(-EINVAL);
> 
> It seems that the purpose of the inherit_sample_read fallback is to
> remove the inherit attribute when both PERF_SAMPLE_READ and inherit are
> present, but PERF_SAMPLE_TID is not. The new change may not be able to
> accomplish this?

No, the purpose of the missing feature check is to detect whether the
current kernel supports this feature or not.  The correct check should
pass both READ and TID together.

Thanks,
Namhyung

Re: [PATCH] perf tools: Refactor precise_ip fallback logic

[Chen, Zide]

On 11/11/2025 11:34 AM, Namhyung Kim wrote:
> On Tue, Nov 11, 2025 at 11:11:45AM -0800, Chen, Zide wrote:
>>
>>
>> On 11/10/2025 11:50 PM, Namhyung Kim wrote:
>>> On Fri, Nov 07, 2025 at 02:31:23PM -0800, Chen, Zide wrote:
>>>>
>>>>
>>>> On 11/7/2025 1:42 PM, Namhyung Kim wrote:
>>>>> On Thu, Nov 06, 2025 at 05:23:09PM -0800, Chen, Zide wrote:
>>>>>>
>>>>>>
>>>>>> On 11/6/2025 10:52 AM, Namhyung Kim wrote:
> 
>>>>> Can you run these commands and show the output here?
>>>>>
>>>>>   $ perf record -e task-clock:S  true
>>>>>   $ perf evlist -v
>>>>
>>>> On 6.18.0-rc4:
>>>>
>>>> $ perf record -e task-clock:S  true
>>>> [ perf record: Woken up 2 times to write data ]
>>>> [ perf record: Captured and wrote 0.006 MB perf.data ]
>>>>
>>>> $ perf evlist -v
>>>> task-clock:Su: type: 1 (PERF_TYPE_SOFTWARE), size: 136, config: 0x1
>>>> (PERF_COUNT_SW_TASK_CLOCK), { sample_period, sample_freq }: 4000,
>>>> sample_type: IP|TID|TIME|READ|ID|PERIOD, read_format: ID|LOST, disabled:
>>>> 1, inherit: 1, exclude_kernel: 1, exclude_hv: 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, build_id: 1
>>>
>>> Thanks for sharing this.  Yep, it has the inherit bit.
>>>
>>> I think there's a bug in the missing feature test.  Indeed, it should
>>> also have PERF_SAMPLE_TID for the test according to the kernel comment.
>>>
>>> 	/*
>>> 	 * We do not support PERF_SAMPLE_READ on inherited events unless
>>> 	 * PERF_SAMPLE_TID is also selected, which allows inherited events to
>>> 	 * collect per-thread samples.
>>> 	 * See perf_output_read().
>>> 	 */
>>> 	if (has_inherit_and_sample_read(attr) && !(attr->sample_type & PERF_SAMPLE_TID))
>>> 		return ERR_PTR(-EINVAL);
>>
>> It seems that the purpose of the inherit_sample_read fallback is to
>> remove the inherit attribute when both PERF_SAMPLE_READ and inherit are
>> present, but PERF_SAMPLE_TID is not. The new change may not be able to
>> accomplish this?
> 
> No, the purpose of the missing feature check is to detect whether the
> current kernel supports this feature or not.  The correct check should
> pass both READ and TID together.


OK, thanks!

> Thanks,
> Namhyung
>