Re: [PATCH 1/9] perf evlist: Fix cpu/thread map leak
Hi Jiri, On Mon, Sep 7, 2020 at 8:32 PM Jiri Olsa wrote: > > On Mon, Sep 07, 2020 at 01:29:29PM +0200, Jiri Olsa wrote: > > On Mon, Sep 07, 2020 at 12:44:54PM +0900, Namhyung Kim wrote: > > > Asan reported leak of cpu and thread maps as they have one more > > > refcount than released. I found that after setting evlist maps it > > > should release it's refcount. > > > > > > It seems to be broken from the beginning so I chose the original > > > commit as the culprit. But not sure how it's applied to stable trees > > > since there are many changes in the code after that. > > > > > > Fixes: 7e2ed097538c5 ("perf evlist: Store pointer to the cpu and thread > > > maps") > > > Fixes: 4112eb1899c0e ("perf evlist: Default to syswide target when no > > > thread/cpu maps set") > > > Signed-off-by: Namhyung Kim > > > --- > > > tools/perf/util/evlist.c | 11 --- > > > 1 file changed, 8 insertions(+), 3 deletions(-) > > > > > > diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c > > > index e3fa3bf7498a..c0768c61eb43 100644 > > > --- a/tools/perf/util/evlist.c > > > +++ b/tools/perf/util/evlist.c > > > @@ -946,6 +946,10 @@ int perf_evlist__create_maps(struct evlist *evlist, > > > struct target *target) > > > > > > perf_evlist__set_maps(>core, cpus, threads); > > > > > > + /* as evlist now has references, put count here */ > > > + perf_cpu_map__put(cpus); > > > + perf_thread_map__put(threads); > > > > nice catch, I checked perf_evlist__create_syswide_maps is doing this > > because you fixed that :))) missed the rest of the patch.. sry No worries and thanks for the review! :) Namhyung
Re: [PATCH 1/9] perf evlist: Fix cpu/thread map leak
On Mon, Sep 07, 2020 at 01:29:29PM +0200, Jiri Olsa wrote: > On Mon, Sep 07, 2020 at 12:44:54PM +0900, Namhyung Kim wrote: > > Asan reported leak of cpu and thread maps as they have one more > > refcount than released. I found that after setting evlist maps it > > should release it's refcount. > > > > It seems to be broken from the beginning so I chose the original > > commit as the culprit. But not sure how it's applied to stable trees > > since there are many changes in the code after that. > > > > Fixes: 7e2ed097538c5 ("perf evlist: Store pointer to the cpu and thread > > maps") > > Fixes: 4112eb1899c0e ("perf evlist: Default to syswide target when no > > thread/cpu maps set") > > Signed-off-by: Namhyung Kim > > --- > > tools/perf/util/evlist.c | 11 --- > > 1 file changed, 8 insertions(+), 3 deletions(-) > > > > diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c > > index e3fa3bf7498a..c0768c61eb43 100644 > > --- a/tools/perf/util/evlist.c > > +++ b/tools/perf/util/evlist.c > > @@ -946,6 +946,10 @@ int perf_evlist__create_maps(struct evlist *evlist, > > struct target *target) > > > > perf_evlist__set_maps(>core, cpus, threads); > > > > + /* as evlist now has references, put count here */ > > + perf_cpu_map__put(cpus); > > + perf_thread_map__put(threads); > > nice catch, I checked perf_evlist__create_syswide_maps is doing this because you fixed that :))) missed the rest of the patch.. sry jirka > correctly, but I think we might have the same issue in script's > set_maps function > > thanks, > jirka > > > + > > return 0; > > > > out_delete_threads: > > @@ -1273,11 +1277,12 @@ static int perf_evlist__create_syswide_maps(struct > > evlist *evlist) > > goto out_put; > > > > perf_evlist__set_maps(>core, cpus, threads); > > -out: > > - return err; > > + > > + perf_thread_map__put(threads); > > out_put: > > perf_cpu_map__put(cpus); > > - goto out; > > +out: > > + return err; > > } > > > > int evlist__open(struct evlist *evlist) > > -- > > 2.28.0.526.ge36021eeef-goog > >
Re: [PATCH 1/9] perf evlist: Fix cpu/thread map leak
On Mon, Sep 07, 2020 at 12:44:54PM +0900, Namhyung Kim wrote: > Asan reported leak of cpu and thread maps as they have one more > refcount than released. I found that after setting evlist maps it > should release it's refcount. > > It seems to be broken from the beginning so I chose the original > commit as the culprit. But not sure how it's applied to stable trees > since there are many changes in the code after that. > > Fixes: 7e2ed097538c5 ("perf evlist: Store pointer to the cpu and thread maps") > Fixes: 4112eb1899c0e ("perf evlist: Default to syswide target when no > thread/cpu maps set") > Signed-off-by: Namhyung Kim > --- > tools/perf/util/evlist.c | 11 --- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c > index e3fa3bf7498a..c0768c61eb43 100644 > --- a/tools/perf/util/evlist.c > +++ b/tools/perf/util/evlist.c > @@ -946,6 +946,10 @@ int perf_evlist__create_maps(struct evlist *evlist, > struct target *target) > > perf_evlist__set_maps(>core, cpus, threads); > > + /* as evlist now has references, put count here */ > + perf_cpu_map__put(cpus); > + perf_thread_map__put(threads); nice catch, I checked perf_evlist__create_syswide_maps is doing this correctly, but I think we might have the same issue in script's set_maps function thanks, jirka > + > return 0; > > out_delete_threads: > @@ -1273,11 +1277,12 @@ static int perf_evlist__create_syswide_maps(struct > evlist *evlist) > goto out_put; > > perf_evlist__set_maps(>core, cpus, threads); > -out: > - return err; > + > + perf_thread_map__put(threads); > out_put: > perf_cpu_map__put(cpus); > - goto out; > +out: > + return err; > } > > int evlist__open(struct evlist *evlist) > -- > 2.28.0.526.ge36021eeef-goog >
[PATCH 1/9] perf evlist: Fix cpu/thread map leak
Asan reported leak of cpu and thread maps as they have one more refcount than released. I found that after setting evlist maps it should release it's refcount. It seems to be broken from the beginning so I chose the original commit as the culprit. But not sure how it's applied to stable trees since there are many changes in the code after that. Fixes: 7e2ed097538c5 ("perf evlist: Store pointer to the cpu and thread maps") Fixes: 4112eb1899c0e ("perf evlist: Default to syswide target when no thread/cpu maps set") Signed-off-by: Namhyung Kim --- tools/perf/util/evlist.c | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c index e3fa3bf7498a..c0768c61eb43 100644 --- a/tools/perf/util/evlist.c +++ b/tools/perf/util/evlist.c @@ -946,6 +946,10 @@ int perf_evlist__create_maps(struct evlist *evlist, struct target *target) perf_evlist__set_maps(>core, cpus, threads); + /* as evlist now has references, put count here */ + perf_cpu_map__put(cpus); + perf_thread_map__put(threads); + return 0; out_delete_threads: @@ -1273,11 +1277,12 @@ static int perf_evlist__create_syswide_maps(struct evlist *evlist) goto out_put; perf_evlist__set_maps(>core, cpus, threads); -out: - return err; + + perf_thread_map__put(threads); out_put: perf_cpu_map__put(cpus); - goto out; +out: + return err; } int evlist__open(struct evlist *evlist) -- 2.28.0.526.ge36021eeef-goog