Re: [PATCH 1/2] perf x86/topdown: Fix topdown leader sampling test error on hybrid

[Falcon, Thomas]

On Mon, 2025-02-24 at 08:33 +0000, Dapeng Mi wrote:
> When running topdown leader smapling test on Intel hybrid platforms,
> such as LNL/ARL, we see the below error.
> 
> Topdown leader sampling test
> Topdown leader sampling [Failed topdown events not reordered
> correctly]
> 
> It indciates the below command fails.
> 
> perf record -o "${perfdata}" -e "{instructions,slots,topdown-
> retiring}:S" true
> 
> The root cause is that perf tool creats a perf event for each PMU
> type
> if it can create.
> 
> As for this command, there would be 5 perf events created,
> cpu_atom/instructions/,cpu_atom/topdown_retiring/,
> cpu_core/slots/,cpu_core/instructions/,cpu_core/topdown-retiring/
> 
> For these 5 events, the 2 cpu_atom events are in a group and the
> other 3
> cpu_core events are in another group.
> 
> When arch_topdown_sample_read() traverses all these 5 events, events
> cpu_atom/instructions/ and cpu_core/slots/ don't have a same group
> leade, and then return false directly and lead to cpu_core/slots/
> event
> is used to sample and this is not allowed by PMU driver.
> 
> It's a overkill to return false directly if "evsel->core.leader !=
>  leader->core.leader" since there could be multiple groups in the
> event
> list.
> 
> Just "continue" instead of "return false" to fix this issue.
> 
> Fixes: 1e53e9d1787b ("perf x86/topdown: Correct leader selection with
> sample_read enabled")
> Signed-off-by: Dapeng Mi <dapeng1.mi@xxxxxxxxxxxxxxx>

Tested-by: Thomas Falcon <thomas.falcon@xxxxxxxxx>

> ---
>  tools/perf/arch/x86/util/topdown.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/perf/arch/x86/util/topdown.c
> b/tools/perf/arch/x86/util/topdown.c
> index f63747d0abdf..d1c654839049 100644
> --- a/tools/perf/arch/x86/util/topdown.c
> +++ b/tools/perf/arch/x86/util/topdown.c
> @@ -81,7 +81,7 @@ bool arch_topdown_sample_read(struct evsel *leader)
>  	 */
>  	evlist__for_each_entry(leader->evlist, evsel) {
>  		if (evsel->core.leader != leader->core.leader)
> -			return false;
> +			continue;
>  		if (evsel != leader &&
> arch_is_topdown_metrics(evsel))
>  			return true;
>  	}
> 
> base-commit: 4bac7fb5862740087825eda3ed6168e91da8b7e6