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

[Chen, Zide]

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;
- Finally, it succeeds after falling back to precision 2 — but the
inherit attribute has been unexpectedly removed.

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.

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.


>  	if (has_attr_feature(&attr, /*flags=*/0))
>  		goto found;
>  	perf_missing_features.inherit_sample_read = true;