Re: [PATCH v3 2/2] perf test: Add event group test

[Madhavan Srinivasan]

On 12/7/22 9:56 AM, Ravi Bangoria wrote:
> Hi Athira,
>
> > > +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@xxxxxxxxxxxxxxxxxx>
> > 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.
> You need to open an event to activate hv_24x7 pmu?
hv_24x7 pmu supports events which can also provide system-wide resource data
and partition should have permissions to access those, hence the check.

Maddy
> > 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@xxxxxxxxxxxxxxxxxx>
> > ---
> >   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;
> Sure. Just add a comment here to explain why are we opening a
> standalone event here.
>
> > +				fd = event_open(types[2], configs[2], -1);
> > +				if (fd < 0)
> > +					return -1;
> > +				close(fd);
> >   				return 0;
> >   			}
> >   		}
> Thanks,
> Ravi