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

[Namhyung Kim <namhyung@kernel.org>]

On Tue, Nov 11, 2025 at 02:36:19PM -0800, Chen, Zide wrote:
> 
> 
> 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@intel.com>
> >>>>> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> >>>>> ---
> >>>>>  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, I'll add your Tested-by then!

Namhyung