Message-Id: <C7F7A9B0-421E-40AB-987C-6D250B9633D3@linux.vnet.ibm.com>
Date:   Tue, 6 Dec 2022 22:57:44 +0530
From:   Athira Rajeev <atrajeev@...ux.vnet.ibm.com>
To:     Ravi Bangoria <ravi.bangoria@....com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>
Cc:     Kan Liang <kan.liang@...ux.intel.com>,
        Ian Rogers <irogers@...gle.com>, Jiri Olsa <jolsa@...hat.com>,
        Namhyung Kim <namhyung@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Mark Rutland <mark.rutland@....com>,
        Adrian Hunter <adrian.hunter@...el.com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        carsten.haitzler@....com, Leo Yan <leo.yan@...aro.org>,
        Madhavan Srinivasan <maddy@...ux.ibm.com>,
        Kajol Jain <kjain@...ux.ibm.com>,
        Thomas Richter <tmricht@...ux.ibm.com>,
        linux-perf-users@...r.kernel.org,
        LKML <linux-kernel@...r.kernel.org>, sandipan.das@....com,
        ananth.narayan@....com, santosh.shukla@....com
Subject: Re: [PATCH v3 2/2] perf test: Add event group test



> On 06-Dec-2022, at 10:02 AM, Ravi Bangoria <ravi.bangoria@....com> wrote:
> 
> Multiple events in a group can belong to one or more pmus, however
> there are some limitations to it. One of the limitation is, perf
> doesn't allow creating a group of events from different hw pmus.
> Write a simple test to create various combinations of hw, sw and
> uncore pmu events and verify group creation succeeds or fails as
> expected.
> 
> Signed-off-by: Ravi Bangoria <ravi.bangoria@....com>
> ---
> tools/perf/tests/Build          |   1 +
> tools/perf/tests/builtin-test.c |   1 +
> tools/perf/tests/event_groups.c | 127 ++++++++++++++++++++++++++++++++
> tools/perf/tests/tests.h        |   1 +
> 4 files changed, 130 insertions(+)
> create mode 100644 tools/perf/tests/event_groups.c
> 
> diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
> index 11b69023011b..658b5052c24d 100644
> --- a/tools/perf/tests/Build
> +++ b/tools/perf/tests/Build
> @@ -67,6 +67,7 @@ perf-y += expand-cgroup.o
> perf-y += perf-time-to-tsc.o
> perf-y += dlfilter-test.o
> perf-y += sigtrap.o
> +perf-y += event_groups.o
> 
> $(OUTPUT)tests/llvm-src-base.c: tests/bpf-script-example.c tests/Build
> 	$(call rule_mkdir)
> diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> index 4c6ae59a4dfd..ddd8262bfa26 100644
> --- a/tools/perf/tests/builtin-test.c
> +++ b/tools/perf/tests/builtin-test.c
> @@ -110,6 +110,7 @@ static struct test_suite *generic_tests[] = {
> 	&suite__perf_time_to_tsc,
> 	&suite__dlfilter,
> 	&suite__sigtrap,
> +	&suite__event_groups,
> 	NULL,
> };
> 
> diff --git a/tools/perf/tests/event_groups.c b/tools/perf/tests/event_groups.c
> new file mode 100644
> index 000000000000..612c0444aaa8
> --- /dev/null
> +++ b/tools/perf/tests/event_groups.c
> @@ -0,0 +1,127 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <string.h>
> +#include <unistd.h>
> +#include <stdio.h>
> +#include "linux/perf_event.h"
> +#include "tests.h"
> +#include "debug.h"
> +#include "pmu.h"
> +#include "pmus.h"
> +#include "header.h"
> +#include "../perf-sys.h"
> +
> +/* hw: cycles, sw: context-switch, uncore: [arch dependent] */
> +static int types[] = {0, 1, -1};
> +static unsigned long configs[] = {0, 3, 0};
> +
> +#define NR_UNCORE_PMUS 5
> +
> +/* Uncore pmus that support more than 3 counters */
> +static struct uncore_pmus {
> +	const char *name;
> +	__u64 config;
> +} uncore_pmus[NR_UNCORE_PMUS] = {
> +	{ "amd_l3", 0x0 },
> +	{ "amd_df", 0x0 },
> +	{ "uncore_imc_0", 0x1 },         /* Intel */
> +	{ "core_imc", 0x318 },           /* PowerPC: core_imc/CPM_STCX_FIN/ */
> +	{ "hv_24x7", 0x22000000003 },    /* PowerPC: hv_24x7/CPM_STCX_FIN/ */
> +};
> +
> +static int event_open(int type, unsigned long config, int group_fd)
> +{
> +	struct perf_event_attr attr;
> +
> +	memset(&attr, 0, sizeof(struct perf_event_attr));
> +	attr.type = type;
> +	attr.size = sizeof(struct perf_event_attr);
> +	attr.config = config;
> +	/*
> +	 * When creating an event group, typically the group leader is
> +	 * initialized with disabled set to 1 and any child events are
> +	 * initialized with disabled set to 0. Despite disabled being 0,
> +	 * the child events will not start until the group leader is
> +	 * enabled.
> +	 */
> +	attr.disabled = group_fd == -1 ? 1 : 0;
> +
> +	return sys_perf_event_open(&attr, -1, 0, group_fd, 0);
> +}
> +
> +static int setup_uncore_event(void)
> +{
> +	struct perf_pmu *pmu;
> +	int i;
> +
> +	if (list_empty(&pmus))
> +		perf_pmu__scan(NULL);
> +
> +	perf_pmus__for_each_pmu(pmu) {
> +		for (i = 0; i < NR_UNCORE_PMUS; i++) {
> +			if (!strcmp(uncore_pmus[i].name, pmu->name)) {
> +				pr_debug("Using %s for uncore pmu event\n", pmu->name);
> +				types[2] = pmu->type;
> +				configs[2] = uncore_pmus[i].config;

Hi Ravi,

Observed failure while running the test on powerpc. It is because the uncore PMU ie hv_24x7 needs
performance monitoring to be enabled in powerpc. So to handle such cases, can we add an “event_open" check before
proceeding with the test. Below is the change on top of “tmp.perf/core” .


From 8b33fb900c26beafc28f75b6f64631f8fdd045c2 Mon Sep 17 00:00:00 2001
From: Athira Rajeev <atrajeev@...ux.vnet.ibm.com>
Date: Tue, 6 Dec 2022 20:17:25 +0530
Subject: [PATCH] perf test: Update event group check for support of uncore
 event

Event group test checks group creation for combinations of
hw, sw and uncore PMU events. Some of the uncore pmu event
requires performance enablement explicitly. Example, hv_24x7
event in powerpc. Hence add a check to see if event_open
succeeds before proceeding.

Fixes: 5c88101b797d ("perf test: Add event group test for events in multiple PMUs")
Signed-off-by: Athira Rajeev <atrajeev@...ux.vnet.ibm.com>
---
 tools/perf/tests/event_groups.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/perf/tests/event_groups.c b/tools/perf/tests/event_groups.c
index 612c0444aaa8..ad52e1da259a 100644
--- a/tools/perf/tests/event_groups.c
+++ b/tools/perf/tests/event_groups.c
@@ -51,7 +51,7 @@ static int event_open(int type, unsigned long config, int group_fd)
 static int setup_uncore_event(void)
 {
 	struct perf_pmu *pmu;
-	int i;
+	int i, fd;
 
 	if (list_empty(&pmus))
 		perf_pmu__scan(NULL);
@@ -62,6 +62,10 @@ static int setup_uncore_event(void)
 				pr_debug("Using %s for uncore pmu event\n", pmu->name);
 				types[2] = pmu->type;
 				configs[2] = uncore_pmus[i].config;
+				fd = event_open(types[2], configs[2], -1);
+				if (fd < 0)
+					return -1;
+				close(fd);
 				return 0;
 			}
 		}
-- 
2.31.1


Ravi, Please share your comments on this. If this looks fine, I can post this as separate fix patch.

Thanks
Athira
> +				return 0;
> +			}
> +		}
> +	}
> +	return -1;
> +}
> +
> +static int run_test(int i, int j, int k)
> +{
> +	int erroneous = ((((1 << i) | (1 << j) | (1 << k)) & 5) == 5);
> +	int group_fd, sibling_fd1, sibling_fd2;
> +
> +	group_fd = event_open(types[i], configs[i], -1);
> +	if (group_fd == -1)
> +		return -1;
> +
> +	sibling_fd1 = event_open(types[j], configs[j], group_fd);
> +	if (sibling_fd1 == -1) {
> +		close(group_fd);
> +		return erroneous ? 0 : -1;
> +	}
> +
> +	sibling_fd2 = event_open(types[k], configs[k], group_fd);
> +	if (sibling_fd2 == -1) {
> +		close(sibling_fd1);
> +		close(group_fd);
> +		return erroneous ? 0 : -1;
> +	}
> +
> +	close(sibling_fd2);
> +	close(sibling_fd1);
> +	close(group_fd);
> +	return erroneous ? -1 : 0;
> +}
> +
> +static int test__event_groups(struct test_suite *text __maybe_unused, int subtest __maybe_unused)
> +{
> +	int i, j, k;
> +	int ret;
> +	int r;
> +
> +	ret = setup_uncore_event();
> +	if (ret || types[2] == -1)
> +		return TEST_SKIP;
> +
> +	ret = TEST_OK;
> +	for (i = 0; i < 3; i++) {
> +		for (j = 0; j < 3; j++) {
> +			for (k = 0; k < 3; k++) {
> +				r = run_test(i, j, k);
> +				if (r)
> +					ret = TEST_FAIL;
> +
> +				pr_debug("0x%x 0x%lx, 0x%x 0x%lx, 0x%x 0x%lx: %s\n",
> +					 types[i], configs[i], types[j], configs[j],
> +					 types[k], configs[k], r ? "Fail" : "Pass");
> +			}
> +		}
> +	}
> +	return ret;
> +}
> +
> +DEFINE_SUITE("Event groups", event_groups);
> diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
> index e15f24cfc909..fb4b5ad4dd0f 100644
> --- a/tools/perf/tests/tests.h
> +++ b/tools/perf/tests/tests.h
> @@ -147,6 +147,7 @@ DECLARE_SUITE(expand_cgroup_events);
> DECLARE_SUITE(perf_time_to_tsc);
> DECLARE_SUITE(dlfilter);
> DECLARE_SUITE(sigtrap);
> +DECLARE_SUITE(event_groups);
> 
> /*
>  * PowerPC and S390 do not support creation of instruction breakpoints using the
> -- 
> 2.38.1
> 

Powered by blists - more mailing lists