Re: [PATCH 38/79] libperf: Add perf_evlist__init function
From: Arnaldo Carvalho de Melo
Date: Mon Jul 22 2019 - 15:39:33 EST
Em Sun, Jul 21, 2019 at 01:24:25PM +0200, Jiri Olsa escreveu:
> Adding perf_evlist__init function to initialize
> perf_evlist struct.
>
> Link: http://lkml.kernel.org/n/tip-uhs894b98iiydutjgr1z5t8h@xxxxxxxxxxxxxx
> Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> ---
> tools/perf/lib/evlist.c | 5 +++++
> tools/perf/lib/include/perf/evlist.h | 4 ++++
> tools/perf/lib/libperf.map | 1 +
> tools/perf/util/evlist.c | 3 ++-
> 4 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/lib/evlist.c b/tools/perf/lib/evlist.c
> index 646bdd518793..fdc8c1894b37 100644
> --- a/tools/perf/lib/evlist.c
> +++ b/tools/perf/lib/evlist.c
> @@ -2,3 +2,8 @@
> #include <perf/evlist.h>
> #include <linux/list.h>
> #include <internal/evlist.h>
> +
> +void perf_evlist__init(struct perf_evlist *evlist)
> +{
> + INIT_LIST_HEAD(&evlist->entries);
> +}
> diff --git a/tools/perf/lib/include/perf/evlist.h b/tools/perf/lib/include/perf/evlist.h
> index 92b0eb39caec..1ddfcca0bd01 100644
> --- a/tools/perf/lib/include/perf/evlist.h
> +++ b/tools/perf/lib/include/perf/evlist.h
> @@ -2,6 +2,10 @@
> #ifndef __LIBPERF_EVLIST_H
> #define __LIBPERF_EVLIST_H
>
> +#include <perf/core.h>
> +
> struct perf_evlist;
>
> +LIBPERF_API void perf_evlist__init(struct perf_evlist *evlist);
> +
> #endif /* __LIBPERF_EVLIST_H */
> diff --git a/tools/perf/lib/libperf.map b/tools/perf/lib/libperf.map
> index 54f8503c6d82..5ca6ff6fcdfa 100644
> --- a/tools/perf/lib/libperf.map
> +++ b/tools/perf/lib/libperf.map
> @@ -10,6 +10,7 @@ LIBPERF_0.0.1 {
> perf_thread_map__get;
> perf_thread_map__put;
> perf_evsel__init;
> + perf_evlist__init;
> local:
> *;
> };
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index faf3ffd81d4c..aacddd9b2d64 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -33,6 +33,7 @@
> #include <linux/log2.h>
> #include <linux/err.h>
> #include <linux/zalloc.h>
> +#include <perf/evlist.h>
>
> #ifdef LACKS_SIGQUEUE_PROTOTYPE
> int sigqueue(pid_t pid, int sig, const union sigval value);
> @@ -48,11 +49,11 @@ void evlist__init(struct evlist *evlist, struct perf_cpu_map *cpus,
>
> for (i = 0; i < PERF_EVLIST__HLIST_SIZE; ++i)
> INIT_HLIST_HEAD(&evlist->heads[i]);
> - INIT_LIST_HEAD(&evlist->core.entries);
> perf_evlist__set_maps(evlist, cpus, threads);
> fdarray__init(&evlist->pollfd, 64);
> evlist->workload.pid = -1;
> evlist->bkw_mmap_state = BKW_MMAP_NOTREADY;
> + perf_evlist__init(&evlist->core);
> }
This causes a segfault, since perf_evlist__set_maps() will touch stuff
that you now moved to the end of this function, applying the patch at
the end of this message fixes the issue, i.e. keeps the existing init
ordering and avoids the segfault:
gdb) run stat sleep 1
Starting program: /root/bin/perf stat sleep 1
Program received signal SIGSEGV, Segmentation fault.
0x00000000004f6b55 in __perf_evlist__propagate_maps (evlist=0xbb34c0, evsel=0x0) at util/evlist.c:161
161 if (!evsel->own_cpus || evlist->has_user_cpus) {
Missing separate debuginfos, use: dnf debuginfo-install bzip2-libs-1.0.6-29.fc30.x86_64 elfutils-libelf-0.176-3.fc30.x86_64 elfutils-libs-0.176-3.fc30.x86_64 glib2-2.60.4-1.fc30.x86_64 libbabeltrace-1.5.6-2.fc30.x86_64 libgcc-9.1.1-1.fc30.x86_64 libunwind-1.3.1-2.fc30.x86_64 libuuid-2.33.2-1.fc30.x86_64 libxcrypt-4.4.6-2.fc30.x86_64 libzstd-1.4.0-1.fc30.x86_64 numactl-libs-2.0.12-2.fc30.x86_64 pcre-8.43-2.fc30.x86_64 perl-libs-5.28.2-436.fc30.x86_64 popt-1.16-17.fc30.x86_64 python2-libs-2.7.16-2.fc30.x86_64 slang-2.3.2-5.fc30.x86_64 xz-libs-5.2.4-5.fc30.x86_64 zlib-1.2.11-15.fc30.x86_64
(gdb) bt
#0 0x00000000004f6b55 in __perf_evlist__propagate_maps (evlist=0xbb34c0, evsel=0x0) at util/evlist.c:161
#1 0x00000000004f6c7a in perf_evlist__propagate_maps (evlist=0xbb34c0) at util/evlist.c:178
#2 0x00000000004f955e in perf_evlist__set_maps (evlist=0xbb34c0, cpus=0x0, threads=0x0) at util/evlist.c:1128
#3 0x00000000004f66f8 in evlist__init (evlist=0xbb34c0, cpus=0x0, threads=0x0) at util/evlist.c:52
#4 0x00000000004f6790 in evlist__new () at util/evlist.c:64
#5 0x0000000000456071 in cmd_stat (argc=3, argv=0x7fffffffd670) at builtin-stat.c:1705
#6 0x00000000004dd0fa in run_builtin (p=0xa21e00 <commands+288>, argc=3, argv=0x7fffffffd670) at perf.c:304
#7 0x00000000004dd367 in handle_internal_command (argc=3, argv=0x7fffffffd670) at perf.c:356
#8 0x00000000004dd4ae in run_argv (argcp=0x7fffffffd4cc, argv=0x7fffffffd4c0) at perf.c:400
#9 0x00000000004dd81a in main (argc=3, argv=0x7fffffffd670) at perf.c:522
(gdb) bt
I'm applying it to fix this issue and avoid a bisection break. I'm now
going to run 'perf test' after each cset too. And probably the next cset
has this issue as well, i.e. reordering of initialization in the
perf_evsel__init() case.
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index aacddd9b2d64..f4aa6cf80559 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -49,11 +49,11 @@ void evlist__init(struct evlist *evlist, struct perf_cpu_map *cpus,
for (i = 0; i < PERF_EVLIST__HLIST_SIZE; ++i)
INIT_HLIST_HEAD(&evlist->heads[i]);
+ perf_evlist__init(&evlist->core);
perf_evlist__set_maps(evlist, cpus, threads);
fdarray__init(&evlist->pollfd, 64);
evlist->workload.pid = -1;
evlist->bkw_mmap_state = BKW_MMAP_NOTREADY;
- perf_evlist__init(&evlist->core);
}
struct evlist *evlist__new(void)