[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <37D7C6CF3E00A74B8858931C1DB2F0770188D0FB@SHSMSX103.ccr.corp.intel.com>
Date: Wed, 15 Jul 2015 21:14:37 +0000
From: "Liang, Kan" <kan.liang@...el.com>
To: "eranian@...gle.com" <eranian@...gle.com>,
"acme@...nel.org" <acme@...nel.org>,
"jolsa@...nel.org" <jolsa@...nel.org>
CC: "ak@...ux.intel.com" <ak@...ux.intel.com>,
"namhyung@...nel.org" <namhyung@...nel.org>,
"Hunter, Adrian" <adrian.hunter@...el.com>,
"dsahern@...il.com" <dsahern@...il.com>,
"a.p.zijlstra@...llo.nl" <a.p.zijlstra@...llo.nl>,
"mingo@...hat.com" <mingo@...hat.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH V2 2/3] perf,tools: check and re-organize evsel cpu maps
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@...el.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@...el.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists