Messages in this thread | | | Date | Thu, 1 Dec 2022 14:43:26 +0530 | Subject | Re: [PATCH v2 2/2] perf test: Add event group test | From | Ravi Bangoria <> |
| |
Hi Kan,
Thanks for the review.
>> +static int setup_uncore_event(void) >> +{ >> + struct perf_pmu *pmu; >> + >> + if (list_empty(&pmus)) >> + perf_pmu__scan(NULL); >> + >> + perf_pmus__for_each_pmu(pmu) { >> + if (pmu->is_uncore) { > > Always using the first uncore PMU may trigger false alarm on some Intel > platforms. For example, Intel has free running uncore PMUs (e.g., > uncore_imc_free_running_0), which only supports special event encoding > 0xff. The config 0 must fails. > You may want to add the below check to ignore the free running uncore PMUs. > if (strstr(pmu->name, "free_running")) > continue; > > > Also, some uncore PMUs only support two counters. But the test assumes > that the number of counters > 2. You may want to limit the size of the > group for 2 for a pure uncore group.
That seems hacky. Instead of ignoring, would it be possible to provide a list of testable pmus? Example with random values:
/* Uncore pmus that support more than 3 counters */ static struct uncore_pmus { char *name; unsigned long config; } uncore_pmus[] = { { "amd_l3", 0x0 }, { "amd_df", 0x0 }, { "uncore_imc_xxx", 0xff }, /* Intel */ { "intel_xxx_pmu2", 0xff }, /* Intel */ { "abc_pmu1", 0x0 }, /* Arm */ { "hv_24x7", 0xa }, /* PowerPC */ { ... }, };
perf_pmus__for_each_pmu(pmu) { if (pmu present in uncore_pmus[]) type[2] = pmu->type; config[2] = pmu->config; }
Ofcourse, this should work for all architectures. Arm, PowerPC, s390 folks?
Thanks, Ravi
| |