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


Groups > linux.kernel > #1673268 > unrolled thread

[PATCH/RFC 3/9] perf symbols: Discard symbols in kallsyms for loaded modules

Started byNamhyung Kim <namhyung@kernel.org>
First post2017-06-23 07:50 +0200
Last post2017-06-25 15:50 +0200
Articles 3 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  [PATCH/RFC 3/9] perf symbols: Discard symbols in kallsyms for loaded modules Namhyung Kim <namhyung@kernel.org> - 2017-06-23 07:50 +0200
    Re: [PATCH/RFC 3/9] perf symbols: Discard symbols in kallsyms for  loaded modules Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-06-23 16:00 +0200
      Re: [PATCH/RFC 3/9] perf symbols: Discard symbols in kallsyms for  loaded modules Namhyung Kim <namhyung@kernel.org> - 2017-06-25 15:50 +0200

#1673268 — [PATCH/RFC 3/9] perf symbols: Discard symbols in kallsyms for loaded modules

FromNamhyung Kim <namhyung@kernel.org>
Date2017-06-23 07:50 +0200
Subject[PATCH/RFC 3/9] perf symbols: Discard symbols in kallsyms for loaded modules
Message-ID<tVpDH-6AF-7@gated-at.bofh.it>
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.

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.

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Wang Nan <wangnan0@huawei.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 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

[toc] | [next] | [standalone]


#1673572 — Re: [PATCH/RFC 3/9] perf symbols: Discard symbols in kallsyms for loaded modules

FromArnaldo Carvalho de Melo <acme@kernel.org>
Date2017-06-23 16:00 +0200
SubjectRe: [PATCH/RFC 3/9] perf symbols: Discard symbols in kallsyms for loaded modules
Message-ID<tVxhU-2U3-19@gated-at.bofh.it>
In reply to#1673268
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?
 
> 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.

- Arnaldo
 
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Cc: Wang Nan <wangnan0@huawei.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  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

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


#1674220 — Re: [PATCH/RFC 3/9] perf symbols: Discard symbols in kallsyms for loaded modules

FromNamhyung Kim <namhyung@kernel.org>
Date2017-06-25 15:50 +0200
SubjectRe: [PATCH/RFC 3/9] perf symbols: Discard symbols in kallsyms for loaded modules
Message-ID<tWg5j-5Jp-3@gated-at.bofh.it>
In reply to#1673572
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 <adrian.hunter@intel.com>
> > Cc: Wang Nan <wangnan0@huawei.com>
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> >  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

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web