Hi Stephane, Any comments about this patch?
Currently, perf stat -a and -C don't have consistent results for per-socket uncore event. With -a and --per-core, only first cpu of each socket show results. Other cpus show "<not counted>". But with -C and --per-core, all cpus have a result. That's not correct. This patch only count the CPU in the cpumask and show "<not counted>" for other CPUs. It only impacts -C. Thanks, Kan > > From: Kan Liang <[email protected]> > > Some PMU events have cpumask, e.g uncore events. The cpu list set by > user may be incompatible with event's cpumask. > This patch will check the user defined cpu list. If the incompatible cpu is > found, it will warn the user and discard the incompatible cpu. Only available > cpu can be stored in evsel->cpus->map. If there is no cpu from cpu list > compatible with event's cpumask. It will error out. > > Here is an example. > According to cpumask, uncore should be default on CPU0 and CPU18. > So the S0-C1 for uncore should show "<not counted>". > > Without this patch > $ sudo ./perf stat -e cycles,uncore_imc_0/cas_count_read/ -C0,1,18 --per- > core sleep 2 > > Performance counter stats for 'CPU(s) 0,1,18': > > S0-C0 1 6749638 cycles > S0-C0 1 0.83 MiB uncore_imc_0/cas_count_read/ > (100.00%) > S0-C1 1 232421 cycles > S0-C1 1 0.83 MiB uncore_imc_0/cas_count_read/ > S1-C0 1 236997 cycles > S1-C0 1 0.35 MiB uncore_imc_0/cas_count_read/ > > 2.001094019 seconds time elapsed > > With this patch > $ perf stat -e cycles,uncore_imc_0/cas_count_read/ -C0,1,18 --per-core > sleep 2 event uncore_imc_0/cas_count_read/ can only be monitored on > CPU 0 18. > Other CPUs will be discard. > > Performance counter stats for 'CPU(s) 0,1,18': > > S0-C0 1 5557406 cycles > S0-C0 1 0.21 MiB uncore_imc_0/cas_count_read/ > S0-C1 1 1012534 cycles > S0-C1 0 <not counted> MiB uncore_imc_0/cas_count_read/ > S1-C0 1 916130 cycles > S1-C0 1 0.08 MiB uncore_imc_0/cas_count_read/ > > 2.001110843 seconds time elapsed > > Signed-off-by: Kan Liang <[email protected]> > --- > > Changes since V1 > - Re-implement the way to find the common cpus > - Add test case > > tools/perf/util/evlist.c | 72 > ++++++++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 67 insertions(+), 5 deletions(-) > > diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c index > 6cfdee6..d67414f 100644 > --- a/tools/perf/util/evlist.c > +++ b/tools/perf/util/evlist.c > @@ -1101,6 +1101,66 @@ int perf_evlist__mmap(struct perf_evlist *evlist, > unsigned int pages, > return perf_evlist__mmap_ex(evlist, pages, overwrite, 0, false); } > > +static int cmp_ids(const void *a, const void *b) { > + return *(int *)a - *(int *)b; > +} > + > +static int perf_evlist__check_evsel_cpus(struct perf_evlist *evlist, > +struct perf_evsel *evsel) { > + const int ncpus = cpu_map__nr(evlist->cpus); > + int i = 0, j = 0, cpu_nr = 0, tmp; > + > + /* ensure we process id in increasing order */ > + qsort(evlist->cpus->map, evlist->cpus->nr, sizeof(int), cmp_ids); > + > + /* find the common cpus between evsel and evlist. */ > + while ((i < cpu_map__nr(evlist->cpus)) && > + (j < cpu_map__nr(evsel->cpus))) { > + > + if (evlist->cpus->map[i] == evsel->cpus->map[j]) { > + cpu_nr++; > + i++; > + j++; > + } else if (evlist->cpus->map[i] > evsel->cpus->map[j]) { > + evsel->cpus->map[j++] = -1; > + } else > + i++; > + > + } > + while (j < cpu_map__nr(evsel->cpus)) > + evsel->cpus->map[j++] = -1; > + > + if (cpu_nr == 0) { > + pr_warning("event %s cannot be monitored on the given > cpus." > + "Please check cpumask\n", evsel->name); > + return -1; > + } > + > + if (ncpus > cpu_nr) > + pr_warning("event %s can only be monitored on CPU", > evsel->name); > + > + /* order evsel cpus */ > + for (i = 0, tmp = 0; i < cpu_nr; i++) { > + if (evsel->cpus->map[i] == -1) { > + while (evsel->cpus->map[tmp] == -1) { > + tmp++; > + BUG_ON(tmp >= cpu_map__nr(evsel- > >cpus)); > + } > + evsel->cpus->map[i] = evsel->cpus->map[tmp]; > + evsel->cpus->map[tmp] = -1; > + } > + if (ncpus > cpu_nr) > + pr_warning(" %d", evsel->cpus->map[i]); > + tmp++; > + } > + evsel->cpus->nr = cpu_nr; > + if (ncpus > cpu_nr) > + pr_warning(". Other CPUs will be discard.\n"); > + > + return 0; > +} > + > static int perf_evlist__propagate_maps(struct perf_evlist *evlist, > struct target *target) > { > @@ -1108,13 +1168,15 @@ static int perf_evlist__propagate_maps(struct > perf_evlist *evlist, > > evlist__for_each(evlist, evsel) { > /* > - * We already have cpus for evsel (via PMU sysfs) so > - * keep it, if there's no target cpu list defined. > + * We already have cpus for evsel (via PMU sysfs) > + * and target cpu list defined, check if they are > + * compatible. If not, discard incompatible cpus. > */ > - if (evsel->cpus && target->cpu_list) > - cpu_map__put(evsel->cpus); > + if (evsel->cpus && target->cpu_list && > + perf_evlist__check_evsel_cpus(evlist, evsel)) > + return -EINVAL; > > - if (!evsel->cpus || target->cpu_list) > + if (!evsel->cpus) > evsel->cpus = cpu_map__get(evlist->cpus); > > evsel->threads = thread_map__get(evlist->threads); > -- > 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
