lkml.org 
[lkml]   [2015]   [Jul]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH V2 2/3] perf,tools: check and re-organize evsel cpu maps
Date
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 <kan.liang@intel.com>
>
> 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 <kan.liang@intel.com>
> ---
>
> 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



\
 
 \ /
  Last update: 2015-07-15 23:41    [W:2.049 / U:0.016 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site