Message-ID: <CAP-5=fWge5+AunVPoy-jxk=5xJeCD8gwFAd0vWk8K8Jif3DB-Q@mail.gmail.com>
Date: Wed, 7 Aug 2024 08:42:23 -0700
From: Ian Rogers <irogers@...gle.com>
To: Arnaldo Carvalho de Melo <acme@...nel.org>
Cc: "Liang, Kan" <kan.liang@...ux.intel.com>, Peter Zijlstra <peterz@...radead.org>, 
	Ingo Molnar <mingo@...hat.com>, Namhyung Kim <namhyung@...nel.org>, 
	Mark Rutland <mark.rutland@....com>, 
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>, 
	Adrian Hunter <adrian.hunter@...el.com>, Sun Haiyong <sunhaiyong@...ngson.cn>, 
	Yanteng Si <siyanteng@...ngson.cn>, linux-perf-users@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1] perf hist: Fix reference counting of branch_info

On Wed, Aug 7, 2024 at 8:07 AM Arnaldo Carvalho de Melo <acme@...nel.org> wrote:
>
> On Wed, Aug 07, 2024 at 09:27:02AM -0400, Liang, Kan wrote:
> >
> >
> > On 2024-08-07 2:51 a.m., Ian Rogers wrote:
> > > iter_finish_branch_entry doesn't put the branch_info from/to map
> > > elements creating memory leaks. This can be seen with:
> > >
> > > ```
> > > $ perf record -e cycles -b perf test -w noploop
> > > $ perf report -D
> > > ...
> > > Direct leak of 984344 byte(s) in 123043 object(s) allocated from:
> > >     #0 0x7fb2654f3bd7 in malloc libsanitizer/asan/asan_malloc_linux.cpp:69
> > >     #1 0x564d3400d10b in map__get util/map.h:186
> > >     #2 0x564d3400d10b in ip__resolve_ams util/machine.c:1981
> > >     #3 0x564d34014d81 in sample__resolve_bstack util/machine.c:2151
> > >     #4 0x564d34094790 in iter_prepare_branch_entry util/hist.c:898
> > >     #5 0x564d34098fa4 in hist_entry_iter__add util/hist.c:1238
> > >     #6 0x564d33d1f0c7 in process_sample_event tools/perf/builtin-report.c:334
> > >     #7 0x564d34031eb7 in perf_session__deliver_event util/session.c:1655
> > >     #8 0x564d3403ba52 in do_flush util/ordered-events.c:245
> > >     #9 0x564d3403ba52 in __ordered_events__flush util/ordered-events.c:324
> > >     #10 0x564d3402d32e in perf_session__process_user_event util/session.c:1708
> > >     #11 0x564d34032480 in perf_session__process_event util/session.c:1877
> > >     #12 0x564d340336ad in reader__read_event util/session.c:2399
> > >     #13 0x564d34033fdc in reader__process_events util/session.c:2448
> > >     #14 0x564d34033fdc in __perf_session__process_events util/session.c:2495
> > >     #15 0x564d34033fdc in perf_session__process_events util/session.c:2661
> > >     #16 0x564d33d27113 in __cmd_report tools/perf/builtin-report.c:1065
> > >     #17 0x564d33d27113 in cmd_report tools/perf/builtin-report.c:1805
> > >     #18 0x564d33e0ccb7 in run_builtin tools/perf/perf.c:350
> > >     #19 0x564d33e0d45e in handle_internal_command tools/perf/perf.c:403
> > >     #20 0x564d33cdd827 in run_argv tools/perf/perf.c:447
> > >     #21 0x564d33cdd827 in main tools/perf/perf.c:561
> > > ...
> > > ```
> > >
> > > Clearing up the map_symbols properly creates maps reference count
> > > issues so resolve those. Resolving this issue doesn't improve peak
> > > heap consumption for the test above.
> > >
> > > Signed-off-by: Ian Rogers <irogers@...gle.com>
> >
> >
> > Reviewed-by: Kan Liang <kan.liang@...ux.intel.com>
>
> Thanks, applying.
>
> While trying to test it:
>
> make -k CORESIGHT=1 EXTRA_CFLAGS="-fsanitize=memory" CC=clang HOSTCC=clang NO_LIBTRACEEVENT=1 NO_LIBELF=1 BUILD_BPF_SKEL=0 NO_LIBPFM=1 O=/tmp/build/$(basename $PWD)/ -C tools/perf install-bin
>
> Used that from a previous patch description from Ian to get rid of some
> other problems with those other libraries:
>
> ⬢[acme@...lbox perf-tools-next]$  perf record -e cycles -b perf test -w noploop
> Uninitialized bytes in fopen64 at offset 46 inside [0x7fff1077e890, 52)
> ==1948231==WARNING: MemorySanitizer: use-of-uninitialized-value
>     #0 0x7921df in perf_pmu_format__load pmu.c
>     #1 0x791f3e in perf_pmu__warn_invalid_formats (/home/acme/bin/perf+0x791f3e) (BuildId: d7742e31f05abb200493b431a6191afda9ed77c8)
>     #2 0x6f62d0 in __add_event parse-events.c
>     #3 0x6fa681 in __parse_events_add_numeric parse-events.c
>     #4 0x6fa3e4 in parse_events_add_numeric (/home/acme/bin/perf+0x6fa3e4) (BuildId: d7742e31f05abb200493b431a6191afda9ed77c8)
>     #5 0x78c6ca in parse_events_parse (/home/acme/bin/perf+0x78c6ca) (BuildId: d7742e31f05abb200493b431a6191afda9ed77c8)
>     #6 0x6fd8eb in __parse_events (/home/acme/bin/perf+0x6fd8eb) (BuildId: d7742e31f05abb200493b431a6191afda9ed77c8)
>     #7 0x6ff232 in parse_events_option (/home/acme/bin/perf+0x6ff232) (BuildId: d7742e31f05abb200493b431a6191afda9ed77c8)
>     #8 0x5be82f in get_value /home/acme/git/perf-tools-next/tools/lib/subcmd/parse-options.c
>     #9 0x5ba474 in parse_short_opt /home/acme/git/perf-tools-next/tools/lib/subcmd/parse-options.c:351:11
>     #10 0x5ba474 in parse_options_step /home/acme/git/perf-tools-next/tools/lib/subcmd/parse-options.c:539:12
>     #11 0x5ba474 in parse_options_subcommand /home/acme/git/perf-tools-next/tools/lib/subcmd/parse-options.c:653:10
>     #12 0x4f089f in cmd_record (/home/acme/bin/perf+0x4f089f) (BuildId: d7742e31f05abb200493b431a6191afda9ed77c8)
>     #13 0x56fda9 in run_builtin perf.c
>     #14 0x56e9ea in main (/home/acme/bin/perf+0x56e9ea) (BuildId: d7742e31f05abb200493b431a6191afda9ed77c8)
>     #15 0x7fbf387ea087 in __libc_start_call_main (/lib64/libc.so.6+0x2a087) (BuildId: 8f53abaad945a669f2bdcd25f471d80e077568ef)
>     #16 0x7fbf387ea14a in __libc_start_main@...BC_2.2.5 (/lib64/libc.so.6+0x2a14a) (BuildId: 8f53abaad945a669f2bdcd25f471d80e077568ef)
>     #17 0x4364e4 in _start (/home/acme/bin/perf+0x4364e4) (BuildId: d7742e31f05abb200493b431a6191afda9ed77c8)
>
> SUMMARY: MemorySanitizer: use-of-uninitialized-value pmu.c in perf_pmu_format__load
> Exiting
> ⬢[acme@...lbox perf-tools-next]$
>
> So I think there is something else nor merged or is this something new?

Taking a look. You are setting HOSTCC here presumably to work past a
libbpf build error I see. Why are we building libbpf with HOSTCC and
not CC?

Thanks,
Ian

Powered by blists - more mailing lists