From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751133AbdFYNrz (ORCPT ); Sun, 25 Jun 2017 09:47:55 -0400 Received: from mail-pg0-f65.google.com ([74.125.83.65]:34879 "EHLO mail-pg0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750748AbdFYNry (ORCPT ); Sun, 25 Jun 2017 09:47:54 -0400 Date: Sun, 25 Jun 2017 22:47:13 +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 3/9] perf symbols: Discard symbols in kallsyms for loaded modules Message-ID: <20170625134713.GA12099@danjae.aot.lge.com> References: <20170623054827.3828-1-namhyung@kernel.org> <20170623054827.3828-4-namhyung@kernel.org> <20170623135105.GA4622@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20170623135105.GA4622@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 10:51:05AM -0300, Arnaldo Carvalho de Melo wrote: > Em Fri, Jun 23, 2017 at 02:48:21PM +0900, Namhyung Kim escreveu: > > If a module is already loaded, it should have symbols and no need to > > load new symbols from kallsyms. Actually kallsyms can have different > > addresses if the module was reloaded. > > Well, if it is loaded, then it should match what is in kallsyms, no? No, the perf.data was recorded with a module, and then the module was modified and reloaded at the same address. Maybe the word "loaded" can be confusing - if perf.data file has symbols in the module v1 but kallsyms has symbols in the module v2. So it needs to read symbols from the module v1 (in the build-id cache). > > > Current code just discards the first symbols only, but it should do the > > same for all symbols in the module. Note that the kernel doesn't set > > the dso->loaded bit so simply checking it would do the job IMHO. > > The kernel sets dso->loaded? Can you rephrase this part? And if this > procedure doesn't set dso->loaded how can simply checking it do the job? > > Can you clarify the above sentence? Please elaborate the scenario more > fully. Oh, I made it confusing, sorry.. I mean that it's in the middle of the dso__load() so the (kernel) dso->loaded bit was not set yet. Thanks, Namhyung > > - Arnaldo > > > Cc: Adrian Hunter > > Cc: Wang Nan > > Signed-off-by: Namhyung Kim > > --- > > tools/perf/util/symbol.c | 9 +++++---- > > 1 file changed, 5 insertions(+), 4 deletions(-) > > > > diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c > > index e7a98dbd2aed..74078ba595b3 100644 > > --- a/tools/perf/util/symbol.c > > +++ b/tools/perf/util/symbol.c > > @@ -787,11 +787,12 @@ static int dso__split_kallsyms(struct dso *dso, struct map *map, u64 delta) > > curr_map = map; > > goto discard_symbol; > > } > > - > > - if (curr_map->dso->loaded && > > - !machine__is_default_guest(machine)) > > - goto discard_symbol; > > } > > + > > + if (curr_map->dso->loaded && > > + !machine__is_default_guest(machine)) > > + goto discard_symbol; > > + > > /* > > * So that we look just like we get from .ko files, > > * i.e. not prelinked, relative to map->start. > > -- > > 2.13.1