Commit 5b546de9 authored by Ian Rogers's avatar Ian Rogers Committed by Namhyung Kim
Browse files

perf topdown: Use attribute to see an event is a topdown metic or slots 



The string comparisons were overly broad and could fire for the
incorrect PMU and events. Switch to using the config in the attribute
then add a perf test to confirm the attribute config values match
those of parsed events of that name and don't match others. This
exposed matches for slots events that shouldn't have matched as the
slots fixed counter event, such as topdown.slots_p.

Fixes: fbc79831 ("perf x86/topdown: Refine helper arch_is_topdown_metrics()")
Signed-off-by: default avatarIan Rogers <irogers@google.com>
Link: https://lore.kernel.org/r/20250719030517.1990983-14-irogers@google.com


Signed-off-by: default avatarNamhyung Kim <namhyung@kernel.org>
parent 811082e4

tools/perf/arch/x86/include/arch-tests.h

+4 −0
Original line number Diff line number Diff line
@@ -2,6 +2,8 @@ 
#ifndef ARCH_TESTS_H

#define ARCH_TESTS_H



#include "tests/tests.h"



struct test_suite;



/* Tests */
@@ -17,6 +19,8 @@ int test__amd_ibs_via_core_pmu(struct test_suite *test, int subtest); 
int test__amd_ibs_period(struct test_suite *test, int subtest);

int test__hybrid(struct test_suite *test, int subtest);



DECLARE_SUITE(x86_topdown);



extern struct test_suite *arch_tests[];



#endif

tools/perf/arch/x86/tests/Build

+1 −0
Original line number Diff line number Diff line
@@ -11,6 +11,7 @@ endif 
perf-test-$(CONFIG_X86_64) += bp-modify.o

perf-test-y += amd-ibs-via-core-pmu.o

perf-test-y += amd-ibs-period.o

perf-test-y += topdown.o



ifdef SHELLCHECK

  SHELL_TESTS := gen-insn-x86-dat.sh

tools/perf/arch/x86/tests/arch-tests.c

+1 −0
Original line number Diff line number Diff line
@@ -53,5 +53,6 @@ struct test_suite *arch_tests[] = { 
	&suite__amd_ibs_via_core_pmu,

	&suite__amd_ibs_period,

	&suite__hybrid,

	&suite__x86_topdown,

	NULL,

};

tools/perf/arch/x86/tests/topdown.c

0 → 100644
+76 −0
Original line number Diff line number Diff line 
// SPDX-License-Identifier: GPL-2.0

#include "arch-tests.h"

#include "../util/topdown.h"

#include "evlist.h"

#include "parse-events.h"

#include "pmu.h"

#include "pmus.h"



static int event_cb(void *state, struct pmu_event_info *info)

{

	char buf[256];

	struct parse_events_error parse_err;

	int *ret = state, err;

	struct evlist *evlist = evlist__new();

	struct evsel *evsel;



	if (!evlist)

		return -ENOMEM;



	parse_events_error__init(&parse_err);

	snprintf(buf, sizeof(buf), "%s/%s/", info->pmu->name, info->name);

	err = parse_events(evlist, buf, &parse_err);

	if (err) {

		parse_events_error__print(&parse_err, buf);

		*ret = TEST_FAIL;

	}

	parse_events_error__exit(&parse_err);

	evlist__for_each_entry(evlist, evsel) {

		bool fail = false;

		bool p_core_pmu = evsel->pmu->type == PERF_TYPE_RAW;

		const char *name = evsel__name(evsel);



		if (strcasestr(name, "uops_retired.slots") ||

		    strcasestr(name, "topdown.backend_bound_slots") ||

		    strcasestr(name, "topdown.br_mispredict_slots") ||

		    strcasestr(name, "topdown.memory_bound_slots") ||

		    strcasestr(name, "topdown.bad_spec_slots") ||

		    strcasestr(name, "topdown.slots_p")) {

			if (arch_is_topdown_slots(evsel) || arch_is_topdown_metrics(evsel))

				fail = true;

		} else if (strcasestr(name, "slots")) {

			if (arch_is_topdown_slots(evsel) != p_core_pmu ||

			    arch_is_topdown_metrics(evsel))

				fail = true;

		} else if (strcasestr(name, "topdown")) {

			if (arch_is_topdown_slots(evsel) ||

			    arch_is_topdown_metrics(evsel) != p_core_pmu)

				fail = true;

		} else if (arch_is_topdown_slots(evsel) || arch_is_topdown_metrics(evsel)) {

			fail = true;

		}

		if (fail) {

			pr_debug("Broken topdown information for '%s'\n", evsel__name(evsel));

			*ret = TEST_FAIL;

		}

	}

	evlist__delete(evlist);

	return 0;

}



static int test__x86_topdown(struct test_suite *test __maybe_unused, int subtest __maybe_unused)

{

	int ret = TEST_OK;

	struct perf_pmu *pmu = NULL;



	if (!topdown_sys_has_perf_metrics())

		return TEST_OK;



	while ((pmu = perf_pmus__scan_core(pmu)) != NULL) {

		if (perf_pmu__for_each_event(pmu, /*skip_duplicate_pmus=*/false, &ret, event_cb))

			break;

	}

	return ret;

}



DEFINE_SUITE("x86 topdown", x86_topdown);

tools/perf/arch/x86/util/evsel.c

+12 −34
Original line number Diff line number Diff line
@@ -23,47 +23,25 @@ void arch_evsel__set_sample_weight(struct evsel *evsel) 
bool evsel__sys_has_perf_metrics(const struct evsel *evsel)

{

	struct perf_pmu *pmu;

	u32 type = evsel->core.attr.type;



	/*

	 * The PERF_TYPE_RAW type is the core PMU type, e.g., "cpu" PMU

	 * on a non-hybrid machine, "cpu_core" PMU on a hybrid machine.

	 * The slots event is only available for the core PMU, which

	 * supports the perf metrics feature.

	 * Checking both the PERF_TYPE_RAW type and the slots event

	 * should be good enough to detect the perf metrics feature.

	 */

again:

	switch (type) {

	case PERF_TYPE_HARDWARE:

	case PERF_TYPE_HW_CACHE:

		type = evsel->core.attr.config >> PERF_PMU_TYPE_SHIFT;

		if (type)

			goto again;

		break;

	case PERF_TYPE_RAW:

		break;

	default:

	if (!topdown_sys_has_perf_metrics())

		return false;

	}



	pmu = evsel->pmu;

	if (pmu && perf_pmu__is_fake(pmu))

		pmu = NULL;



	if (!pmu) {

		while ((pmu = perf_pmus__scan_core(pmu)) != NULL) {

			if (pmu->type == PERF_TYPE_RAW)

				break;

		}

	}

	return pmu && perf_pmu__have_event(pmu, "slots");

	/*

	 * The PERF_TYPE_RAW type is the core PMU type, e.g., "cpu" PMU on a

	 * non-hybrid machine, "cpu_core" PMU on a hybrid machine.  The

	 * topdown_sys_has_perf_metrics checks the slots event is only available

	 * for the core PMU, which supports the perf metrics feature. Checking

	 * both the PERF_TYPE_RAW type and the slots event should be good enough

	 * to detect the perf metrics feature.

	 */

	pmu = evsel__find_pmu(evsel);

	return pmu && pmu->type == PERF_TYPE_RAW;

}



bool arch_evsel__must_be_in_group(const struct evsel *evsel)

{

	if (!evsel__sys_has_perf_metrics(evsel) || !evsel->name ||

	    strcasestr(evsel->name, "uops_retired.slots"))

	if (!evsel__sys_has_perf_metrics(evsel))

		return false;



	return arch_is_topdown_metrics(evsel) || arch_is_topdown_slots(evsel);