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

[Namhyung Kim]

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.

Thanks,
Namhyung