Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1271875 > unrolled thread

[PATCH perf/core 00/13] perf memory/refcnt leak fixes

Started byMasami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
First post2015-11-18 07:50 +0100
Last post2015-11-19 04:00 +0100
Articles 17 — 5 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH perf/core  00/13] perf memory/refcnt leak fixes Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> - 2015-11-18 07:50 +0100
    [PATCH perf/core 12/13] perf: Fix machine__findnew_module_map to  put dso Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> - 2015-11-18 07:50 +0100
      [tip:perf/core] perf machine:   Fix machine__findnew_module_map to put dso tip-bot for Masami Hiramatsu <tipbot@zytor.com> - 2015-11-23 17:20 +0100
    [PATCH perf/core  13/13] perf: Fix dso__load_sym to put dso Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> - 2015-11-18 07:50 +0100
    [PATCH perf/core 10/13] perf: Fix __dsos__addnew to put dso after  adding it to the list Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> - 2015-11-18 07:50 +0100
      [tip:perf/core] perf tools:   Fix __dsos__addnew to put dso after adding it to the list tip-bot for Masami Hiramatsu <tipbot@zytor.com> - 2015-11-23 17:20 +0100
    [PATCH perf/core 05/13] perf: Fix machine__findnew_module_map to  put registered map Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> - 2015-11-18 07:50 +0100
      Re: [PATCH perf/core 05/13] perf: Fix machine__findnew_module_map to  put registered map Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-11-18 23:40 +0100
      [tip:perf/core] perf machine:   Fix machine__findnew_module_map to put registered map tip-bot for Masami Hiramatsu <tipbot@zytor.com> - 2015-11-23 17:20 +0100
    [PATCH perf/core  09/13] perf: Make dso to use refcnt for debug Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> - 2015-11-18 07:50 +0100
    [PATCH perf/core 02/13] perf: Make perf_exec_path always returns  malloc'd string Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> - 2015-11-18 08:00 +0100
    [PATCH perf/core 03/13] perf: Introduce generic refcount APIs with  debug feature Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> - 2015-11-18 08:00 +0100
      Re: [PATCH perf/core 03/13] perf: Introduce generic refcount APIs  with debug feature Namhyung Kim <namhyung@kernel.org> - 2015-11-20 04:00 +0100
        RE: [PATCH perf/core 03/13] perf: Introduce generic refcount APIs  with debug feature 平松雅巳 / HIRAMATU,MASAMI   <masami.hiramatsu.pt@hitachi.com> - 2015-11-20 05:20 +0100
          Re: [PATCH perf/core 03/13] perf: Introduce generic refcount APIs  with debug feature Namhyung Kim <namhyung@kernel.org> - 2015-11-20 07:00 +0100
    Re: [PATCH perf/core  00/13] perf memory/refcnt leak fixes Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-11-18 13:50 +0100
      RE: [PATCH perf/core  00/13] perf memory/refcnt leak fixes 平松雅巳 / HIRAMATU,MASAMI   <masami.hiramatsu.pt@hitachi.com> - 2015-11-19 04:00 +0100

#1271875 — [PATCH perf/core 00/13] perf memory/refcnt leak fixes

FromMasami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Date2015-11-18 07:50 +0100
Subject[PATCH perf/core 00/13] perf memory/refcnt leak fixes
Message-ID<qw4J3-7HS-3@gated-at.bofh.it>
Hi,

Here is a series to fix some memory leaks and refcount
leaks on map and dso. This also includes the refcnt APIs
with backtrace debugging feature.

The story has started from the posible memory leak report
reported by Wnag Nan.
I've tried to use valgrind to ensure the perf probe doesn't
have other memory leaks. The result is here:

  ----
  # valgrind ./perf probe vfs_read
  ==17521== Memcheck, a memory error detector
  ==17521== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
  ==17521== Using Valgrind-3.10.0 and LibVEX; rerun with -h for copyright info
  ==17521== Command: ./perf probe vfs_read
  ==17521==
  Added new event:
    probe:vfs_read       (on vfs_read)
  
  You can now use it in all perf tools, such as:
  
          perf record -e probe:vfs_read -aR sleep 1
  
  ==17521==
  ==17521== HEAP SUMMARY:
  ==17521==     in use at exit: 3,512,761 bytes in 38,012 blocks
  ==17521==   total heap usage: 74,723 allocs, 36,711 frees, 24,014,927
  bytes allocated
  ==17521==
  ==17521== LEAK SUMMARY:
  ==17521==    definitely lost: 6,857 bytes in 49 blocks
  ==17521==    indirectly lost: 3,501,287 bytes in 37,891 blocks
  ==17521==      possibly lost: 0 bytes in 0 blocks
  ==17521==    still reachable: 4,617 bytes in 72 blocks
  ==17521==         suppressed: 0 bytes in 0 blocks
  ==17521== Rerun with --leak-check=full to see details of leaked memory
  ==17521==
  ==17521== For counts of detected and suppressed errors, rerun with: -v
  ==17521== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 2 from 2)
  ----

Oops! It leaked almost 4 MB memories. I've tried to find
the root causes, and what I've found is there are many
leaks in not only perf-probe specific code, but also maps
and dsos (and some other pieces).

The first 3 patches are just fixing 'easy' memory leaks. However,
most of the leaks are caused by refcnt. Since valgrind seems not
able to debug this kind of issues, I introduced a hand-made refcnt
backtrace APIs for debugging.
The rest of patches are for fixing refcnt leak bugs and replcing
refcnt apis.

After all, most of the issues are gone, except for just a few issues
in elfutils. I'll continue to investigate that.

  ----
  valgrind ./perf probe vfs_read
  ==29521== Memcheck, a memory error detector
  ==29521== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
  ==29521== Using Valgrind-3.10.0 and LibVEX; rerun with -h for copyright info
  ==29521== Command: ./perf probe vfs_read
  ==29521==
  Added new event:
    probe:vfs_read       (on vfs_read)
  
  You can now use it in all perf tools, such as:
  
          perf record -e probe:vfs_read -aR sleep 1
  
  ==29521==
  ==29521== HEAP SUMMARY:
  ==29521==     in use at exit: 5,137 bytes in 75 blocks
  ==29521==   total heap usage: 74,723 allocs, 74,648 frees, 24,014,927
  bytes allocated
  ==29521==
  ==29521== LEAK SUMMARY:
  ==29521==    definitely lost: 520 bytes in 3 blocks
  ==29521==    indirectly lost: 0 bytes in 0 blocks
  ==29521==      possibly lost: 0 bytes in 0 blocks
  ==29521==    still reachable: 4,617 bytes in 72 blocks
  ==29521==         suppressed: 0 bytes in 0 blocks
  ==29521== Rerun with --leak-check=full to see details of leaked memory
  ==29521==
  ==29521== For counts of detected and suppressed errors, rerun with: -v
  ==29521== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 2 from 2)
  ----

Anyway, I decided to release these fixes and the debugging feature
because at least this will improve perf quality.

Thank you,

---

Masami Hiramatsu (13):
      perf probe: Fix to free temporal Dwarf_Frame
      perf: Make perf_exec_path always returns malloc'd string
      perf: Introduce generic refcount APIs with debug feature
      perf: make map to use refcnt
      perf: Fix machine__findnew_module_map to put registered map
      perf: Fix machine__destroy_kernel_maps to put vmlinux_maps
      perf: Fix to destroy kernel maps when machine exits
      perf: Fix to put new map after inserting to map_groups in dso__load_sym
      perf: Make dso to use refcnt for debug
      perf: Fix __dsos__addnew to put dso after adding it to the list
      perf: Fix machine__create_kernel_maps to put kernel dso
      perf: Fix machine__findnew_module_map to put dso
      perf: Fix dso__load_sym to put dso


 tools/perf/config/Makefile     |    5 ++
 tools/perf/util/Build          |    1 
 tools/perf/util/dso.c          |    9 ++-
 tools/perf/util/exec_cmd.c     |   20 ++++--
 tools/perf/util/exec_cmd.h     |    5 +-
 tools/perf/util/help.c         |    6 +-
 tools/perf/util/machine.c      |   17 ++++-
 tools/perf/util/map.c          |    7 +-
 tools/perf/util/map.h          |    3 +
 tools/perf/util/probe-finder.c |    9 ++-
 tools/perf/util/refcnt.c       |  125 ++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/refcnt.h       |   65 +++++++++++++++++++++
 tools/perf/util/symbol-elf.c   |    4 +
 13 files changed, 250 insertions(+), 26 deletions(-)
 create mode 100644 tools/perf/util/refcnt.c
 create mode 100644 tools/perf/util/refcnt.h


-- 
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering 
Hitachi, Ltd., Research & Development Group
E-mail: masami.hiramatsu.pt@hitachi.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1271876 — [PATCH perf/core 12/13] perf: Fix machine__findnew_module_map to put dso

FromMasami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Date2015-11-18 07:50 +0100
Subject[PATCH perf/core 12/13] perf: Fix machine__findnew_module_map to put dso
Message-ID<qw4J4-7HS-19@gated-at.bofh.it>
In reply to#1271875
Fix machine__findnew_module_map to put dso because the dso is
already got by machine__findnew_module_dso() and map__new2().

Refcnt debugger shows:

  ==== [1] ====
  Unreclaimed dso: 0x1ffd980
  Refcount +1 => 1 at
    ./perf(dso__new+0x1ff) [0x4a62df]
    ./perf(__dsos__addnew+0x29) [0x4a6e19]
    ./perf() [0x4b8b91]
    ./perf(modules__parse+0xfc) [0x4a9d5c]
    ./perf() [0x4b8460]
    ./perf(machine__create_kernel_maps+0x150) [0x4bb550]
    ./perf(machine__new_host+0xfa) [0x4bb75a]
    ./perf(init_probe_symbol_maps+0x93) [0x506623]
    ./perf() [0x455ffa]
    ./perf(cmd_probe+0x6c) [0x4566bc]
    ./perf() [0x47abc5]
    ./perf(main+0x610) [0x421f90]
    /lib64/libc.so.6(__libc_start_main+0xf5) [0x7f1345a8eaf5]
    ./perf() [0x4220a9]

This map_groups__insert(0x4b8b91) already get the new dso,
  ----
  eu-addr2line -e ./perf -f 0x4b8b91
  map_groups__insert inlined at util/machine.c:586 in
  machine__create_module
  util/map.h:207
  ----
So this dso refcnt will be released when map_groups released.

  [snip]
  Refcount +1 => 2 at
    ./perf(dso__get+0x34) [0x4a65f4]
    ./perf() [0x4b8b35]
    ./perf(modules__parse+0xfc) [0x4a9d5c]
    ./perf() [0x4b8460]
    ./perf(machine__create_kernel_maps+0x150) [0x4bb550]
    ./perf(machine__new_host+0xfa) [0x4bb75a]
    ./perf(init_probe_symbol_maps+0x93) [0x506623]
    ./perf() [0x455ffa]
    ./perf(cmd_probe+0x6c) [0x4566bc]
    ./perf() [0x47abc5]
    ./perf(main+0x610) [0x421f90]
    /lib64/libc.so.6(__libc_start_main+0xf5) [0x7f1345a8eaf5]
    ./perf() [0x4220a9]

Here, machine__findnew_module_dso(0x4b8b35) gets the dso.
(and stores the dso to local variable)
  ----
  # eu-addr2line -e ./perf -f 0x4b8b35
  machine__findnew_module_dso inlined at util/machine.c:578 in
  machine__create_module
  util/machine.c:514
  ----

  Refcount +1 => 3 at
    ./perf(dso__get+0x34) [0x4a65f4]
    ./perf(map__new2+0x76) [0x4be1c6]
    ./perf() [0x4b8b4f]
    ./perf(modules__parse+0xfc) [0x4a9d5c]
    ./perf() [0x4b8460]
    ./perf(machine__create_kernel_maps+0x150) [0x4bb550]
    ./perf(machine__new_host+0xfa) [0x4bb75a]
    ./perf(init_probe_symbol_maps+0x93) [0x506623]
    ./perf() [0x455ffa]
    ./perf(cmd_probe+0x6c) [0x4566bc]
    ./perf() [0x47abc5]
    ./perf(main+0x610) [0x421f90]
    /lib64/libc.so.6(__libc_start_main+0xf5) [0x7f1345a8eaf5]
    ./perf() [0x4220a9]

But also map__new2 gets the dso which will be put when
the map is released.

So, we have to put the dso corresponding to above
machine__findnew_module_dso.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
 tools/perf/util/machine.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index d38ecb5..96e5942 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -564,7 +564,7 @@ struct map *machine__findnew_module_map(struct machine *machine, u64 start,
 					const char *filename)
 {
 	struct map *map = NULL;
-	struct dso *dso;
+	struct dso *dso = NULL;
 	struct kmod_path m;
 
 	if (kmod_path__parse_name(&m, filename))
@@ -588,6 +588,8 @@ struct map *machine__findnew_module_map(struct machine *machine, u64 start,
 	/* Put the map here because map_groups__insert alread got it */
 	map__put(map);
 out:
+	/* put the dso here, corresponding to  machine__findnew_module_dso */
+	dso__put(dso);
 	free(m.name);
 	return map;
 }

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1275534 — [tip:perf/core] perf machine: Fix machine__findnew_module_map to put dso

Fromtip-bot for Masami Hiramatsu <tipbot@zytor.com>
Date2015-11-23 17:20 +0100
Subject[tip:perf/core] perf machine: Fix machine__findnew_module_map to put dso
Message-ID<qy20p-4cV-7@gated-at.bofh.it>
In reply to#1271876
Commit-ID:  566c69c36e6178774dd484ea4a02b76f6bd0ede4
Gitweb:     http://git.kernel.org/tip/566c69c36e6178774dd484ea4a02b76f6bd0ede4
Author:     Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
AuthorDate: Wed, 18 Nov 2015 15:40:35 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 19 Nov 2015 13:19:21 -0300

perf machine: Fix machine__findnew_module_map to put dso

Fix machine__findnew_module_map to drop the reference to the dso because
it is already referenced by both machine__findnew_module_dso() and
map__new2().

Refcnt debugger shows:

  ==== [1] ====
  Unreclaimed dso: 0x1ffd980
  Refcount +1 => 1 at
    ./perf(dso__new+0x1ff) [0x4a62df]
    ./perf(__dsos__addnew+0x29) [0x4a6e19]
    ./perf() [0x4b8b91]
    ./perf(modules__parse+0xfc) [0x4a9d5c]
    ./perf() [0x4b8460]
    ./perf(machine__create_kernel_maps+0x150) [0x4bb550]
    ./perf(machine__new_host+0xfa) [0x4bb75a]
    ./perf(init_probe_symbol_maps+0x93) [0x506623]
    ./perf() [0x455ffa]
    ./perf(cmd_probe+0x6c) [0x4566bc]
    ./perf() [0x47abc5]
    ./perf(main+0x610) [0x421f90]
    /lib64/libc.so.6(__libc_start_main+0xf5) [0x7f1345a8eaf5]
    ./perf() [0x4220a9]

This map_groups__insert(0x4b8b91) already gets a reference to the new
dso:

  ----
  eu-addr2line -e ./perf -f 0x4b8b91
  map_groups__insert inlined at util/machine.c:586 in
  machine__create_module
  util/map.h:207
  ----

So this dso refcnt will be released when map_groups gets released.

  [snip]
  Refcount +1 => 2 at
    ./perf(dso__get+0x34) [0x4a65f4]
    ./perf() [0x4b8b35]
    ./perf(modules__parse+0xfc) [0x4a9d5c]
    ./perf() [0x4b8460]
    ./perf(machine__create_kernel_maps+0x150) [0x4bb550]
    ./perf(machine__new_host+0xfa) [0x4bb75a]
    ./perf(init_probe_symbol_maps+0x93) [0x506623]
    ./perf() [0x455ffa]
    ./perf(cmd_probe+0x6c) [0x4566bc]
    ./perf() [0x47abc5]
    ./perf(main+0x610) [0x421f90]
    /lib64/libc.so.6(__libc_start_main+0xf5) [0x7f1345a8eaf5]
    ./perf() [0x4220a9]

Here, machine__findnew_module_dso(0x4b8b35) gets the dso (and stores it
in a local variable):

  ----
  # eu-addr2line -e ./perf -f 0x4b8b35
  machine__findnew_module_dso inlined at util/machine.c:578 in
  machine__create_module
  util/machine.c:514
  ----

  Refcount +1 => 3 at
    ./perf(dso__get+0x34) [0x4a65f4]
    ./perf(map__new2+0x76) [0x4be1c6]
    ./perf() [0x4b8b4f]
    ./perf(modules__parse+0xfc) [0x4a9d5c]
    ./perf() [0x4b8460]
    ./perf(machine__create_kernel_maps+0x150) [0x4bb550]
    ./perf(machine__new_host+0xfa) [0x4bb75a]
    ./perf(init_probe_symbol_maps+0x93) [0x506623]
    ./perf() [0x455ffa]
    ./perf(cmd_probe+0x6c) [0x4566bc]
    ./perf() [0x47abc5]
    ./perf(main+0x610) [0x421f90]
    /lib64/libc.so.6(__libc_start_main+0xf5) [0x7f1345a8eaf5]
    ./perf() [0x4220a9]

But also map__new2() gets the dso which will be put when the map is
released.

So, we have to drop the constructor reference obtained in
machine__findnew_module_dso().

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/20151118064035.30709.58824.stgit@localhost.localdomain
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/machine.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 0b4a05c..7f5071a 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -565,7 +565,7 @@ struct map *machine__findnew_module_map(struct machine *machine, u64 start,
 					const char *filename)
 {
 	struct map *map = NULL;
-	struct dso *dso;
+	struct dso *dso = NULL;
 	struct kmod_path m;
 
 	if (kmod_path__parse_name(&m, filename))
@@ -589,6 +589,8 @@ struct map *machine__findnew_module_map(struct machine *machine, u64 start,
 	/* Put the map here because map_groups__insert alread got it */
 	map__put(map);
 out:
+	/* put the dso here, corresponding to  machine__findnew_module_dso */
+	dso__put(dso);
 	free(m.name);
 	return map;
 }
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1271877 — [PATCH perf/core 13/13] perf: Fix dso__load_sym to put dso

FromMasami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Date2015-11-18 07:50 +0100
Subject[PATCH perf/core 13/13] perf: Fix dso__load_sym to put dso
Message-ID<qw4J3-7HS-11@gated-at.bofh.it>
In reply to#1271875
Fix dso__load_sym to put dso because dsos__add already got it.

Refcnt debugger explain the problem:
  ----
  ==== [0] ====
  Unreclaimed dso: 0x19dd200
  Refcount +1 => 1 at
    ./perf(dso__new+0x1ff) [0x4a62df]
    ./perf(dso__load_sym+0xe89) [0x503509]
    ./perf(dso__load_vmlinux+0xbf) [0x4aa77f]
    ./perf(dso__load_vmlinux_path+0x8c) [0x4aa8dc]
    ./perf() [0x50539a]
    ./perf(convert_perf_probe_events+0xd79) [0x50ad39]
    ./perf() [0x45600f]
    ./perf(cmd_probe+0x6c) [0x4566bc]
    ./perf() [0x47abc5]
    ./perf(main+0x610) [0x421f90]
    /lib64/libc.so.6(__libc_start_main+0xf5) [0x7f74dd0efaf5]
    ./perf() [0x4220a9]
  Refcount +1 => 2 at
    ./perf(dso__get+0x34) [0x4a65f4]
    ./perf(map__new2+0x76) [0x4be216]
    ./perf(dso__load_sym+0xee1) [0x503561]
    ./perf(dso__load_vmlinux+0xbf) [0x4aa77f]
    ./perf(dso__load_vmlinux_path+0x8c) [0x4aa8dc]
    ./perf() [0x50539a]
    ./perf(convert_perf_probe_events+0xd79) [0x50ad39]
    ./perf() [0x45600f]
    ./perf(cmd_probe+0x6c) [0x4566bc]
    ./perf() [0x47abc5]
    ./perf(main+0x610) [0x421f90]
    /lib64/libc.so.6(__libc_start_main+0xf5) [0x7f74dd0efaf5]
    ./perf() [0x4220a9]
  Refcount +1 => 3 at
    ./perf(dsos__add+0xf3) [0x4a6bc3]
    ./perf(dso__load_sym+0xfc1) [0x503641]
    ./perf(dso__load_vmlinux+0xbf) [0x4aa77f]
    ./perf(dso__load_vmlinux_path+0x8c) [0x4aa8dc]
    ./perf() [0x50539a]
    ./perf(convert_perf_probe_events+0xd79) [0x50ad39]
    ./perf() [0x45600f]
    ./perf(cmd_probe+0x6c) [0x4566bc]
    ./perf() [0x47abc5]
    ./perf(main+0x610) [0x421f90]
    /lib64/libc.so.6(__libc_start_main+0xf5) [0x7f74dd0efaf5]
    ./perf() [0x4220a9]
  Refcount -1 => 2 at
    ./perf(dso__put+0x2f) [0x4a664f]
    ./perf(map_groups__exit+0xb9) [0x4bee29]
    ./perf(machine__delete+0xb0) [0x4b93d0]
    ./perf(exit_probe_symbol_maps+0x28) [0x506718]
    ./perf() [0x45628a]
    ./perf(cmd_probe+0x6c) [0x4566bc]
    ./perf() [0x47abc5]
    ./perf(main+0x610) [0x421f90]
    /lib64/libc.so.6(__libc_start_main+0xf5) [0x7f74dd0efaf5]
    ./perf() [0x4220a9]
  Refcount -1 => 1 at
    ./perf(dso__put+0x2f) [0x4a664f]
    ./perf(machine__delete+0xfe) [0x4b941e]
    ./perf(exit_probe_symbol_maps+0x28) [0x506718]
    ./perf() [0x45628a]
    ./perf(cmd_probe+0x6c) [0x4566bc]
    ./perf() [0x47abc5]
    ./perf(main+0x610) [0x421f90]
    /lib64/libc.so.6(__libc_start_main+0xf5) [0x7f74dd0efaf5]
    ./perf() [0x4220a9]
  ----
So, in the dso__load_sym, dso is gotten 3 times, by dso__new,
map__new2, and dsos__add. The last 2 is actually released by
map_groups and machine__delete correspondingly. However, the
first reference by dso__new, is never released.

This solves this issue by putting dso right after dsos__add.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
 tools/perf/util/symbol-elf.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 53f1996..a5703ac 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -1045,6 +1045,8 @@ int dso__load_sym(struct dso *dso, struct map *map,
 				/* kmaps already got it */
 				map__put(curr_map);
 				dsos__add(&map->groups->machine->dsos, curr_dso);
+				/* curr_map and machine->dsos already got it */
+				dso__put(curr_dso);
 				dso__set_loaded(curr_dso, map->type);
 			} else
 				curr_dso = curr_map->dso;

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1271878 — [PATCH perf/core 10/13] perf: Fix __dsos__addnew to put dso after adding it to the list

FromMasami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Date2015-11-18 07:50 +0100
Subject[PATCH perf/core 10/13] perf: Fix __dsos__addnew to put dso after adding it to the list
Message-ID<qw4J4-7HS-21@gated-at.bofh.it>
In reply to#1271875
Fix __dsos__addnew to put dso after adding it to the list, because
__dsos__add() gets the dso refcount.
This eases refcount leaks on dso.

Refcnt debugger shows:
  ==== [0] ====
  Unreclaimed dso: 0x2fccab0
  Refcount +1 => 1 at
    ./perf(dso__new+0x1ff) [0x4a62df]
    ./perf(__dsos__addnew+0x29) [0x4a6e19]
    ./perf(dsos__findnew+0xd1) [0x4a7281]
    ./perf(machine__findnew_kernel+0x27) [0x4a5e17]
    ./perf() [0x4b8df2]
    ./perf(machine__create_kernel_maps+0x28) [0x4bb528]
    ./perf(machine__new_host+0xfa) [0x4bb84a]
    ./perf(init_probe_symbol_maps+0x93) [0x506713]
    ./perf() [0x455ffa]
    ./perf(cmd_probe+0x6c) [0x4566bc]
    ./perf() [0x47abc5]
    ./perf(main+0x610) [0x421f90]
    /lib64/libc.so.6(__libc_start_main+0xf5) [0x7f46df132af5]
    ./perf() [0x4220a9]
  Refcount +1 => 2 at
    ./perf(__dsos__addnew+0xfb) [0x4a6eeb]
    ./perf(dsos__findnew+0xd1) [0x4a7281]
    ./perf(machine__findnew_kernel+0x27) [0x4a5e17]
    ./perf() [0x4b8df2]
    ./perf(machine__create_kernel_maps+0x28) [0x4bb528]
    ./perf(machine__new_host+0xfa) [0x4bb84a]
    ./perf(init_probe_symbol_maps+0x93) [0x506713]
    ./perf() [0x455ffa]
    ./perf(cmd_probe+0x6c) [0x4566bc]
    ./perf() [0x47abc5]
    ./perf(main+0x610) [0x421f90]
    /lib64/libc.so.6(__libc_start_main+0xf5) [0x7f46df132af5]
    ./perf() [0x4220a9]
  Refcount +1 => 3 at
    ./perf(dsos__findnew+0x7e) [0x4a722e]
    ./perf(machine__findnew_kernel+0x27) [0x4a5e17]
    ./perf() [0x4b8df2]
    ./perf(machine__create_kernel_maps+0x28) [0x4bb528]
    ./perf(machine__new_host+0xfa) [0x4bb84a]
    ./perf(init_probe_symbol_maps+0x93) [0x506713]
    ./perf() [0x455ffa]
    ./perf(cmd_probe+0x6c) [0x4566bc]
    ./perf() [0x47abc5]
    ./perf(main+0x610) [0x421f90]
    /lib64/libc.so.6(__libc_start_main+0xf5) [0x7f46df132af5]
    ./perf() [0x4220a9]
  [snip]

Here, __dsos__addnew() gets the dso twice, once for init,
once for adding the dso to its list. But after added, we
don't need the dso reference (since we already have).

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
 tools/perf/util/dso.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 28ad06a..d21057c 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -1227,6 +1227,8 @@ struct dso *__dsos__addnew(struct dsos *dsos, const char *name)
 	if (dso != NULL) {
 		__dsos__add(dsos, dso);
 		dso__set_basename(dso);
+		/* Put dso here because __dsos_add already got it */
+		dso__put(dso);
 	}
 	return dso;
 }

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1275553 — [tip:perf/core] perf tools: Fix __dsos__addnew to put dso after adding it to the list

Fromtip-bot for Masami Hiramatsu <tipbot@zytor.com>
Date2015-11-23 17:20 +0100
Subject[tip:perf/core] perf tools: Fix __dsos__addnew to put dso after adding it to the list
Message-ID<qy20r-4cV-59@gated-at.bofh.it>
In reply to#1271878
Commit-ID:  82de26abdc127172fd7453a61d35a9b33bf4f871
Gitweb:     http://git.kernel.org/tip/82de26abdc127172fd7453a61d35a9b33bf4f871
Author:     Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
AuthorDate: Wed, 18 Nov 2015 15:40:31 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 19 Nov 2015 13:19:20 -0300

perf tools: Fix __dsos__addnew to put dso after adding it to the list

__dsos__addnew should drop the constructor reference to dso after adding
it to the list, because __dsos__add() will get a reference that will be
kept while it is in the list.

This fixes DSO leaks when entries are removed to the list and the refcount
never gets to zero.

Refcnt debugger shows:
  ==== [0] ====
  Unreclaimed dso: 0x2fccab0
  Refcount +1 => 1 at
    ./perf(dso__new+0x1ff) [0x4a62df]
    ./perf(__dsos__addnew+0x29) [0x4a6e19]
    ./perf(dsos__findnew+0xd1) [0x4a7281]
    ./perf(machine__findnew_kernel+0x27) [0x4a5e17]
    ./perf() [0x4b8df2]
    ./perf(machine__create_kernel_maps+0x28) [0x4bb528]
    ./perf(machine__new_host+0xfa) [0x4bb84a]
    ./perf(init_probe_symbol_maps+0x93) [0x506713]
    ./perf() [0x455ffa]
    ./perf(cmd_probe+0x6c) [0x4566bc]
    ./perf() [0x47abc5]
    ./perf(main+0x610) [0x421f90]
    /lib64/libc.so.6(__libc_start_main+0xf5) [0x7f46df132af5]
    ./perf() [0x4220a9]
  Refcount +1 => 2 at
    ./perf(__dsos__addnew+0xfb) [0x4a6eeb]
    ./perf(dsos__findnew+0xd1) [0x4a7281]
    ./perf(machine__findnew_kernel+0x27) [0x4a5e17]
    ./perf() [0x4b8df2]
    ./perf(machine__create_kernel_maps+0x28) [0x4bb528]
    ./perf(machine__new_host+0xfa) [0x4bb84a]
    ./perf(init_probe_symbol_maps+0x93) [0x506713]
    ./perf() [0x455ffa]
    ./perf(cmd_probe+0x6c) [0x4566bc]
    ./perf() [0x47abc5]
    ./perf(main+0x610) [0x421f90]
    /lib64/libc.so.6(__libc_start_main+0xf5) [0x7f46df132af5]
    ./perf() [0x4220a9]
  Refcount +1 => 3 at
    ./perf(dsos__findnew+0x7e) [0x4a722e]
    ./perf(machine__findnew_kernel+0x27) [0x4a5e17]
    ./perf() [0x4b8df2]
    ./perf(machine__create_kernel_maps+0x28) [0x4bb528]
    ./perf(machine__new_host+0xfa) [0x4bb84a]
    ./perf(init_probe_symbol_maps+0x93) [0x506713]
    ./perf() [0x455ffa]
    ./perf(cmd_probe+0x6c) [0x4566bc]
    ./perf() [0x47abc5]
    ./perf(main+0x610) [0x421f90]
    /lib64/libc.so.6(__libc_start_main+0xf5) [0x7f46df132af5]
    ./perf() [0x4220a9]
  [snip]

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/20151118064031.30709.81460.stgit@localhost.localdomain
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/dso.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 425df5c..e8e9a9d 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -1243,6 +1243,8 @@ struct dso *__dsos__addnew(struct dsos *dsos, const char *name)
 	if (dso != NULL) {
 		__dsos__add(dsos, dso);
 		dso__set_basename(dso);
+		/* Put dso here because __dsos_add already got it */
+		dso__put(dso);
 	}
 	return dso;
 }
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1271879 — [PATCH perf/core 05/13] perf: Fix machine__findnew_module_map to put registered map

FromMasami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Date2015-11-18 07:50 +0100
Subject[PATCH perf/core 05/13] perf: Fix machine__findnew_module_map to put registered map
Message-ID<qw4J4-7HS-23@gated-at.bofh.it>
In reply to#1271875
Fix machine object to put the map object which is already
insterted to machine->kmaps.

refcnt debugger shows what happened:
  ----
  ==== [2] ====
  Unreclaimed map: 0x346f750
  Refcount +1 => 1 at
    ./perf(map__new2+0xb5) [0x4bdea5]
    ./perf() [0x4b8aaf]
    ./perf(modules__parse+0xfc) [0x4a9cbc]
    ./perf() [0x4b83c0]
    ./perf(machine__create_kernel_maps+0x148) [0x4bb208]
    ./perf(machine__new_host+0xfa) [0x4bb3fa]
    ./perf(init_probe_symbol_maps+0x93) [0x5062b3]
    ./perf() [0x455ffa]
    ./perf(cmd_probe+0x6c) [0x4566bc]
    ./perf() [0x47abc5]
    ./perf(main+0x610) [0x421f90]
    /lib64/libc.so.6(__libc_start_main+0xf5) [0x7f5373899af5]
    ./perf() [0x4220a9]
  Refcount +1 => 2 at
    ./perf(maps__insert+0x9a) [0x4bfd4a]
    ./perf() [0x4b8acb]
    ./perf(modules__parse+0xfc) [0x4a9cbc]
    ./perf() [0x4b83c0]
    ./perf(machine__create_kernel_maps+0x148) [0x4bb208]
    ./perf(machine__new_host+0xfa) [0x4bb3fa]
    ./perf(init_probe_symbol_maps+0x93) [0x5062b3]
    ./perf() [0x455ffa]
    ./perf(cmd_probe+0x6c) [0x4566bc]
    ./perf() [0x47abc5]
    ./perf(main+0x610) [0x421f90]
    /lib64/libc.so.6(__libc_start_main+0xf5) [0x7f5373899af5]
    ./perf() [0x4220a9]
  Refcount -1 => 1 at
    ./perf(map_groups__exit+0x94) [0x4bea54]
    ./perf(machine__delete+0x3d) [0x4b91ed]
    ./perf(exit_probe_symbol_maps+0x28) [0x506358]
    ./perf() [0x45628a]
    ./perf(cmd_probe+0x6c) [0x4566bc]
    ./perf() [0x47abc5]
    ./perf(main+0x610) [0x421f90]
    /lib64/libc.so.6(__libc_start_main+0xf5) [0x7f5373899af5]
    ./perf() [0x4220a9]
  ----
This pattern clearly shows that the refcnt of the map is acquired
twice ny map__new2 and maps__insert but released once at map_groups_exit.
Since maps__insert already got the dso, we have to put it right
after that which corresponding to the map__new2.

These are happened in machine__findnew_module_map, as below.
  ----
  # eu-addr2line -e ./perf -f 0x4b8aaf
  machine__findnew_module_map inlined at util/machine.c:1046
  in machine__create_module
  util/machine.c:582
  # eu-addr2line -e ./perf -f 0x4b8acb
  map_groups__insert inlined at util/machine.c:585
  in machine__create_module
  util/map.h:208
  ----
(note that both are at util/machine.c:58X which is
 machine__findnew_module_map)
So, this patch fixes machine__findnew_module_map to put the map
right after map_groups__insert.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
 tools/perf/util/machine.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 5ef90be..5ca4064 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -584,6 +584,8 @@ struct map *machine__findnew_module_map(struct machine *machine, u64 start,
 
 	map_groups__insert(&machine->kmaps, map);
 
+	/* Put the map here because map_groups__insert alread got it */
+	map__put(map);
 out:
 	free(m.name);
 	return map;

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1272635 — Re: [PATCH perf/core 05/13] perf: Fix machine__findnew_module_map to put registered map

FromArnaldo Carvalho de Melo <acme@kernel.org>
Date2015-11-18 23:40 +0100
SubjectRe: [PATCH perf/core 05/13] perf: Fix machine__findnew_module_map to put registered map
Message-ID<qwjyq-RO-15@gated-at.bofh.it>
In reply to#1271879
Em Wed, Nov 18, 2015 at 03:40:20PM +0900, Masami Hiramatsu escreveu:
> Fix machine object to put the map object which is already
> insterted to machine->kmaps.
  inserted :-)

Applied!

- Arnaldo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1275557 — [tip:perf/core] perf machine: Fix machine__findnew_module_map to put registered map

Fromtip-bot for Masami Hiramatsu <tipbot@zytor.com>
Date2015-11-23 17:20 +0100
Subject[tip:perf/core] perf machine: Fix machine__findnew_module_map to put registered map
Message-ID<qy20r-4cV-57@gated-at.bofh.it>
In reply to#1271879
Commit-ID:  9afcb420d6cfeadf5d872f395061c611536615fb
Gitweb:     http://git.kernel.org/tip/9afcb420d6cfeadf5d872f395061c611536615fb
Author:     Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
AuthorDate: Wed, 18 Nov 2015 15:40:20 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 19 Nov 2015 13:19:18 -0300

perf machine: Fix machine__findnew_module_map to put registered map

Fix machine object to drop the reference to the map object after it
inserted it into machine->kmaps.

refcnt debugger shows what happened:
  ----
  ==== [2] ====
  Unreclaimed map: 0x346f750
  Refcount +1 => 1 at
    ./perf(map__new2+0xb5) [0x4bdea5]
    ./perf() [0x4b8aaf]
    ./perf(modules__parse+0xfc) [0x4a9cbc]
    ./perf() [0x4b83c0]
    ./perf(machine__create_kernel_maps+0x148) [0x4bb208]
    ./perf(machine__new_host+0xfa) [0x4bb3fa]
    ./perf(init_probe_symbol_maps+0x93) [0x5062b3]
    ./perf() [0x455ffa]
    ./perf(cmd_probe+0x6c) [0x4566bc]
    ./perf() [0x47abc5]
    ./perf(main+0x610) [0x421f90]
    /lib64/libc.so.6(__libc_start_main+0xf5) [0x7f5373899af5]
    ./perf() [0x4220a9]
  Refcount +1 => 2 at
    ./perf(maps__insert+0x9a) [0x4bfd4a]
    ./perf() [0x4b8acb]
    ./perf(modules__parse+0xfc) [0x4a9cbc]
    ./perf() [0x4b83c0]
    ./perf(machine__create_kernel_maps+0x148) [0x4bb208]
    ./perf(machine__new_host+0xfa) [0x4bb3fa]
    ./perf(init_probe_symbol_maps+0x93) [0x5062b3]
    ./perf() [0x455ffa]
    ./perf(cmd_probe+0x6c) [0x4566bc]
    ./perf() [0x47abc5]
    ./perf(main+0x610) [0x421f90]
    /lib64/libc.so.6(__libc_start_main+0xf5) [0x7f5373899af5]
    ./perf() [0x4220a9]
  Refcount -1 => 1 at
    ./perf(map_groups__exit+0x94) [0x4bea54]
    ./perf(machine__delete+0x3d) [0x4b91ed]
    ./perf(exit_probe_symbol_maps+0x28) [0x506358]
    ./perf() [0x45628a]
    ./perf(cmd_probe+0x6c) [0x4566bc]
    ./perf() [0x47abc5]
    ./perf(main+0x610) [0x421f90]
    /lib64/libc.so.6(__libc_start_main+0xf5) [0x7f5373899af5]
    ./perf() [0x4220a9]
  ----

This pattern clearly shows that the refcnt of the map is acquired twice
by map__new2 and maps__insert but released onlu once at
map_groups__exit, when we purge its maps rbtree.

Since maps__insert already reference counted the map, we have to drop
the constructor (map__new2) reference count right after inserting it.

These happened in machine__findnew_module_map, as below.

  ----
  # eu-addr2line -e ./perf -f 0x4b8aaf
  machine__findnew_module_map inlined at util/machine.c:1046
  in machine__create_module
  util/machine.c:582
  # eu-addr2line -e ./perf -f 0x4b8acb
  map_groups__insert inlined at util/machine.c:585
  in machine__create_module
  util/map.h:208
  ----

(note that both are at util/machine.c:58X which is
 machine__findnew_module_map)

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/20151118064020.30709.40499.stgit@localhost.localdomain
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/machine.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 8b303ff..0487d77 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -585,6 +585,8 @@ struct map *machine__findnew_module_map(struct machine *machine, u64 start,
 
 	map_groups__insert(&machine->kmaps, map);
 
+	/* Put the map here because map_groups__insert alread got it */
+	map__put(map);
 out:
 	free(m.name);
 	return map;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1271880 — [PATCH perf/core 09/13] perf: Make dso to use refcnt for debug

FromMasami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Date2015-11-18 07:50 +0100
Subject[PATCH perf/core 09/13] perf: Make dso to use refcnt for debug
Message-ID<qw4J4-7HS-25@gated-at.bofh.it>
In reply to#1271875
Make 'dso' object to use refcnt interface for debug.
This can find refcnt related memory leaks on dsos.

E.g.
  ----
  # ./perf probe vfs_read
  Added new event:
    probe:vfs_read       (on vfs_read)

  You can now use it in all perf tools, such as:

          perf record -e probe:vfs_read -aR sleep 1

  REFCNT: BUG: Unreclaimed objects found.
  REFCNT: Total 75 objects are not reclaimed.
     To see all backtraces, rerun with -v option
  ----

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
 tools/perf/util/dso.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 7c0c083..28ad06a 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -1049,7 +1049,7 @@ struct dso *dso__new(const char *name)
 		INIT_LIST_HEAD(&dso->node);
 		INIT_LIST_HEAD(&dso->data.open_entry);
 		pthread_mutex_init(&dso->lock, NULL);
-		atomic_set(&dso->refcnt, 1);
+		refcnt__init(dso, refcnt);
 	}
 
 	return dso;
@@ -1081,19 +1081,20 @@ void dso__delete(struct dso *dso)
 	dso__free_a2l(dso);
 	zfree(&dso->symsrc_filename);
 	pthread_mutex_destroy(&dso->lock);
+	refcnt__exit(dso, refcnt);
 	free(dso);
 }
 
 struct dso *dso__get(struct dso *dso)
 {
 	if (dso)
-		atomic_inc(&dso->refcnt);
+		refcnt__get(dso, refcnt);
 	return dso;
 }
 
 void dso__put(struct dso *dso)
 {
-	if (dso && atomic_dec_and_test(&dso->refcnt))
+	if (dso && refcnt__put(dso, refcnt))
 		dso__delete(dso);
 }
 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1271881 — [PATCH perf/core 02/13] perf: Make perf_exec_path always returns malloc'd string

FromMasami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Date2015-11-18 08:00 +0100
Subject[PATCH perf/core 02/13] perf: Make perf_exec_path always returns malloc'd string
Message-ID<qw4SK-7Nb-1@gated-at.bofh.it>
In reply to#1271875
Since system_path() returns malloc'd string if given path is
not an absolute path, perf_exec_path sometimes returns static
string and sometimes returns malloc'd string depends on the
environment variables or command options.

This causes a memory leak because caller can not free the
returned string.

This fixes perf_exec_path and system_path to always return
malloc'd string, so caller can always free it.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
 tools/perf/util/exec_cmd.c |   20 ++++++++++++--------
 tools/perf/util/exec_cmd.h |    5 +++--
 tools/perf/util/help.c     |    6 ++++--
 3 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/tools/perf/util/exec_cmd.c b/tools/perf/util/exec_cmd.c
index 7adf4ad..7031ffc 100644
--- a/tools/perf/util/exec_cmd.c
+++ b/tools/perf/util/exec_cmd.c
@@ -9,17 +9,17 @@
 static const char *argv_exec_path;
 static const char *argv0_path;
 
-const char *system_path(const char *path)
+char *system_path(const char *path)
 {
 	static const char *prefix = PREFIX;
 	struct strbuf d = STRBUF_INIT;
 
 	if (is_absolute_path(path))
-		return path;
+		return strdup(path);
 
 	strbuf_addf(&d, "%s/%s", prefix, path);
 	path = strbuf_detach(&d, NULL);
-	return path;
+	return (char *)path;
 }
 
 const char *perf_extract_argv0_path(const char *argv0)
@@ -52,16 +52,18 @@ void perf_set_argv_exec_path(const char *exec_path)
 
 
 /* Returns the highest-priority, location to look for perf programs. */
-const char *perf_exec_path(void)
+char *perf_exec_path(void)
 {
-	const char *env;
+	char *env;
 
 	if (argv_exec_path)
-		return argv_exec_path;
+		return strdup(argv_exec_path);
 
 	env = getenv(EXEC_PATH_ENVIRONMENT);
 	if (env && *env) {
-		return env;
+		env = strdup(env);
+		if (env)
+			return env;
 	}
 
 	return system_path(PERF_EXEC_PATH);
@@ -83,9 +85,11 @@ void setup_path(void)
 {
 	const char *old_path = getenv("PATH");
 	struct strbuf new_path = STRBUF_INIT;
+	char *tmp = perf_exec_path();
 
-	add_path(&new_path, perf_exec_path());
+	add_path(&new_path, tmp);
 	add_path(&new_path, argv0_path);
+	free(tmp);
 
 	if (old_path)
 		strbuf_addstr(&new_path, old_path);
diff --git a/tools/perf/util/exec_cmd.h b/tools/perf/util/exec_cmd.h
index bc4b915..48b4175 100644
--- a/tools/perf/util/exec_cmd.h
+++ b/tools/perf/util/exec_cmd.h
@@ -3,10 +3,11 @@
 
 extern void perf_set_argv_exec_path(const char *exec_path);
 extern const char *perf_extract_argv0_path(const char *path);
-extern const char *perf_exec_path(void);
 extern void setup_path(void);
 extern int execv_perf_cmd(const char **argv); /* NULL terminated */
 extern int execl_perf_cmd(const char *cmd, ...);
-extern const char *system_path(const char *path);
+/* perf_exec_path and system_path return malloc'd string, caller must free it */
+extern char *perf_exec_path(void);
+extern char *system_path(const char *path);
 
 #endif /* __PERF_EXEC_CMD_H */
diff --git a/tools/perf/util/help.c b/tools/perf/util/help.c
index 86c37c4..fa1fc4a 100644
--- a/tools/perf/util/help.c
+++ b/tools/perf/util/help.c
@@ -159,7 +159,7 @@ void load_command_list(const char *prefix,
 		struct cmdnames *other_cmds)
 {
 	const char *env_path = getenv("PATH");
-	const char *exec_path = perf_exec_path();
+	char *exec_path = perf_exec_path();
 
 	if (exec_path) {
 		list_commands_in_dir(main_cmds, exec_path, prefix);
@@ -187,6 +187,7 @@ void load_command_list(const char *prefix,
 		      sizeof(*other_cmds->names), cmdname_compare);
 		uniq(other_cmds);
 	}
+	free(exec_path);
 	exclude_cmds(other_cmds, main_cmds);
 }
 
@@ -203,13 +204,14 @@ void list_commands(const char *title, struct cmdnames *main_cmds,
 			longest = other_cmds->names[i]->len;
 
 	if (main_cmds->cnt) {
-		const char *exec_path = perf_exec_path();
+		char *exec_path = perf_exec_path();
 		printf("available %s in '%s'\n", title, exec_path);
 		printf("----------------");
 		mput_char('-', strlen(title) + strlen(exec_path));
 		putchar('\n');
 		pretty_print_string_list(main_cmds, longest);
 		putchar('\n');
+		free(exec_path);
 	}
 
 	if (other_cmds->cnt) {

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1271884 — [PATCH perf/core 03/13] perf: Introduce generic refcount APIs with debug feature

FromMasami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Date2015-11-18 08:00 +0100
Subject[PATCH perf/core 03/13] perf: Introduce generic refcount APIs with debug feature
Message-ID<qw4SK-7Nb-9@gated-at.bofh.it>
In reply to#1271875
This is a kind of debugging feature for atomic reference counter.
The reference counters are widely used in perf tools but not well
debugged. It sometimes causes memory leaks but no one has noticed
the issue, since it is hard to debug such un-reclaimed objects.

This refcnt interface provides fully backtrace debug feature to
analyze such issue. User just replace atomic_t ops with refcnt
APIs and add refcnt__exit() where the object is released.

/* At object initializing */
refcnt__init(obj, refcnt); /* <- atomic_set(&obj->refcnt, 1); */

/* At object get method */
refcnt__get(obj, refcnt); /* <- atomic_inc(&obj->refcnt); */

/* At object put method */
if (obj && refcnt__put(obj, refcnt)) /* <-atmoic_dec_and_test(&obj->refcnt)*/

/* At object releasing */
refcnt__exit(obj, refcnt); /* <- Newly added */

The debugging feature is enabled when building perf with
REFCNT_DEBUG=1. Otherwides it is just translated as normal
atomic ops.

Debugging binary warns you if it finds leaks when the perf exits.
If you give -v (or --verbose) to the perf, it also shows backtrace
logs on all refcnt operations of leaked objects.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
 tools/perf/config/Makefile |    5 ++
 tools/perf/util/Build      |    1 
 tools/perf/util/refcnt.c   |  125 ++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/refcnt.h   |   65 +++++++++++++++++++++++
 4 files changed, 196 insertions(+)
 create mode 100644 tools/perf/util/refcnt.c
 create mode 100644 tools/perf/util/refcnt.h

diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
index de89ec5..efc2e03 100644
--- a/tools/perf/config/Makefile
+++ b/tools/perf/config/Makefile
@@ -147,6 +147,11 @@ ifdef PARSER_DEBUG
   $(call detected_var,PARSER_DEBUG_FLEX)
 endif
 
+ifdef REFCNT_DEBUG
+  CFLAGS += -DREFCNT_DEBUG
+  $(call detected,CONFIG_REFCNT_DEBUG)
+endif
+
 ifndef NO_LIBPYTHON
   # Try different combinations to accommodate systems that only have
   # python[2][-config] in weird combinations but always preferring
diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 591b3fe..f64d2a4 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -86,6 +86,7 @@ libperf-$(CONFIG_AUXTRACE) += intel-pt.o
 libperf-$(CONFIG_AUXTRACE) += intel-bts.o
 libperf-y += parse-branch-options.o
 libperf-y += parse-regs-options.o
+libperf-$(CONFIG_REFCNT_DEBUG) += refcnt.o
 
 libperf-$(CONFIG_LIBBPF) += bpf-loader.o
 libperf-$(CONFIG_LIBELF) += symbol-elf.o
diff --git a/tools/perf/util/refcnt.c b/tools/perf/util/refcnt.c
new file mode 100644
index 0000000..f828419
--- /dev/null
+++ b/tools/perf/util/refcnt.c
@@ -0,0 +1,125 @@
+/* Refcount backtrace for debugging leaks */
+#include "../perf.h"
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <execinfo.h>	/* For backtrace */
+
+#include "event.h"
+#include "debug.h"
+#include "util.h"
+#include "refcnt.h"
+
+/* A root of backtrace */
+static LIST_HEAD(refcnt_root);	/* List head of refcnt object */
+
+static void __refcnt_object__delete(struct refcnt_object *ref)
+{
+	struct refcnt_buffer *buf;
+
+	while (!list_empty(&ref->head)) {
+		buf = list_entry(ref->head.next, struct refcnt_buffer, list);
+		list_del_init(&buf->list);
+		free(buf);
+	}
+	list_del_init(&ref->list);
+	free(ref);
+}
+
+static struct refcnt_object *refcnt__find_object(void *obj)
+{
+	struct refcnt_object *ref;
+
+	/* TODO: use hash list */
+	list_for_each_entry(ref, &refcnt_root, list)
+		if (ref->obj == obj)
+			return ref;
+
+	return NULL;
+}
+
+void refcnt_object__delete(void *addr)
+{
+	struct refcnt_object *ref = refcnt__find_object(addr);
+
+	if (!ref) {
+		pr_debug("REFCNT: Delete uninitialized refcnt: %p\n", addr);
+		return;
+	}
+	__refcnt_object__delete(ref);
+}
+
+void refcnt_object__record(void *obj, const char *name, int count)
+{
+	struct refcnt_object *ref = refcnt__find_object(obj);
+	struct refcnt_buffer *buf;
+
+	/* If no entry, allocate new one */
+	if (!ref) {
+		ref = malloc(sizeof(*ref));
+		if (!ref) {
+			pr_debug("REFCNT: Out of memory for %p (%s)\n",
+				 obj, name);
+			return;
+		}
+		INIT_LIST_HEAD(&ref->list);
+		INIT_LIST_HEAD(&ref->head);
+		ref->name = name;
+		ref->obj = obj;
+		list_add_tail(&ref->list, &refcnt_root);
+	}
+
+	buf = malloc(sizeof(*buf));
+	if (!buf) {
+		pr_debug("REFCNT: Out of memory for %p (%s)\n", obj, ref->name);
+		return;
+	}
+
+	INIT_LIST_HEAD(&buf->list);
+	buf->count = count;
+	buf->nr = backtrace(buf->buf, BACKTRACE_SIZE);
+	list_add_tail(&buf->list, &ref->head);
+}
+
+static void pr_refcnt_buffer(struct refcnt_buffer *buf)
+{
+	char **symbuf;
+	int i;
+
+	if (!buf)
+		return;
+	symbuf = backtrace_symbols(buf->buf, buf->nr);
+	/* Skip the first one because it is always btrace__record */
+	for (i = 1; i < buf->nr; i++)
+		pr_debug("  %s\n", symbuf[i]);
+	free(symbuf);
+}
+
+void refcnt__dump_unreclaimed(void) __attribute__((destructor));
+void refcnt__dump_unreclaimed(void)
+{
+	struct refcnt_object *ref, *n;
+	struct refcnt_buffer *buf;
+	int i = 0;
+
+	if (list_empty(&refcnt_root))
+		return;
+
+	pr_warning("REFCNT: BUG: Unreclaimed objects found.\n");
+	list_for_each_entry_safe(ref, n, &refcnt_root, list) {
+		pr_debug("==== [%d] ====\nUnreclaimed %s: %p\n", i,
+			 ref->name ? ref->name : "(object)", ref->obj);
+		list_for_each_entry(buf, &ref->head, list) {
+			pr_debug("Refcount %s => %d at\n",
+				 buf->count > 0 ? "+1" : "-1",
+				 buf->count > 0 ? buf->count : -buf->count - 1);
+			pr_refcnt_buffer(buf);
+		}
+		__refcnt_object__delete(ref);
+		i++;
+	}
+	pr_warning("REFCNT: Total %d objects are not reclaimed.\n", i);
+	if (!verbose)
+		pr_warning("   To see all backtraces, rerun with -v option\n");
+}
+
diff --git a/tools/perf/util/refcnt.h b/tools/perf/util/refcnt.h
new file mode 100644
index 0000000..a8e2d29
--- /dev/null
+++ b/tools/perf/util/refcnt.h
@@ -0,0 +1,65 @@
+/*
+ * Atomic reference counter API with debugging feature
+ */
+#ifndef __PERF_REFCNT_H
+#define __PERF_REFCNT_H
+
+#include <linux/atomic.h>
+
+#ifdef REFCNT_DEBUG
+
+struct refcnt_object {
+	struct list_head	list;	/* List of objects */
+	void			*obj;	/* Object address which has refcnt */
+	const char		*name;	/* Object class name */
+	struct list_head	head;	/* List head for buffers */
+};
+
+#define BACKTRACE_SIZE 32
+struct refcnt_buffer {
+	struct list_head	list;	/* List of buffers */
+	int			count;	/* Count number at recording point */
+	int			nr;	/* Number of recorded buffer entries */
+	void			*buf[BACKTRACE_SIZE];	/* Backtrace buffer */
+};
+
+void refcnt_object__record(void *obj, const char *name, int count);
+void refcnt_object__delete(void *obj);
+
+static inline void __refcnt__init(atomic_t *refcnt, void *obj, const char *name)
+{
+	atomic_set(refcnt, 1);
+	refcnt_object__record(obj, name, 1);
+}
+
+static inline void __refcnt__get(atomic_t *refcnt, void *obj)
+{
+	atomic_inc(refcnt);
+	refcnt_object__record(obj, NULL, atomic_read(refcnt));
+}
+
+static inline int __refcnt__put(atomic_t *refcnt, void *obj)
+{
+	refcnt_object__record(obj, NULL, -atomic_read(refcnt));
+	return atomic_dec_and_test(refcnt);
+}
+
+#define refcnt__init(obj, member)	\
+	__refcnt__init(&obj->member, obj, #obj)
+#define refcnt__exit(obj, member)	\
+	refcnt_object__delete(obj)
+#define refcnt__get(obj, member)	\
+	__refcnt__get(&obj->member, obj)
+#define refcnt__put(obj, member)	\
+	__refcnt__put(&obj->member, obj)
+
+#else	/* !REFCNT_DEBUG */
+
+#define refcnt__init(obj, member)	atomic_set(&obj->member, 1)
+#define refcnt__exit(obj, member)	do { } while (0)
+#define refcnt__get(obj, member)	atomic_inc(&obj->member)
+#define refcnt__put(obj, member)	atomic_dec_and_test(&obj->member)
+
+#endif	/* !REFCNT_DEBUG */
+
+#endif	/* __PERF_REFCNT_H */

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1273706 — Re: [PATCH perf/core 03/13] perf: Introduce generic refcount APIs with debug feature

FromNamhyung Kim <namhyung@kernel.org>
Date2015-11-20 04:00 +0100
SubjectRe: [PATCH perf/core 03/13] perf: Introduce generic refcount APIs with debug feature
Message-ID<qwK5z-1aE-7@gated-at.bofh.it>
In reply to#1271884
On Wed, Nov 18, 2015 at 03:40:16PM +0900, Masami Hiramatsu wrote:
> This is a kind of debugging feature for atomic reference counter.
> The reference counters are widely used in perf tools but not well
> debugged. It sometimes causes memory leaks but no one has noticed
> the issue, since it is hard to debug such un-reclaimed objects.
> 
> This refcnt interface provides fully backtrace debug feature to
> analyze such issue. User just replace atomic_t ops with refcnt
> APIs and add refcnt__exit() where the object is released.
> 
> /* At object initializing */
> refcnt__init(obj, refcnt); /* <- atomic_set(&obj->refcnt, 1); */
> 
> /* At object get method */
> refcnt__get(obj, refcnt); /* <- atomic_inc(&obj->refcnt); */
> 
> /* At object put method */
> if (obj && refcnt__put(obj, refcnt)) /* <-atmoic_dec_and_test(&obj->refcnt)*/
> 
> /* At object releasing */
> refcnt__exit(obj, refcnt); /* <- Newly added */
> 
> The debugging feature is enabled when building perf with
> REFCNT_DEBUG=1. Otherwides it is just translated as normal
> atomic ops.
> 
> Debugging binary warns you if it finds leaks when the perf exits.
> If you give -v (or --verbose) to the perf, it also shows backtrace
> logs on all refcnt operations of leaked objects.
> 
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>

Looks really useful!

Acked-by: Namhyung Kim <namhyung@kernel.org>

Just a nitpick below..


> ---

[SNIP]
> +void refcnt_object__record(void *obj, const char *name, int count)
> +{
> +	struct refcnt_object *ref = refcnt__find_object(obj);
> +	struct refcnt_buffer *buf;
> +
> +	/* If no entry, allocate new one */
> +	if (!ref) {
> +		ref = malloc(sizeof(*ref));
> +		if (!ref) {
> +			pr_debug("REFCNT: Out of memory for %p (%s)\n",
> +				 obj, name);
> +			return;
> +		}
> +		INIT_LIST_HEAD(&ref->list);
> +		INIT_LIST_HEAD(&ref->head);
> +		ref->name = name;
> +		ref->obj = obj;
> +		list_add_tail(&ref->list, &refcnt_root);
> +	}
> +
> +	buf = malloc(sizeof(*buf));
> +	if (!buf) {
> +		pr_debug("REFCNT: Out of memory for %p (%s)\n", obj, ref->name);
> +		return;
> +	}
> +
> +	INIT_LIST_HEAD(&buf->list);
> +	buf->count = count;
> +	buf->nr = backtrace(buf->buf, BACKTRACE_SIZE);
> +	list_add_tail(&buf->list, &ref->head);
> +}
> +
> +static void pr_refcnt_buffer(struct refcnt_buffer *buf)
> +{
> +	char **symbuf;
> +	int i;
> +
> +	if (!buf)
> +		return;
> +	symbuf = backtrace_symbols(buf->buf, buf->nr);

It seems you need to check the return value.  Maybe we can use
backtrace_symbols_fd() instead, or just in case of an error.

Thanks,
Namhyung


> +	/* Skip the first one because it is always btrace__record */
> +	for (i = 1; i < buf->nr; i++)
> +		pr_debug("  %s\n", symbuf[i]);
> +	free(symbuf);
> +}
> +
> +void refcnt__dump_unreclaimed(void) __attribute__((destructor));
> +void refcnt__dump_unreclaimed(void)
> +{
> +	struct refcnt_object *ref, *n;
> +	struct refcnt_buffer *buf;
> +	int i = 0;
> +
> +	if (list_empty(&refcnt_root))
> +		return;
> +
> +	pr_warning("REFCNT: BUG: Unreclaimed objects found.\n");
> +	list_for_each_entry_safe(ref, n, &refcnt_root, list) {
> +		pr_debug("==== [%d] ====\nUnreclaimed %s: %p\n", i,
> +			 ref->name ? ref->name : "(object)", ref->obj);
> +		list_for_each_entry(buf, &ref->head, list) {
> +			pr_debug("Refcount %s => %d at\n",
> +				 buf->count > 0 ? "+1" : "-1",
> +				 buf->count > 0 ? buf->count : -buf->count - 1);
> +			pr_refcnt_buffer(buf);
> +		}
> +		__refcnt_object__delete(ref);
> +		i++;
> +	}
> +	pr_warning("REFCNT: Total %d objects are not reclaimed.\n", i);
> +	if (!verbose)
> +		pr_warning("   To see all backtraces, rerun with -v option\n");
> +}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1273717 — RE: [PATCH perf/core 03/13] perf: Introduce generic refcount APIs with debug feature

From平松雅巳 / HIRAMATU,MASAMI <masami.hiramatsu.pt@hitachi.com>
Date2015-11-20 05:20 +0100
SubjectRE: [PATCH perf/core 03/13] perf: Introduce generic refcount APIs with debug feature
Message-ID<qwLkZ-2dC-1@gated-at.bofh.it>
In reply to#1273706
RnJvbTogTmFtaHl1bmcgS2ltIFttYWlsdG86bmFtaHl1bmdAa2VybmVsLm9yZ10NCj4NCj5PbiBX
ZWQsIE5vdiAxOCwgMjAxNSBhdCAwMzo0MDoxNlBNICswOTAwLCBNYXNhbWkgSGlyYW1hdHN1IHdy
b3RlOg0KPj4gVGhpcyBpcyBhIGtpbmQgb2YgZGVidWdnaW5nIGZlYXR1cmUgZm9yIGF0b21pYyBy
ZWZlcmVuY2UgY291bnRlci4NCj4+IFRoZSByZWZlcmVuY2UgY291bnRlcnMgYXJlIHdpZGVseSB1
c2VkIGluIHBlcmYgdG9vbHMgYnV0IG5vdCB3ZWxsDQo+PiBkZWJ1Z2dlZC4gSXQgc29tZXRpbWVz
IGNhdXNlcyBtZW1vcnkgbGVha3MgYnV0IG5vIG9uZSBoYXMgbm90aWNlZA0KPj4gdGhlIGlzc3Vl
LCBzaW5jZSBpdCBpcyBoYXJkIHRvIGRlYnVnIHN1Y2ggdW4tcmVjbGFpbWVkIG9iamVjdHMuDQo+
Pg0KPj4gVGhpcyByZWZjbnQgaW50ZXJmYWNlIHByb3ZpZGVzIGZ1bGx5IGJhY2t0cmFjZSBkZWJ1
ZyBmZWF0dXJlIHRvDQo+PiBhbmFseXplIHN1Y2ggaXNzdWUuIFVzZXIganVzdCByZXBsYWNlIGF0
b21pY190IG9wcyB3aXRoIHJlZmNudA0KPj4gQVBJcyBhbmQgYWRkIHJlZmNudF9fZXhpdCgpIHdo
ZXJlIHRoZSBvYmplY3QgaXMgcmVsZWFzZWQuDQo+Pg0KPj4gLyogQXQgb2JqZWN0IGluaXRpYWxp
emluZyAqLw0KPj4gcmVmY250X19pbml0KG9iaiwgcmVmY250KTsgLyogPC0gYXRvbWljX3NldCgm
b2JqLT5yZWZjbnQsIDEpOyAqLw0KPj4NCj4+IC8qIEF0IG9iamVjdCBnZXQgbWV0aG9kICovDQo+
PiByZWZjbnRfX2dldChvYmosIHJlZmNudCk7IC8qIDwtIGF0b21pY19pbmMoJm9iai0+cmVmY250
KTsgKi8NCj4+DQo+PiAvKiBBdCBvYmplY3QgcHV0IG1ldGhvZCAqLw0KPj4gaWYgKG9iaiAmJiBy
ZWZjbnRfX3B1dChvYmosIHJlZmNudCkpIC8qIDwtYXRtb2ljX2RlY19hbmRfdGVzdCgmb2JqLT5y
ZWZjbnQpKi8NCj4+DQo+PiAvKiBBdCBvYmplY3QgcmVsZWFzaW5nICovDQo+PiByZWZjbnRfX2V4
aXQob2JqLCByZWZjbnQpOyAvKiA8LSBOZXdseSBhZGRlZCAqLw0KPj4NCj4+IFRoZSBkZWJ1Z2dp
bmcgZmVhdHVyZSBpcyBlbmFibGVkIHdoZW4gYnVpbGRpbmcgcGVyZiB3aXRoDQo+PiBSRUZDTlRf
REVCVUc9MS4gT3RoZXJ3aWRlcyBpdCBpcyBqdXN0IHRyYW5zbGF0ZWQgYXMgbm9ybWFsDQo+PiBh
dG9taWMgb3BzLg0KPj4NCj4+IERlYnVnZ2luZyBiaW5hcnkgd2FybnMgeW91IGlmIGl0IGZpbmRz
IGxlYWtzIHdoZW4gdGhlIHBlcmYgZXhpdHMuDQo+PiBJZiB5b3UgZ2l2ZSAtdiAob3IgLS12ZXJi
b3NlKSB0byB0aGUgcGVyZiwgaXQgYWxzbyBzaG93cyBiYWNrdHJhY2UNCj4+IGxvZ3Mgb24gYWxs
IHJlZmNudCBvcGVyYXRpb25zIG9mIGxlYWtlZCBvYmplY3RzLg0KPj4NCj4+IFNpZ25lZC1vZmYt
Ynk6IE1hc2FtaSBIaXJhbWF0c3UgPG1hc2FtaS5oaXJhbWF0c3UucHRAaGl0YWNoaS5jb20+DQo+
DQo+TG9va3MgcmVhbGx5IHVzZWZ1bCENCj4NCj5BY2tlZC1ieTogTmFtaHl1bmcgS2ltIDxuYW1o
eXVuZ0BrZXJuZWwub3JnPg0KPg0KPkp1c3QgYSBuaXRwaWNrIGJlbG93Li4NCg0KVGhhbmtzIQ0K
DQo+PiAtLS0NCj4NCj5bU05JUF0NCj4+ICt2b2lkIHJlZmNudF9vYmplY3RfX3JlY29yZCh2b2lk
ICpvYmosIGNvbnN0IGNoYXIgKm5hbWUsIGludCBjb3VudCkNCj4+ICt7DQo+PiArCXN0cnVjdCBy
ZWZjbnRfb2JqZWN0ICpyZWYgPSByZWZjbnRfX2ZpbmRfb2JqZWN0KG9iaik7DQo+PiArCXN0cnVj
dCByZWZjbnRfYnVmZmVyICpidWY7DQo+PiArDQo+PiArCS8qIElmIG5vIGVudHJ5LCBhbGxvY2F0
ZSBuZXcgb25lICovDQo+PiArCWlmICghcmVmKSB7DQo+PiArCQlyZWYgPSBtYWxsb2Moc2l6ZW9m
KCpyZWYpKTsNCj4+ICsJCWlmICghcmVmKSB7DQo+PiArCQkJcHJfZGVidWcoIlJFRkNOVDogT3V0
IG9mIG1lbW9yeSBmb3IgJXAgKCVzKVxuIiwNCj4+ICsJCQkJIG9iaiwgbmFtZSk7DQo+PiArCQkJ
cmV0dXJuOw0KPj4gKwkJfQ0KPj4gKwkJSU5JVF9MSVNUX0hFQUQoJnJlZi0+bGlzdCk7DQo+PiAr
CQlJTklUX0xJU1RfSEVBRCgmcmVmLT5oZWFkKTsNCj4+ICsJCXJlZi0+bmFtZSA9IG5hbWU7DQo+
PiArCQlyZWYtPm9iaiA9IG9iajsNCj4+ICsJCWxpc3RfYWRkX3RhaWwoJnJlZi0+bGlzdCwgJnJl
ZmNudF9yb290KTsNCj4+ICsJfQ0KPj4gKw0KPj4gKwlidWYgPSBtYWxsb2Moc2l6ZW9mKCpidWYp
KTsNCj4+ICsJaWYgKCFidWYpIHsNCj4+ICsJCXByX2RlYnVnKCJSRUZDTlQ6IE91dCBvZiBtZW1v
cnkgZm9yICVwICglcylcbiIsIG9iaiwgcmVmLT5uYW1lKTsNCj4+ICsJCXJldHVybjsNCj4+ICsJ
fQ0KPj4gKw0KPj4gKwlJTklUX0xJU1RfSEVBRCgmYnVmLT5saXN0KTsNCj4+ICsJYnVmLT5jb3Vu
dCA9IGNvdW50Ow0KPj4gKwlidWYtPm5yID0gYmFja3RyYWNlKGJ1Zi0+YnVmLCBCQUNLVFJBQ0Vf
U0laRSk7DQo+PiArCWxpc3RfYWRkX3RhaWwoJmJ1Zi0+bGlzdCwgJnJlZi0+aGVhZCk7DQo+PiAr
fQ0KPj4gKw0KPj4gK3N0YXRpYyB2b2lkIHByX3JlZmNudF9idWZmZXIoc3RydWN0IHJlZmNudF9i
dWZmZXIgKmJ1ZikNCj4+ICt7DQo+PiArCWNoYXIgKipzeW1idWY7DQo+PiArCWludCBpOw0KPj4g
Kw0KPj4gKwlpZiAoIWJ1ZikNCj4+ICsJCXJldHVybjsNCj4+ICsJc3ltYnVmID0gYmFja3RyYWNl
X3N5bWJvbHMoYnVmLT5idWYsIGJ1Zi0+bnIpOw0KPg0KPkl0IHNlZW1zIHlvdSBuZWVkIHRvIGNo
ZWNrIHRoZSByZXR1cm4gdmFsdWUuICBNYXliZSB3ZSBjYW4gdXNlDQo+YmFja3RyYWNlX3N5bWJv
bHNfZmQoKSBpbnN0ZWFkLCBvciBqdXN0IGluIGNhc2Ugb2YgYW4gZXJyb3IuDQoNClllYWgsIGl0
IHNob3VsZCBiZSBjaGVja2VkIGFuZCBpbiB0aGF0IGNhc2Ugd2UgY2FuIGZhbGwgYmFjayB0bw0K
YmFja3RyYWNlX3N5bWJvbHNfZmQoYXMgdGhlIGxhc3QgcmVzb3J0KSwgYnV0IEkgZG9u4oCZdCBs
aWtlDQpiYWNrdHJhY2Vfc3ltYm9sc19mZCByZXBsYWNpbmcgYmVjYXVzZSBpdCBkb2Vzbid0IGFs
bG93IHVzIHRvDQppbmRlbnQgdGhlIGJhY2t0cmFjZSByZXN1bHQuDQoNClRoYW5rIHlvdSwNCg0K
DQo+DQo+VGhhbmtzLA0KPk5hbWh5dW5nDQo+DQo+DQo+PiArCS8qIFNraXAgdGhlIGZpcnN0IG9u
ZSBiZWNhdXNlIGl0IGlzIGFsd2F5cyBidHJhY2VfX3JlY29yZCAqLw0KPj4gKwlmb3IgKGkgPSAx
OyBpIDwgYnVmLT5ucjsgaSsrKQ0KPj4gKwkJcHJfZGVidWcoIiAgJXNcbiIsIHN5bWJ1ZltpXSk7
DQo+PiArCWZyZWUoc3ltYnVmKTsNCj4+ICt9DQo+PiArDQo+PiArdm9pZCByZWZjbnRfX2R1bXBf
dW5yZWNsYWltZWQodm9pZCkgX19hdHRyaWJ1dGVfXygoZGVzdHJ1Y3RvcikpOw0KPj4gK3ZvaWQg
cmVmY250X19kdW1wX3VucmVjbGFpbWVkKHZvaWQpDQo+PiArew0KPj4gKwlzdHJ1Y3QgcmVmY250
X29iamVjdCAqcmVmLCAqbjsNCj4+ICsJc3RydWN0IHJlZmNudF9idWZmZXIgKmJ1ZjsNCj4+ICsJ
aW50IGkgPSAwOw0KPj4gKw0KPj4gKwlpZiAobGlzdF9lbXB0eSgmcmVmY250X3Jvb3QpKQ0KPj4g
KwkJcmV0dXJuOw0KPj4gKw0KPj4gKwlwcl93YXJuaW5nKCJSRUZDTlQ6IEJVRzogVW5yZWNsYWlt
ZWQgb2JqZWN0cyBmb3VuZC5cbiIpOw0KPj4gKwlsaXN0X2Zvcl9lYWNoX2VudHJ5X3NhZmUocmVm
LCBuLCAmcmVmY250X3Jvb3QsIGxpc3QpIHsNCj4+ICsJCXByX2RlYnVnKCI9PT09IFslZF0gPT09
PVxuVW5yZWNsYWltZWQgJXM6ICVwXG4iLCBpLA0KPj4gKwkJCSByZWYtPm5hbWUgPyByZWYtPm5h
bWUgOiAiKG9iamVjdCkiLCByZWYtPm9iaik7DQo+PiArCQlsaXN0X2Zvcl9lYWNoX2VudHJ5KGJ1
ZiwgJnJlZi0+aGVhZCwgbGlzdCkgew0KPj4gKwkJCXByX2RlYnVnKCJSZWZjb3VudCAlcyA9PiAl
ZCBhdFxuIiwNCj4+ICsJCQkJIGJ1Zi0+Y291bnQgPiAwID8gIisxIiA6ICItMSIsDQo+PiArCQkJ
CSBidWYtPmNvdW50ID4gMCA/IGJ1Zi0+Y291bnQgOiAtYnVmLT5jb3VudCAtIDEpOw0KPj4gKwkJ
CXByX3JlZmNudF9idWZmZXIoYnVmKTsNCj4+ICsJCX0NCj4+ICsJCV9fcmVmY250X29iamVjdF9f
ZGVsZXRlKHJlZik7DQo+PiArCQlpKys7DQo+PiArCX0NCj4+ICsJcHJfd2FybmluZygiUkVGQ05U
OiBUb3RhbCAlZCBvYmplY3RzIGFyZSBub3QgcmVjbGFpbWVkLlxuIiwgaSk7DQo+PiArCWlmICgh
dmVyYm9zZSkNCj4+ICsJCXByX3dhcm5pbmcoIiAgIFRvIHNlZSBhbGwgYmFja3RyYWNlcywgcmVy
dW4gd2l0aCAtdiBvcHRpb25cbiIpOw0KPj4gK30NCg==
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1273738 — Re: [PATCH perf/core 03/13] perf: Introduce generic refcount APIs with debug feature

FromNamhyung Kim <namhyung@kernel.org>
Date2015-11-20 07:00 +0100
SubjectRe: [PATCH perf/core 03/13] perf: Introduce generic refcount APIs with debug feature
Message-ID<qwMTM-35c-7@gated-at.bofh.it>
In reply to#1273717
On Fri, Nov 20, 2015 at 04:12:06AM +0000, 平松雅巳 / HIRAMATU,MASAMI wrote:
> From: Namhyung Kim [mailto:namhyung@kernel.org]
> >
> >On Wed, Nov 18, 2015 at 03:40:16PM +0900, Masami Hiramatsu wrote:
> >> This is a kind of debugging feature for atomic reference counter.
> >> The reference counters are widely used in perf tools but not well
> >> debugged. It sometimes causes memory leaks but no one has noticed
> >> the issue, since it is hard to debug such un-reclaimed objects.
> >>
> >> This refcnt interface provides fully backtrace debug feature to
> >> analyze such issue. User just replace atomic_t ops with refcnt
> >> APIs and add refcnt__exit() where the object is released.
> >>
> >> /* At object initializing */
> >> refcnt__init(obj, refcnt); /* <- atomic_set(&obj->refcnt, 1); */
> >>
> >> /* At object get method */
> >> refcnt__get(obj, refcnt); /* <- atomic_inc(&obj->refcnt); */
> >>
> >> /* At object put method */
> >> if (obj && refcnt__put(obj, refcnt)) /* <-atmoic_dec_and_test(&obj->refcnt)*/
> >>
> >> /* At object releasing */
> >> refcnt__exit(obj, refcnt); /* <- Newly added */
> >>
> >> The debugging feature is enabled when building perf with
> >> REFCNT_DEBUG=1. Otherwides it is just translated as normal
> >> atomic ops.
> >>
> >> Debugging binary warns you if it finds leaks when the perf exits.
> >> If you give -v (or --verbose) to the perf, it also shows backtrace
> >> logs on all refcnt operations of leaked objects.
> >>
> >> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> >
> >Looks really useful!
> >
> >Acked-by: Namhyung Kim <namhyung@kernel.org>
> >
> >Just a nitpick below..
> 
> Thanks!
> 
> >> ---
> >
> >[SNIP]
> >> +void refcnt_object__record(void *obj, const char *name, int count)
> >> +{
> >> +	struct refcnt_object *ref = refcnt__find_object(obj);
> >> +	struct refcnt_buffer *buf;
> >> +
> >> +	/* If no entry, allocate new one */
> >> +	if (!ref) {
> >> +		ref = malloc(sizeof(*ref));
> >> +		if (!ref) {
> >> +			pr_debug("REFCNT: Out of memory for %p (%s)\n",
> >> +				 obj, name);
> >> +			return;
> >> +		}
> >> +		INIT_LIST_HEAD(&ref->list);
> >> +		INIT_LIST_HEAD(&ref->head);
> >> +		ref->name = name;
> >> +		ref->obj = obj;
> >> +		list_add_tail(&ref->list, &refcnt_root);
> >> +	}
> >> +
> >> +	buf = malloc(sizeof(*buf));
> >> +	if (!buf) {
> >> +		pr_debug("REFCNT: Out of memory for %p (%s)\n", obj, ref->name);
> >> +		return;
> >> +	}
> >> +
> >> +	INIT_LIST_HEAD(&buf->list);
> >> +	buf->count = count;
> >> +	buf->nr = backtrace(buf->buf, BACKTRACE_SIZE);
> >> +	list_add_tail(&buf->list, &ref->head);
> >> +}
> >> +
> >> +static void pr_refcnt_buffer(struct refcnt_buffer *buf)
> >> +{
> >> +	char **symbuf;
> >> +	int i;
> >> +
> >> +	if (!buf)
> >> +		return;
> >> +	symbuf = backtrace_symbols(buf->buf, buf->nr);
> >
> >It seems you need to check the return value.  Maybe we can use
> >backtrace_symbols_fd() instead, or just in case of an error.
> 
> Yeah, it should be checked and in that case we can fall back to
> backtrace_symbols_fd(as the last resort), but I don’t like
> backtrace_symbols_fd replacing because it doesn't allow us to
> indent the backtrace result.

OK, I think we need to improve the backtrace code in general.  I'll
send a related patch soon.

Thanks,
Namhyung


> >
> >> +	/* Skip the first one because it is always btrace__record */
> >> +	for (i = 1; i < buf->nr; i++)
> >> +		pr_debug("  %s\n", symbuf[i]);
> >> +	free(symbuf);
> >> +}
> >> +
> >> +void refcnt__dump_unreclaimed(void) __attribute__((destructor));
> >> +void refcnt__dump_unreclaimed(void)
> >> +{
> >> +	struct refcnt_object *ref, *n;
> >> +	struct refcnt_buffer *buf;
> >> +	int i = 0;
> >> +
> >> +	if (list_empty(&refcnt_root))
> >> +		return;
> >> +
> >> +	pr_warning("REFCNT: BUG: Unreclaimed objects found.\n");
> >> +	list_for_each_entry_safe(ref, n, &refcnt_root, list) {
> >> +		pr_debug("==== [%d] ====\nUnreclaimed %s: %p\n", i,
> >> +			 ref->name ? ref->name : "(object)", ref->obj);
> >> +		list_for_each_entry(buf, &ref->head, list) {
> >> +			pr_debug("Refcount %s => %d at\n",
> >> +				 buf->count > 0 ? "+1" : "-1",
> >> +				 buf->count > 0 ? buf->count : -buf->count - 1);
> >> +			pr_refcnt_buffer(buf);
> >> +		}
> >> +		__refcnt_object__delete(ref);
> >> +		i++;
> >> +	}
> >> +	pr_warning("REFCNT: Total %d objects are not reclaimed.\n", i);
> >> +	if (!verbose)
> >> +		pr_warning("   To see all backtraces, rerun with -v option\n");
> >> +}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1272130

FromArnaldo Carvalho de Melo <acme@kernel.org>
Date2015-11-18 13:50 +0100
Message-ID<qwals-2Vg-19@gated-at.bofh.it>
In reply to#1271875
Em Wed, Nov 18, 2015 at 03:40:09PM +0900, Masami Hiramatsu escreveu:
> Hi,
> 
> Here is a series to fix some memory leaks and refcount
> leaks on map and dso. This also includes the refcnt APIs
> with backtrace debugging feature.

Cool, I wonder if this could be usable in the kernel proper... Is there
such a facility there? I'll check.

But thanks for doing this work, I'll go thru the fixes first, then look
at the debugging feature.

- Arnaldo
 
> The story has started from the posible memory leak report
> reported by Wnag Nan.
> I've tried to use valgrind to ensure the perf probe doesn't
> have other memory leaks. The result is here:
> 
>   ----
>   # valgrind ./perf probe vfs_read
>   ==17521== Memcheck, a memory error detector
>   ==17521== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
>   ==17521== Using Valgrind-3.10.0 and LibVEX; rerun with -h for copyright info
>   ==17521== Command: ./perf probe vfs_read
>   ==17521==
>   Added new event:
>     probe:vfs_read       (on vfs_read)
>   
>   You can now use it in all perf tools, such as:
>   
>           perf record -e probe:vfs_read -aR sleep 1
>   
>   ==17521==
>   ==17521== HEAP SUMMARY:
>   ==17521==     in use at exit: 3,512,761 bytes in 38,012 blocks
>   ==17521==   total heap usage: 74,723 allocs, 36,711 frees, 24,014,927
>   bytes allocated
>   ==17521==
>   ==17521== LEAK SUMMARY:
>   ==17521==    definitely lost: 6,857 bytes in 49 blocks
>   ==17521==    indirectly lost: 3,501,287 bytes in 37,891 blocks
>   ==17521==      possibly lost: 0 bytes in 0 blocks
>   ==17521==    still reachable: 4,617 bytes in 72 blocks
>   ==17521==         suppressed: 0 bytes in 0 blocks
>   ==17521== Rerun with --leak-check=full to see details of leaked memory
>   ==17521==
>   ==17521== For counts of detected and suppressed errors, rerun with: -v
>   ==17521== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 2 from 2)
>   ----
> 
> Oops! It leaked almost 4 MB memories. I've tried to find
> the root causes, and what I've found is there are many
> leaks in not only perf-probe specific code, but also maps
> and dsos (and some other pieces).
> 
> The first 3 patches are just fixing 'easy' memory leaks. However,
> most of the leaks are caused by refcnt. Since valgrind seems not
> able to debug this kind of issues, I introduced a hand-made refcnt
> backtrace APIs for debugging.
> The rest of patches are for fixing refcnt leak bugs and replcing
> refcnt apis.
> 
> After all, most of the issues are gone, except for just a few issues
> in elfutils. I'll continue to investigate that.
> 
>   ----
>   valgrind ./perf probe vfs_read
>   ==29521== Memcheck, a memory error detector
>   ==29521== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
>   ==29521== Using Valgrind-3.10.0 and LibVEX; rerun with -h for copyright info
>   ==29521== Command: ./perf probe vfs_read
>   ==29521==
>   Added new event:
>     probe:vfs_read       (on vfs_read)
>   
>   You can now use it in all perf tools, such as:
>   
>           perf record -e probe:vfs_read -aR sleep 1
>   
>   ==29521==
>   ==29521== HEAP SUMMARY:
>   ==29521==     in use at exit: 5,137 bytes in 75 blocks
>   ==29521==   total heap usage: 74,723 allocs, 74,648 frees, 24,014,927
>   bytes allocated
>   ==29521==
>   ==29521== LEAK SUMMARY:
>   ==29521==    definitely lost: 520 bytes in 3 blocks
>   ==29521==    indirectly lost: 0 bytes in 0 blocks
>   ==29521==      possibly lost: 0 bytes in 0 blocks
>   ==29521==    still reachable: 4,617 bytes in 72 blocks
>   ==29521==         suppressed: 0 bytes in 0 blocks
>   ==29521== Rerun with --leak-check=full to see details of leaked memory
>   ==29521==
>   ==29521== For counts of detected and suppressed errors, rerun with: -v
>   ==29521== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 2 from 2)
>   ----
> 
> Anyway, I decided to release these fixes and the debugging feature
> because at least this will improve perf quality.
> 
> Thank you,
> 
> ---
> 
> Masami Hiramatsu (13):
>       perf probe: Fix to free temporal Dwarf_Frame
>       perf: Make perf_exec_path always returns malloc'd string
>       perf: Introduce generic refcount APIs with debug feature
>       perf: make map to use refcnt
>       perf: Fix machine__findnew_module_map to put registered map
>       perf: Fix machine__destroy_kernel_maps to put vmlinux_maps
>       perf: Fix to destroy kernel maps when machine exits
>       perf: Fix to put new map after inserting to map_groups in dso__load_sym
>       perf: Make dso to use refcnt for debug
>       perf: Fix __dsos__addnew to put dso after adding it to the list
>       perf: Fix machine__create_kernel_maps to put kernel dso
>       perf: Fix machine__findnew_module_map to put dso
>       perf: Fix dso__load_sym to put dso
> 
> 
>  tools/perf/config/Makefile     |    5 ++
>  tools/perf/util/Build          |    1 
>  tools/perf/util/dso.c          |    9 ++-
>  tools/perf/util/exec_cmd.c     |   20 ++++--
>  tools/perf/util/exec_cmd.h     |    5 +-
>  tools/perf/util/help.c         |    6 +-
>  tools/perf/util/machine.c      |   17 ++++-
>  tools/perf/util/map.c          |    7 +-
>  tools/perf/util/map.h          |    3 +
>  tools/perf/util/probe-finder.c |    9 ++-
>  tools/perf/util/refcnt.c       |  125 ++++++++++++++++++++++++++++++++++++++++
>  tools/perf/util/refcnt.h       |   65 +++++++++++++++++++++
>  tools/perf/util/symbol-elf.c   |    4 +
>  13 files changed, 250 insertions(+), 26 deletions(-)
>  create mode 100644 tools/perf/util/refcnt.c
>  create mode 100644 tools/perf/util/refcnt.h
> 
> 
> -- 
> Masami HIRAMATSU
> Linux Technology Research Center, System Productivity Research Dept.
> Center for Technology Innovation - Systems Engineering 
> Hitachi, Ltd., Research & Development Group
> E-mail: masami.hiramatsu.pt@hitachi.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1272769

From平松雅巳 / HIRAMATU,MASAMI <masami.hiramatsu.pt@hitachi.com>
Date2015-11-19 04:00 +0100
Message-ID<qwnC2-3lg-5@gated-at.bofh.it>
In reply to#1272130
From: Arnaldo Carvalho de Melo [mailto:acme@kernel.org]
>
>Em Wed, Nov 18, 2015 at 03:40:09PM +0900, Masami Hiramatsu escreveu:
>> Hi,
>>
>> Here is a series to fix some memory leaks and refcount
>> leaks on map and dso. This also includes the refcnt APIs
>> with backtrace debugging feature.
>
>Cool, I wonder if this could be usable in the kernel proper... Is there
>such a facility there? I'll check.

As far as I know, there is no same feature, but I guess expanding
kmemleak is possible to provide similar feature.

>But thanks for doing this work, I'll go thru the fixes first, then look
>at the debugging feature.

Thanks!

>
>- Arnaldo
>
>> The story has started from the posible memory leak report
>> reported by Wnag Nan.
>> I've tried to use valgrind to ensure the perf probe doesn't
>> have other memory leaks. The result is here:
>>
>>   ----
>>   # valgrind ./perf probe vfs_read
>>   ==17521== Memcheck, a memory error detector
>>   ==17521== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
>>   ==17521== Using Valgrind-3.10.0 and LibVEX; rerun with -h for copyright info
>>   ==17521== Command: ./perf probe vfs_read
>>   ==17521==
>>   Added new event:
>>     probe:vfs_read       (on vfs_read)
>>
>>   You can now use it in all perf tools, such as:
>>
>>           perf record -e probe:vfs_read -aR sleep 1
>>
>>   ==17521==
>>   ==17521== HEAP SUMMARY:
>>   ==17521==     in use at exit: 3,512,761 bytes in 38,012 blocks
>>   ==17521==   total heap usage: 74,723 allocs, 36,711 frees, 24,014,927
>>   bytes allocated
>>   ==17521==
>>   ==17521== LEAK SUMMARY:
>>   ==17521==    definitely lost: 6,857 bytes in 49 blocks
>>   ==17521==    indirectly lost: 3,501,287 bytes in 37,891 blocks
>>   ==17521==      possibly lost: 0 bytes in 0 blocks
>>   ==17521==    still reachable: 4,617 bytes in 72 blocks
>>   ==17521==         suppressed: 0 bytes in 0 blocks
>>   ==17521== Rerun with --leak-check=full to see details of leaked memory
>>   ==17521==
>>   ==17521== For counts of detected and suppressed errors, rerun with: -v
>>   ==17521== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 2 from 2)
>>   ----
>>
>> Oops! It leaked almost 4 MB memories. I've tried to find
>> the root causes, and what I've found is there are many
>> leaks in not only perf-probe specific code, but also maps
>> and dsos (and some other pieces).
>>
>> The first 3 patches are just fixing 'easy' memory leaks. However,
>> most of the leaks are caused by refcnt. Since valgrind seems not
>> able to debug this kind of issues, I introduced a hand-made refcnt
>> backtrace APIs for debugging.
>> The rest of patches are for fixing refcnt leak bugs and replcing
>> refcnt apis.
>>
>> After all, most of the issues are gone, except for just a few issues
>> in elfutils. I'll continue to investigate that.
>>
>>   ----
>>   valgrind ./perf probe vfs_read
>>   ==29521== Memcheck, a memory error detector
>>   ==29521== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
>>   ==29521== Using Valgrind-3.10.0 and LibVEX; rerun with -h for copyright info
>>   ==29521== Command: ./perf probe vfs_read
>>   ==29521==
>>   Added new event:
>>     probe:vfs_read       (on vfs_read)
>>
>>   You can now use it in all perf tools, such as:
>>
>>           perf record -e probe:vfs_read -aR sleep 1
>>
>>   ==29521==
>>   ==29521== HEAP SUMMARY:
>>   ==29521==     in use at exit: 5,137 bytes in 75 blocks
>>   ==29521==   total heap usage: 74,723 allocs, 74,648 frees, 24,014,927
>>   bytes allocated
>>   ==29521==
>>   ==29521== LEAK SUMMARY:
>>   ==29521==    definitely lost: 520 bytes in 3 blocks
>>   ==29521==    indirectly lost: 0 bytes in 0 blocks
>>   ==29521==      possibly lost: 0 bytes in 0 blocks
>>   ==29521==    still reachable: 4,617 bytes in 72 blocks
>>   ==29521==         suppressed: 0 bytes in 0 blocks
>>   ==29521== Rerun with --leak-check=full to see details of leaked memory
>>   ==29521==
>>   ==29521== For counts of detected and suppressed errors, rerun with: -v
>>   ==29521== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 2 from 2)
>>   ----
>>
>> Anyway, I decided to release these fixes and the debugging feature
>> because at least this will improve perf quality.
>>
>> Thank you,
>>
>> ---
>>
>> Masami Hiramatsu (13):
>>       perf probe: Fix to free temporal Dwarf_Frame
>>       perf: Make perf_exec_path always returns malloc'd string
>>       perf: Introduce generic refcount APIs with debug feature
>>       perf: make map to use refcnt
>>       perf: Fix machine__findnew_module_map to put registered map
>>       perf: Fix machine__destroy_kernel_maps to put vmlinux_maps
>>       perf: Fix to destroy kernel maps when machine exits
>>       perf: Fix to put new map after inserting to map_groups in dso__load_sym
>>       perf: Make dso to use refcnt for debug
>>       perf: Fix __dsos__addnew to put dso after adding it to the list
>>       perf: Fix machine__create_kernel_maps to put kernel dso
>>       perf: Fix machine__findnew_module_map to put dso
>>       perf: Fix dso__load_sym to put dso
>>
>>
>>  tools/perf/config/Makefile     |    5 ++
>>  tools/perf/util/Build          |    1
>>  tools/perf/util/dso.c          |    9 ++-
>>  tools/perf/util/exec_cmd.c     |   20 ++++--
>>  tools/perf/util/exec_cmd.h     |    5 +-
>>  tools/perf/util/help.c         |    6 +-
>>  tools/perf/util/machine.c      |   17 ++++-
>>  tools/perf/util/map.c          |    7 +-
>>  tools/perf/util/map.h          |    3 +
>>  tools/perf/util/probe-finder.c |    9 ++-
>>  tools/perf/util/refcnt.c       |  125 ++++++++++++++++++++++++++++++++++++++++
>>  tools/perf/util/refcnt.h       |   65 +++++++++++++++++++++
>>  tools/perf/util/symbol-elf.c   |    4 +
>>  13 files changed, 250 insertions(+), 26 deletions(-)
>>  create mode 100644 tools/perf/util/refcnt.c
>>  create mode 100644 tools/perf/util/refcnt.h
>>
>>
>> --
>> Masami HIRAMATSU
>> Linux Technology Research Center, System Productivity Research Dept.
>> Center for Technology Innovation - Systems Engineering
>> Hitachi, Ltd., Research & Development Group
>> E-mail: masami.hiramatsu.pt@hitachi.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web