* [PATCH v2 1/2] libperf: Add perf_evlist__reset_id_hash()
@ 2021-02-25 3:51 Namhyung Kim
2021-02-25 3:51 ` [PATCH v2 2/2] perf stat: Fix use-after-free when -r option is used Namhyung Kim
2021-02-25 16:12 ` [PATCH v2 1/2] libperf: Add perf_evlist__reset_id_hash() Jiri Olsa
0 siblings, 2 replies; 4+ messages in thread
From: Namhyung Kim @ 2021-02-25 3:51 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa
Cc: Ingo Molnar, Peter Zijlstra, Mark Rutland, Alexander Shishkin,
LKML, Stephane Eranian, Ian Rogers
Add the perf_evlist__reset_id_hash() function as an internal function
so that it can be called by perf to reset the hash table. This is
necessary for perf stat to run the workload multiple times.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/lib/perf/evlist.c | 13 +++++++++----
tools/lib/perf/include/internal/evlist.h | 2 ++
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/tools/lib/perf/evlist.c b/tools/lib/perf/evlist.c
index 17465d454a0e..a0aaf385cbb5 100644
--- a/tools/lib/perf/evlist.c
+++ b/tools/lib/perf/evlist.c
@@ -26,13 +26,10 @@
void perf_evlist__init(struct perf_evlist *evlist)
{
- int i;
-
- for (i = 0; i < PERF_EVLIST__HLIST_SIZE; ++i)
- INIT_HLIST_HEAD(&evlist->heads[i]);
INIT_LIST_HEAD(&evlist->entries);
evlist->nr_entries = 0;
fdarray__init(&evlist->pollfd, 64);
+ perf_evlist__reset_id_hash(evlist);
}
static void __perf_evlist__propagate_maps(struct perf_evlist *evlist,
@@ -237,6 +234,14 @@ static void perf_evlist__id_hash(struct perf_evlist *evlist,
hlist_add_head(&sid->node, &evlist->heads[hash]);
}
+void perf_evlist__reset_id_hash(struct perf_evlist *evlist)
+{
+ int i;
+
+ for (i = 0; i < PERF_EVLIST__HLIST_SIZE; ++i)
+ INIT_HLIST_HEAD(&evlist->heads[i]);
+}
+
void perf_evlist__id_add(struct perf_evlist *evlist,
struct perf_evsel *evsel,
int cpu, int thread, u64 id)
diff --git a/tools/lib/perf/include/internal/evlist.h b/tools/lib/perf/include/internal/evlist.h
index 2d0fa02b036f..212c29063ad4 100644
--- a/tools/lib/perf/include/internal/evlist.h
+++ b/tools/lib/perf/include/internal/evlist.h
@@ -124,4 +124,6 @@ int perf_evlist__id_add_fd(struct perf_evlist *evlist,
struct perf_evsel *evsel,
int cpu, int thread, int fd);
+void perf_evlist__reset_id_hash(struct perf_evlist *evlist);
+
#endif /* __LIBPERF_INTERNAL_EVLIST_H */
--
2.30.0.617.g56c4b15f3c-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH v2 2/2] perf stat: Fix use-after-free when -r option is used
2021-02-25 3:51 [PATCH v2 1/2] libperf: Add perf_evlist__reset_id_hash() Namhyung Kim
@ 2021-02-25 3:51 ` Namhyung Kim
2021-02-25 16:12 ` [PATCH v2 1/2] libperf: Add perf_evlist__reset_id_hash() Jiri Olsa
1 sibling, 0 replies; 4+ messages in thread
From: Namhyung Kim @ 2021-02-25 3:51 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa
Cc: Ingo Molnar, Peter Zijlstra, Mark Rutland, Alexander Shishkin,
LKML, Stephane Eranian, Ian Rogers
I got a segfault when using -r option with event groups. The option
makes it run the workload multiple times and it will reuse the evlist
and evsel for each run.
While most of resources are allocated and freed properly, the id hash
in the evlist was not and it resulted in the bug. You can see it with
the address sanitizer like below:
$ perf stat -r 100 -e '{cycles,instructions}' true
=================================================================
==693052==ERROR: AddressSanitizer: heap-use-after-free on
address 0x6080000003d0 at pc 0x558c57732835 bp 0x7fff1526adb0 sp 0x7fff1526ada8
WRITE of size 8 at 0x6080000003d0 thread T0
#0 0x558c57732834 in hlist_add_head /home/namhyung/project/linux/tools/include/linux/list.h:644
#1 0x558c57732834 in perf_evlist__id_hash /home/namhyung/project/linux/tools/lib/perf/evlist.c:237
#2 0x558c57732834 in perf_evlist__id_add /home/namhyung/project/linux/tools/lib/perf/evlist.c:244
#3 0x558c57732834 in perf_evlist__id_add_fd /home/namhyung/project/linux/tools/lib/perf/evlist.c:285
#4 0x558c5747733e in store_evsel_ids util/evsel.c:2765
#5 0x558c5747733e in evsel__store_ids util/evsel.c:2782
#6 0x558c5730b717 in __run_perf_stat /home/namhyung/project/linux/tools/perf/builtin-stat.c:895
#7 0x558c5730b717 in run_perf_stat /home/namhyung/project/linux/tools/perf/builtin-stat.c:1014
#8 0x558c5730b717 in cmd_stat /home/namhyung/project/linux/tools/perf/builtin-stat.c:2446
#9 0x558c57427c24 in run_builtin /home/namhyung/project/linux/tools/perf/perf.c:313
#10 0x558c572b1a48 in handle_internal_command /home/namhyung/project/linux/tools/perf/perf.c:365
#11 0x558c572b1a48 in run_argv /home/namhyung/project/linux/tools/perf/perf.c:409
#12 0x558c572b1a48 in main /home/namhyung/project/linux/tools/perf/perf.c:539
#13 0x7fcadb9f7d09 in __libc_start_main ../csu/libc-start.c:308
#14 0x558c572b60f9 in _start (/home/namhyung/project/linux/tools/perf/perf+0x45d0f9)
Actually the nodes in the hash table are struct perf_stream_id and
they were freed in the previous run. Fix it by resetting the hash.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/util/evlist.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 5121b4db66fe..882cd1f721d9 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -1306,6 +1306,7 @@ void evlist__close(struct evlist *evlist)
perf_evsel__free_fd(&evsel->core);
perf_evsel__free_id(&evsel->core);
}
+ perf_evlist__reset_id_hash(&evlist->core);
}
static int evlist__create_syswide_maps(struct evlist *evlist)
--
2.30.0.617.g56c4b15f3c-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v2 1/2] libperf: Add perf_evlist__reset_id_hash()
2021-02-25 3:51 [PATCH v2 1/2] libperf: Add perf_evlist__reset_id_hash() Namhyung Kim
2021-02-25 3:51 ` [PATCH v2 2/2] perf stat: Fix use-after-free when -r option is used Namhyung Kim
@ 2021-02-25 16:12 ` Jiri Olsa
2021-03-03 15:51 ` Arnaldo Carvalho de Melo
1 sibling, 1 reply; 4+ messages in thread
From: Jiri Olsa @ 2021-02-25 16:12 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra,
Mark Rutland, Alexander Shishkin, LKML, Stephane Eranian,
Ian Rogers
On Thu, Feb 25, 2021 at 12:51:47PM +0900, Namhyung Kim wrote:
> Add the perf_evlist__reset_id_hash() function as an internal function
> so that it can be called by perf to reset the hash table. This is
> necessary for perf stat to run the workload multiple times.
>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
for the patchset
Acked-by: Jiri Olsa <jolsa@redhat.com>
thanks,
jirka
> ---
> tools/lib/perf/evlist.c | 13 +++++++++----
> tools/lib/perf/include/internal/evlist.h | 2 ++
> 2 files changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/tools/lib/perf/evlist.c b/tools/lib/perf/evlist.c
> index 17465d454a0e..a0aaf385cbb5 100644
> --- a/tools/lib/perf/evlist.c
> +++ b/tools/lib/perf/evlist.c
> @@ -26,13 +26,10 @@
>
> void perf_evlist__init(struct perf_evlist *evlist)
> {
> - int i;
> -
> - for (i = 0; i < PERF_EVLIST__HLIST_SIZE; ++i)
> - INIT_HLIST_HEAD(&evlist->heads[i]);
> INIT_LIST_HEAD(&evlist->entries);
> evlist->nr_entries = 0;
> fdarray__init(&evlist->pollfd, 64);
> + perf_evlist__reset_id_hash(evlist);
> }
>
> static void __perf_evlist__propagate_maps(struct perf_evlist *evlist,
> @@ -237,6 +234,14 @@ static void perf_evlist__id_hash(struct perf_evlist *evlist,
> hlist_add_head(&sid->node, &evlist->heads[hash]);
> }
>
> +void perf_evlist__reset_id_hash(struct perf_evlist *evlist)
> +{
> + int i;
> +
> + for (i = 0; i < PERF_EVLIST__HLIST_SIZE; ++i)
> + INIT_HLIST_HEAD(&evlist->heads[i]);
> +}
> +
> void perf_evlist__id_add(struct perf_evlist *evlist,
> struct perf_evsel *evsel,
> int cpu, int thread, u64 id)
> diff --git a/tools/lib/perf/include/internal/evlist.h b/tools/lib/perf/include/internal/evlist.h
> index 2d0fa02b036f..212c29063ad4 100644
> --- a/tools/lib/perf/include/internal/evlist.h
> +++ b/tools/lib/perf/include/internal/evlist.h
> @@ -124,4 +124,6 @@ int perf_evlist__id_add_fd(struct perf_evlist *evlist,
> struct perf_evsel *evsel,
> int cpu, int thread, int fd);
>
> +void perf_evlist__reset_id_hash(struct perf_evlist *evlist);
> +
> #endif /* __LIBPERF_INTERNAL_EVLIST_H */
> --
> 2.30.0.617.g56c4b15f3c-goog
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v2 1/2] libperf: Add perf_evlist__reset_id_hash()
2021-02-25 16:12 ` [PATCH v2 1/2] libperf: Add perf_evlist__reset_id_hash() Jiri Olsa
@ 2021-03-03 15:51 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 4+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-03-03 15:51 UTC (permalink / raw)
To: Jiri Olsa
Cc: Namhyung Kim, Ingo Molnar, Peter Zijlstra, Mark Rutland,
Alexander Shishkin, LKML, Stephane Eranian, Ian Rogers
Em Thu, Feb 25, 2021 at 05:12:37PM +0100, Jiri Olsa escreveu:
> On Thu, Feb 25, 2021 at 12:51:47PM +0900, Namhyung Kim wrote:
> > Add the perf_evlist__reset_id_hash() function as an internal function
> > so that it can be called by perf to reset the hash table. This is
> > necessary for perf stat to run the workload multiple times.
> >
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
>
> for the patchset
>
> Acked-by: Jiri Olsa <jolsa@redhat.com>
> thanks,
Thanks, applied.
- Arnaldo
> jirka
>
> > ---
> > tools/lib/perf/evlist.c | 13 +++++++++----
> > tools/lib/perf/include/internal/evlist.h | 2 ++
> > 2 files changed, 11 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/lib/perf/evlist.c b/tools/lib/perf/evlist.c
> > index 17465d454a0e..a0aaf385cbb5 100644
> > --- a/tools/lib/perf/evlist.c
> > +++ b/tools/lib/perf/evlist.c
> > @@ -26,13 +26,10 @@
> >
> > void perf_evlist__init(struct perf_evlist *evlist)
> > {
> > - int i;
> > -
> > - for (i = 0; i < PERF_EVLIST__HLIST_SIZE; ++i)
> > - INIT_HLIST_HEAD(&evlist->heads[i]);
> > INIT_LIST_HEAD(&evlist->entries);
> > evlist->nr_entries = 0;
> > fdarray__init(&evlist->pollfd, 64);
> > + perf_evlist__reset_id_hash(evlist);
> > }
> >
> > static void __perf_evlist__propagate_maps(struct perf_evlist *evlist,
> > @@ -237,6 +234,14 @@ static void perf_evlist__id_hash(struct perf_evlist *evlist,
> > hlist_add_head(&sid->node, &evlist->heads[hash]);
> > }
> >
> > +void perf_evlist__reset_id_hash(struct perf_evlist *evlist)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < PERF_EVLIST__HLIST_SIZE; ++i)
> > + INIT_HLIST_HEAD(&evlist->heads[i]);
> > +}
> > +
> > void perf_evlist__id_add(struct perf_evlist *evlist,
> > struct perf_evsel *evsel,
> > int cpu, int thread, u64 id)
> > diff --git a/tools/lib/perf/include/internal/evlist.h b/tools/lib/perf/include/internal/evlist.h
> > index 2d0fa02b036f..212c29063ad4 100644
> > --- a/tools/lib/perf/include/internal/evlist.h
> > +++ b/tools/lib/perf/include/internal/evlist.h
> > @@ -124,4 +124,6 @@ int perf_evlist__id_add_fd(struct perf_evlist *evlist,
> > struct perf_evsel *evsel,
> > int cpu, int thread, int fd);
> >
> > +void perf_evlist__reset_id_hash(struct perf_evlist *evlist);
> > +
> > #endif /* __LIBPERF_INTERNAL_EVLIST_H */
> > --
> > 2.30.0.617.g56c4b15f3c-goog
> >
>
--
- Arnaldo
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-03-03 19:07 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-25 3:51 [PATCH v2 1/2] libperf: Add perf_evlist__reset_id_hash() Namhyung Kim
2021-02-25 3:51 ` [PATCH v2 2/2] perf stat: Fix use-after-free when -r option is used Namhyung Kim
2021-02-25 16:12 ` [PATCH v2 1/2] libperf: Add perf_evlist__reset_id_hash() Jiri Olsa
2021-03-03 15:51 ` Arnaldo Carvalho de Melo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).