Re: [PATCH] perf tools: Fix missing feature check for inherit + SAMPLE_READ

[Chen, Zide]

On 11/11/2025 11:40 AM, Namhyung Kim wrote:
> On Tue, Nov 11, 2025 at 11:13:20AM -0800, Chen, Zide wrote:
>>
>>
>> On 11/10/2025 11:59 PM, Namhyung Kim wrote:
>>> It should also have PERF_SAMPLE_TID to enable inherit and PERF_SAMPLE_READ
>>> on recent kernels.  Not having _TID makes the feature check wrongly detect
>>> the inherit and _READ support.
>>>
>>> It was reported that the following command failed due to the error in
>>> the missing feature check on Intel SPR machines.
>>>
>>>   $ perf record -e '{cpu/mem-loads-aux/S,cpu/mem-loads,ldlat=3/PS}' -- ls
>>>   Error:
>>>   Failure to open event 'cpu/mem-loads,ldlat=3/PS' on PMU 'cpu' which will be removed.
>>>   Invalid event (cpu/mem-loads,ldlat=3/PS) in per-thread mode, enable system wide with '-a'.
>>>
>>> Fixes: 3b193a57baf15c468 ("perf tools: Detect missing kernel features properly")
>>> Reported-by: "Chen, Zide" <zide.chen@xxxxxxxxx>
>>> Signed-off-by: Namhyung Kim <namhyung@xxxxxxxxxx>
>>> ---
>>>  tools/perf/util/evsel.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
>>> index 67a898cda86ab559..989c56d4a23f74f4 100644
>>> --- a/tools/perf/util/evsel.c
>>> +++ b/tools/perf/util/evsel.c
>>> @@ -2474,7 +2474,7 @@ static bool evsel__detect_missing_features(struct evsel *evsel, struct perf_cpu
>>>  	/* Please add new feature detection here. */
>>>  
>>>  	attr.inherit = true;
>>> -	attr.sample_type = PERF_SAMPLE_READ;
>>> +	attr.sample_type = PERF_SAMPLE_READ | PERF_SAMPLE_TID;
>>
>>
>> Seems this could have some unintended side effects. For example,
>> consider a :ppp event with PERF_SAMPLE_READ and inherit attributes
>> running on a system where the maximum precise_ip is 2:
>>
>> - It fails to open the event on the first attempt;
>> - It goes through the inherit_sample_read detection and fails again
>> after removing inherit;
> 
> This is not what we want.  The kernel supports inherit + SAMPLE_READ
> so it should not remove the inherit bit.
> 
> 
>> - Finally, it succeeds after falling back to precision 2 — but the
>> inherit attribute has been unexpectedly removed.
> 
> So it'll fallback to precision 2 without removing inherit.
> 
>>
>> I may have missed something, but I don’t quite understand why commit
>> 3b193a57baf15 ("perf tools: Detect missing kernel features properly")
>> performs the check on a dummy evsel instead of the original one. In this
>> way, it might incorrectly fall back an attribute that doesn’t actually help.
> 
> Because different platforms have different limitations on hardware
> events.  You cannot simply use current event for kernel feature check
> since it can result in wrong decisions due to the limitation.  So we
> picked the software event to avoid the hardware characteristics and to
> focus on kernel features.
> 
>>
>> This means evsel__detect_missing_features() could theoretically roll
>> back a feature that might not actually work. Given that it cannot
>> restore the original evsel state after a failed attempt, side effects
>> may occur.
> 
> The purpose is to turn off the non-supported features only and try with
> other settings like precise_ip and exclude_kernels and so on.

OK, thanks!
> Thanks,
> Namhyung
>