Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1753756AbbGOVOr (ORCPT <rfc822;w@1wt.eu>);
	Wed, 15 Jul 2015 17:14:47 -0400
Received: from mga02.intel.com ([134.134.136.20]:63612 "EHLO mga02.intel.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1753428AbbGOVOq convert rfc822-to-8bit (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Wed, 15 Jul 2015 17:14:46 -0400
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.15,482,1432623600"; 
   d="scan'208";a="763317432"
From: "Liang, Kan" <kan.liang@intel.com>
To: "eranian@google.com" <eranian@google.com>,
        "acme@kernel.org" <acme@kernel.org>,
        "jolsa@kernel.org" <jolsa@kernel.org>
CC: "ak@linux.intel.com" <ak@linux.intel.com>,
        "namhyung@kernel.org" <namhyung@kernel.org>,
        "Hunter, Adrian" <adrian.hunter@intel.com>,
        "dsahern@gmail.com" <dsahern@gmail.com>,
        "a.p.zijlstra@chello.nl" <a.p.zijlstra@chello.nl>,
        "mingo@redhat.com" <mingo@redhat.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH V2 2/3] perf,tools: check and re-organize evsel cpu maps
Thread-Topic: [PATCH V2 2/3] perf,tools: check and re-organize evsel cpu maps
Thread-Index: AQHQtNKRzd3UJ+IGbE2pZ0XQqX04SJ3dGGQA
Date: Wed, 15 Jul 2015 21:14:37 +0000
Message-ID: <37D7C6CF3E00A74B8858931C1DB2F0770188D0FB@SHSMSX103.ccr.corp.intel.com>
References: <1435820925-51091-1-git-send-email-kan.liang@intel.com>
 <1435820925-51091-2-git-send-email-kan.liang@intel.com>
In-Reply-To: <1435820925-51091-2-git-send-email-kan.liang@intel.com>
Accept-Language: zh-CN, en-US
Content-Language: en-US
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
x-originating-ip: [10.239.127.40]
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 8BIT
MIME-Version: 1.0
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 5793
Lines: 175

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/