linux-kernel - Re: [RFC PATCH 3/3] perf: util: only open events on CPUs an evsel permits

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]

Message-ID: <20160708075514.GA26532@krava>
Date:	Fri, 8 Jul 2016 09:55:14 +0200
From:	Jiri Olsa <jolsa@...hat.com>
To:	Mark Rutland <mark.rutland@....com>
Cc:	linux-kernel@...r.kernel.org, acme@...nel.org,
	adrian.hunter@...el.com, alexander.shishkin@...ux.intel.com,
	hekuang@...wei.com, jolsa@...nel.org, kan.liang@...el.com,
	mingo@...hat.com, peterz@...radead.org, wangnan0@...wei.com
Subject: Re: [RFC PATCH 3/3] perf: util: only open events on CPUs an evsel
 permits

On Thu, Jul 07, 2016 at 05:04:34PM +0100, Mark Rutland wrote:
> In systems with heterogeneous CPU PMUs, it's possible for each evsel to
> cover a distinct set of CPUs, and hence the cpu_map associated with each
> evsel may have a distinct idx<->id mapping. Any of these may be distinct from
> the evlist's cpu map.
> 
> Events can be tied to the same fd so long as they use the same per-cpu
> ringbuffer (i.e. so long as they are on the same CPU). To acquire the
> correct FDs, we must compare the Linux logical IDs rather than the evsel
> or evlist indices.
> 
> This path adds logic to perf_evlist__mmap_per_evsel to handle this,
> translating IDs as required. As PMUs may cover a subset of CPUs from the
> evlist, we skip the CPUs a PMU cannot handle.
> 
> Signed-off-by: Mark Rutland <mark.rutland@....com>
> Cc: Adrian Hunter <adrian.hunter@...el.com>
> Cc: Alexander Shishkin <alexander.shishkin@...ux.intel.com>
> Cc: Arnaldo Carvalho de Melo <acme@...nel.org>
> Cc: He Kuang <hekuang@...wei.com>
> Cc: Ingo Molnar <mingo@...hat.com>
> Cc: Jiri Olsa <jolsa@...nel.org>
> Cc: Peter Zijlstra <peterz@...radead.org>
> Cc: Wang Nan <wangnan0@...wei.com>
> Cc: linux-kernel@...r.kernel.org
> ---
>  tools/perf/util/evlist.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index e82ba90..0b5b1be 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -984,17 +984,24 @@ static int __perf_evlist__mmap(struct perf_evlist *evlist, int idx,
>  }
>  
>  static int perf_evlist__mmap_per_evsel(struct perf_evlist *evlist, int idx,
> -				       struct mmap_params *mp, int cpu,
> +				       struct mmap_params *mp, int cpu_idx,
>  				       int thread, int *output)
>  {
>  	struct perf_evsel *evsel;
> +	int evlist_cpu = cpu_map__cpu(evlist->cpus, cpu_idx);
>  
>  	evlist__for_each(evlist, evsel) {
>  		int fd;
> +		int cpu;
>  
>  		if (evsel->system_wide && thread)
>  			continue;
>  
> +		if (!cpu_map__has(evsel->cpus, evlist_cpu))
> +			continue;
> +
> +		cpu = cpu_map__idx(evsel->cpus, evlist_cpu);

you basicaly call cpu_map__idx twice in here,
I think it might be better call it just once
and check the cpu for -1

jirka