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

[Chen, Zide]

On 11/11/2025 2:31 PM, Namhyung Kim wrote:
> On Tue, Nov 11, 2025 at 12:03:22PM -0800, Chen, Zide wrote:
>>
>>
>> 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!
> 
> Can you please confirm if this patch fixes your problem?

Yes, it works!
> Thanks,
> Namhyung
>