lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
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

Powered by Openwall GNU/*/Linux Powered by OpenVZ