Messages in this thread | | | Date | Mon, 24 May 2021 19:19:25 +0200 | From | Jiri Olsa <> | Subject | Re: [PATCH v1 3/5] perf tools: Check if mem_events is supported for hybrid |
| |
On Thu, May 20, 2021 at 03:00:38PM +0800, Jin Yao wrote: > Check if the mem_events ('mem-loads' and 'mem-stores') exist > in the sysfs path. > > For Alderlake, the hybrid cpu pmu are "cpu_core" and "cpu_atom". > Check the existing of following paths: > /sys/devices/cpu_atom/events/mem-loads > /sys/devices/cpu_atom/events/mem-stores > /sys/devices/cpu_core/events/mem-loads > /sys/devices/cpu_core/events/mem-stores > > If the patch exists, the mem_event is supported. > > Signed-off-by: Jin Yao <yao.jin@linux.intel.com> > --- > tools/perf/util/mem-events.c | 43 +++++++++++++++++++++++++++++------- > 1 file changed, 35 insertions(+), 8 deletions(-) > > diff --git a/tools/perf/util/mem-events.c b/tools/perf/util/mem-events.c > index c736eaded06c..e8f6e745eaf0 100644 > --- a/tools/perf/util/mem-events.c > +++ b/tools/perf/util/mem-events.c > @@ -12,14 +12,16 @@ > #include "mem-events.h" > #include "debug.h" > #include "symbol.h" > +#include "pmu.h" > +#include "pmu-hybrid.h" > > unsigned int perf_mem_events__loads_ldlat = 30; > > #define E(t, n, s) { .tag = t, .name = n, .sysfs_name = s } > > static struct perf_mem_event perf_mem_events[PERF_MEM_EVENTS__MAX] = { > - E("ldlat-loads", "cpu/mem-loads,ldlat=%u/P", "cpu/events/mem-loads"), > - E("ldlat-stores", "cpu/mem-stores/P", "cpu/events/mem-stores"), > + E("ldlat-loads", "%s/mem-loads,ldlat=%u/P", "%s/events/mem-loads"), > + E("ldlat-stores", "%s/mem-stores/P", "%s/events/mem-stores"), > E(NULL, NULL, NULL),
so this was generic place, now it's x86 specific, I wonder we should move it under arch/x86 to avoid confusion
> }; > #undef E > @@ -100,6 +102,18 @@ int perf_mem_events__parse(const char *str) > return -1; > } > > +static bool perf_mem_events__supported(const char *mnt, char *sysfs_name) > +{ > + char path[PATH_MAX]; > + struct stat st; > + > + scnprintf(path, PATH_MAX, "%s/devices/%s", mnt, sysfs_name); > + if (!stat(path, &st)) > + return true; > + > + return false;
could be just 'return !stat(path, &st);' right?
> +} > + > int perf_mem_events__init(void) > { > const char *mnt = sysfs__mount(); > @@ -110,9 +124,10 @@ int perf_mem_events__init(void) > return -ENOENT; > > for (j = 0; j < PERF_MEM_EVENTS__MAX; j++) { > - char path[PATH_MAX]; > struct perf_mem_event *e = perf_mem_events__ptr(j); > - struct stat st; > + struct perf_pmu *pmu; > + char sysfs_name[100]; > + int unsupported = 0; > > /* > * If the event entry isn't valid, skip initialization > @@ -121,11 +136,23 @@ int perf_mem_events__init(void) > if (!e->tag) > continue; > > - scnprintf(path, PATH_MAX, "%s/devices/%s", > - mnt, e->sysfs_name); > + if (!perf_pmu__has_hybrid()) { > + scnprintf(sysfs_name, sizeof(sysfs_name), > + e->sysfs_name, "cpu"); > + e->supported = perf_mem_events__supported(mnt, sysfs_name); > + } else { > + perf_pmu__for_each_hybrid_pmu(pmu) { > + scnprintf(sysfs_name, sizeof(sysfs_name), > + e->sysfs_name, pmu->name); > + if (!perf_mem_events__supported(mnt, sysfs_name)) > + unsupported++; > + } > + > + e->supported = (unsupported == 0) ? true : false;
could you just do in the above loop: e->supported |= perf_mem_events__supported(mnt, sysfs_name);
jirka
> + } > > - if (!stat(path, &st)) > - e->supported = found = true; > + if (e->supported) > + found = true; > } > > return found ? 0 : -ENOENT; > -- > 2.17.1 >
| |