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

[Namhyung Kim]

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?

Thanks,
Namhyung