linux-kernel - Re: [PATCH V3 11/14] perf tools: Fix perf_evlist_

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150915141808.GC2346@redhat.com>
Date:	Tue, 15 Sep 2015 11:18:08 -0300
From:	Arnaldo Carvalho de Melo <acme@...hat.com>
To:	Adrian Hunter <adrian.hunter@...el.com>
Cc:	Jiri Olsa <jolsa@...hat.com>, mingo@...nel.org,
	kan.liang@...el.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH V3 11/14] perf tools: Fix perf_evlist__add() not
 propagating maps

Em Tue, Sep 08, 2015 at 10:58:59AM +0300, Adrian Hunter escreveu:
> If evsels are added after maps are created, then they won't
> have any maps propagated to them.  Fix that.

Next time please consider, when introducing this a new function A() that
will be needed by a function B() in a subsequent patch to introduce A()
before B() if A() is static, i.e. to avoid making the patch making B()
use A() be larger than necessary by having to move A() to before B().

When reviewing I went, "oh, but this function was introduced already in
the previous patch?!" only to figure out that it was unchanged, just
being moved.

I did it this time, please see the two attached patches.

Thanks,

- Arnaldo
 
> Signed-off-by: Adrian Hunter <adrian.hunter@...el.com>
> ---
>  tools/perf/util/evlist.c | 56 +++++++++++++++++++++++++-----------------------
>  1 file changed, 29 insertions(+), 27 deletions(-)
> 
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index fcaabd1e5dbb..99267ab0d24a 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -124,6 +124,33 @@ void perf_evlist__delete(struct perf_evlist *evlist)
>  	free(evlist);
>  }
>  
> +static void __perf_evlist__propagate_maps(struct perf_evlist *evlist,
> +					  struct perf_evsel *evsel)
> +{
> +	/*
> +	 * We already have cpus for evsel (via PMU sysfs) so
> +	 * keep it, if there's no target cpu list defined.
> +	 */
> +	if (!evsel->own_cpus || evlist->has_user_cpus) {
> +		cpu_map__put(evsel->cpus);
> +		evsel->cpus = cpu_map__get(evlist->cpus);
> +	} else if (evsel->cpus != evsel->own_cpus) {
> +		cpu_map__put(evsel->cpus);
> +		evsel->cpus = cpu_map__get(evsel->own_cpus);
> +	}
> +
> +	thread_map__put(evsel->threads);
> +	evsel->threads = thread_map__get(evlist->threads);
> +}
> +
> +static void perf_evlist__propagate_maps(struct perf_evlist *evlist)
> +{
> +	struct perf_evsel *evsel;
> +
> +	evlist__for_each(evlist, evsel)
> +		__perf_evlist__propagate_maps(evlist, evsel);
> +}
> +
>  void perf_evlist__add(struct perf_evlist *evlist, struct perf_evsel *entry)
>  {
>  	entry->evlist = evlist;
> @@ -133,6 +160,8 @@ void perf_evlist__add(struct perf_evlist *evlist, struct perf_evsel *entry)
>  
>  	if (!evlist->nr_entries++)
>  		perf_evlist__set_id_pos(evlist);
> +
> +	__perf_evlist__propagate_maps(evlist, entry);
>  }
>  
>  void perf_evlist__splice_list_tail(struct perf_evlist *evlist,
> @@ -1102,33 +1131,6 @@ int perf_evlist__mmap(struct perf_evlist *evlist, unsigned int pages,
>  	return perf_evlist__mmap_ex(evlist, pages, overwrite, 0, false);
>  }
>  
> -static void __perf_evlist__propagate_maps(struct perf_evlist *evlist,
> -					  struct perf_evsel *evsel)
> -{
> -	/*
> -	 * We already have cpus for evsel (via PMU sysfs) so
> -	 * keep it, if there's no target cpu list defined.
> -	 */
> -	if (!evsel->own_cpus || evlist->has_user_cpus) {
> -		cpu_map__put(evsel->cpus);
> -		evsel->cpus = cpu_map__get(evlist->cpus);
> -	} else if (evsel->cpus != evsel->own_cpus) {
> -		cpu_map__put(evsel->cpus);
> -		evsel->cpus = cpu_map__get(evsel->own_cpus);
> -	}
> -
> -	thread_map__put(evsel->threads);
> -	evsel->threads = thread_map__get(evlist->threads);
> -}
> -
> -static void perf_evlist__propagate_maps(struct perf_evlist *evlist)
> -{
> -	struct perf_evsel *evsel;
> -
> -	evlist__for_each(evlist, evsel)
> -		__perf_evlist__propagate_maps(evlist, evsel);
> -}
> -
>  int perf_evlist__create_maps(struct perf_evlist *evlist, struct target *target)
>  {
>  	struct cpu_map *cpus;
> -- 
> 1.9.1

View attachment "0001-perf-evlist-Factor-out-a-function-to-propagate-maps-.patch" of type "text/plain" (2898 bytes)

View attachment "0002-perf-evlist-Fix-add-not-propagating-maps.patch" of type "text/plain" (1263 bytes)