Messages in this thread | | | Date | Wed, 29 Nov 2023 08:52:13 -0500 | Subject | Re: [PATCH] perf mem: Fix perf mem error on hybrid | From | "Liang, Kan" <> |
| |
On 2023-11-29 1:24 a.m., Ian Rogers wrote: > On Tue, Nov 28, 2023 at 12:39 PM <kan.liang@linux.intel.com> wrote: >> >> From: Kan Liang <kan.liang@linux.intel.com> >> >> The below error can be triggered on a hybrid machine. >> >> $ perf mem record -t load sleep 1 >> event syntax error: 'breakpoint/mem-loads,ldlat=30/P' >> \___ Bad event or PMU >> >> Unable to find PMU or event on a PMU of 'breakpoint' >> >> In the perf_mem_events__record_args(), the current perf never checks the >> availability of a mem event on a given PMU. All the PMUs will be added >> to the perf mem event list. Perf errors out for the unsupported PMU. >> >> Extend perf_mem_event__supported() and take a PMU into account. Check >> the mem event for each PMU before adding it to the perf mem event list. >> >> Optimize the perf_mem_events__init() a little bit. The function is to >> check whether the mem events are supported in the system. It doesn't >> need to scan all PMUs. Just return with the first supported PMU is good >> enough. >> >> Fixes: 5752c20f3787 ("perf mem: Scan all PMUs instead of just core ones") >> Reported-by: Ammy Yi <ammy.yi@intel.com> >> Tested-by: Ammy Yi <ammy.yi@intel.com> >> Signed-off-by: Kan Liang <kan.liang@linux.intel.com> >> --- >> tools/perf/util/mem-events.c | 25 ++++++++++++++----------- >> 1 file changed, 14 insertions(+), 11 deletions(-) >> >> diff --git a/tools/perf/util/mem-events.c b/tools/perf/util/mem-events.c >> index 954b235e12e5..3a2e3687878c 100644 >> --- a/tools/perf/util/mem-events.c >> +++ b/tools/perf/util/mem-events.c >> @@ -100,11 +100,14 @@ int perf_mem_events__parse(const char *str) >> return -1; >> } >> >> -static bool perf_mem_event__supported(const char *mnt, char *sysfs_name) >> +static bool perf_mem_event__supported(const char *mnt, struct perf_pmu *pmu, >> + struct perf_mem_event *e) >> { >> + char sysfs_name[100]; >> char path[PATH_MAX]; >> struct stat st; >> >> + scnprintf(sysfs_name, sizeof(sysfs_name), e->sysfs_name, pmu->name); > > Not sure if this is right. Looking at sysfs_name values: > https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/mem-events.c?h=perf-tools-next#n23 > "cpu/events/mem-loads" and "cpu/events/mem-stores", so won't pmu->name > never be used? > Is there a missed change to change the cpu to %s?
There is a X86 specific perf_mem_events__ptr(), which uses the "%s/mem-loads,ldlat=%u/P" and "%s/events/mem-loads" for Intel platforms. https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/arch/x86/util/mem-events.c?h=perf-tools-next#n20 The pmu->name is used especially for the hybrid platforms.
Thanks, Kan > > Thanks, > Ian > >> scnprintf(path, PATH_MAX, "%s/devices/%s", mnt, sysfs_name); >> return !stat(path, &st); >> } >> @@ -120,7 +123,6 @@ int perf_mem_events__init(void) >> >> for (j = 0; j < PERF_MEM_EVENTS__MAX; j++) { >> struct perf_mem_event *e = perf_mem_events__ptr(j); >> - char sysfs_name[100]; >> struct perf_pmu *pmu = NULL; >> >> /* >> @@ -136,12 +138,12 @@ int perf_mem_events__init(void) >> * of core PMU. >> */ >> while ((pmu = perf_pmus__scan(pmu)) != NULL) { >> - scnprintf(sysfs_name, sizeof(sysfs_name), e->sysfs_name, pmu->name); >> - e->supported |= perf_mem_event__supported(mnt, sysfs_name); >> + e->supported |= perf_mem_event__supported(mnt, pmu, e); >> + if (e->supported) { >> + found = true; >> + break; >> + } >> } >> - >> - if (e->supported) >> - found = true; >> } >> >> return found ? 0 : -ENOENT; >> @@ -167,13 +169,10 @@ static void perf_mem_events__print_unsupport_hybrid(struct perf_mem_event *e, >> int idx) >> { >> const char *mnt = sysfs__mount(); >> - char sysfs_name[100]; >> struct perf_pmu *pmu = NULL; >> >> while ((pmu = perf_pmus__scan(pmu)) != NULL) { >> - scnprintf(sysfs_name, sizeof(sysfs_name), e->sysfs_name, >> - pmu->name); >> - if (!perf_mem_event__supported(mnt, sysfs_name)) { >> + if (!perf_mem_event__supported(mnt, pmu, e)) { >> pr_err("failed: event '%s' not supported\n", >> perf_mem_events__name(idx, pmu->name)); >> } >> @@ -183,6 +182,7 @@ static void perf_mem_events__print_unsupport_hybrid(struct perf_mem_event *e, >> int perf_mem_events__record_args(const char **rec_argv, int *argv_nr, >> char **rec_tmp, int *tmp_nr) >> { >> + const char *mnt = sysfs__mount(); >> int i = *argv_nr, k = 0; >> struct perf_mem_event *e; >> >> @@ -211,6 +211,9 @@ int perf_mem_events__record_args(const char **rec_argv, int *argv_nr, >> while ((pmu = perf_pmus__scan(pmu)) != NULL) { >> const char *s = perf_mem_events__name(j, pmu->name); >> >> + if (!perf_mem_event__supported(mnt, pmu, e)) >> + continue; >> + >> rec_argv[i++] = "-e"; >> if (s) { >> char *copy = strdup(s); >> -- >> 2.35.1 >>
| |