From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751142AbdFYOSe (ORCPT ); Sun, 25 Jun 2017 10:18:34 -0400 Received: from mail-pg0-f66.google.com ([74.125.83.66]:35044 "EHLO mail-pg0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750763AbdFYOSc (ORCPT ); Sun, 25 Jun 2017 10:18:32 -0400 Date: Sun, 25 Jun 2017 23:17:51 +0900 From: Namhyung Kim To: Arnaldo Carvalho de Melo Cc: Ingo Molnar , Peter Zijlstra , Jiri Olsa , LKML , kernel-team@lge.com, Masami Hiramatsu , Andi Kleen , Adrian Hunter , Wang Nan Subject: Re: [PATCH/RFC 4/9] perf symbols: Load kernel module symbols ASAP Message-ID: <20170625141751.GC12099@danjae.aot.lge.com> References: <20170623054827.3828-1-namhyung@kernel.org> <20170623054827.3828-5-namhyung@kernel.org> <20170623142604.GC4622@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20170623142604.GC4622@kernel.org> User-Agent: Mutt/1.8.2 (2017-04-18) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Jun 23, 2017 at 11:26:04AM -0300, Arnaldo Carvalho de Melo wrote: > Em Fri, Jun 23, 2017 at 02:48:22PM +0900, Namhyung Kim escreveu: > > When loading kernel symbols from /proc/kallsyms, it might have different > > addresses for modules. We should honor the mmap event recorded in a > > perf.data so load the module symbols when it sees the event so that it > > cannot be overridden by symbols in /proc/kallsyms later. > > > Cc: Adrian Hunter > > Cc: Wang Nan > > Signed-off-by: Namhyung Kim > > --- > > tools/perf/util/machine.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c > > index d838f893e639..799efe920f0c 100644 > > --- a/tools/perf/util/machine.c > > +++ b/tools/perf/util/machine.c > > @@ -656,6 +656,9 @@ struct map *machine__findnew_module_map(struct machine *machine, u64 start, > > > > map_groups__insert(&machine->kmaps, map); > > > > + if (dso->has_build_id) > > + map__load(map); > > How could this be set at this point? I just tried processing a > PERF_RECORD_MMAP event for a kernel module and at this point it is set > to false :-\ > > Humm, but I did it for video.ko, that has no hits in this case, so this > must be coming from reading the perf.data build-id table... /me tries > that... yeah, when processing the buildid table in perf.data we > populate the machine->dsos list and set the dso->has_build_id flag: > > [root@jouet ~]# perf buildid-list | grep .ko > 57a47c2f0d85ef5726b80e8f55403c3f508a4eac /lib/modules/4.12.0-rc4+/kernel/drivers/net/wireless/intel/iwlwifi/iwlwifi.ko > [root@jouet ~]# > (gdb) bt > #0 machine__findnew_module_map (machine=0x21c6c78, start=18446744072647282688, filename=0x7fd8fc057a00 "/lib/modules/4.12.0-rc4+/kernel/drivers/net/wireless/intel/iwlwifi/iwlwifi.ko") > at util/machine.c:650 > #1 0x00000000005094a3 in machine__process_kernel_mmap_event (machine=0x21c6c78, event=0x7fd8fc0579d8) at util/machine.c:1274 > #2 0x00000000005099de in machine__process_mmap_event (machine=0x21c6c78, event=0x7fd8fc0579d8, sample=0x7fffffffbef0) at util/machine.c:1433 > #3 0x00000000004cc021 in perf_event__process_mmap (tool=0x7fffffffc440, event=0x7fd8fc0579d8, sample=0x7fffffffbef0, machine=0x21c6c78) at util/event.c:1265 > #4 0x0000000000512686 in machines__deliver_event (machines=0x21c6c78, evlist=0x21c6f30, event=0x7fd8fc0579d8, sample=0x7fffffffbef0, tool=0x7fffffffc440, file_offset=10712) > at util/session.c:1250 > #5 0x00000000005129ce in perf_session__deliver_event (session=0x21c6b90, event=0x7fd8fc0579d8, sample=0x7fffffffbef0, tool=0x7fffffffc440, file_offset=10712) at util/session.c:1310 > #6 0x0000000000513255 in perf_session__process_event (session=0x21c6b90, event=0x7fd8fc0579d8, file_offset=10712) at util/session.c:1490 > #7 0x0000000000513ff4 in __perf_session__process_events (session=0x21c6b90, data_offset=232, data_size=23120, file_size=23352) at util/session.c:1867 > #8 0x00000000005141f2 in perf_session__process_events (session=0x21c6b90) at util/session.c:1921 > #9 0x00000000004427c9 in __cmd_report (rep=0x7fffffffc440) at builtin-report.c:575 > #10 0x000000000044427c in cmd_report (argc=0, argv=0x7fffffffe160) at builtin-report.c:1054 > #11 0x00000000004bad91 in run_builtin (p=0xa28fd0 , argc=2, argv=0x7fffffffe160) at perf.c:296 > #12 0x00000000004baffe in handle_internal_command (argc=2, argv=0x7fffffffe160) at perf.c:348 > #13 0x00000000004bb150 in run_argv (argcp=0x7fffffffdfbc, argv=0x7fffffffdfb0) at perf.c:392 > #14 0x00000000004bb52a in main (argc=2, argv=0x7fffffffe160) at perf.c:530 > (gdb) p dso > $127 = (struct dso *) 0x21c7dc0 > (gdb) p dso->has_build_id > $128 = 1 '\001' > (gdb) > > So what you're doing is to load it here... perhaps we should not do > that, and instead in the kallsyms loading code, when processing the > symbols for this specific module, discard them and leave its symbol > table unpopulated, then, later, when resolving a sample in this module, > it will see that it is not loaded, will see that it has a build-id and > will use that, and _refuse_ to get it from anywhere else than an ELF > file with that build-id. > > I.e. in general, when we have a build-id for a given DSO, we should not > read it from kallsyms or any other file that don't have a matching > build-id. > > The only possibility would be: hey, I have a build id, and reading the > currently loaded module (/sys/module/e1000e/notes/.note.gnu.build-id) I > see it _still_ has that same build-id, ok, I can read it from kallsyms. > > What do you think? I'm ok with that, will change to compare build-id before loading symbols. Thanks, Namhyung