Re: [PATCH/RFC 5/9] perf symbols: Fixup the end address of kernel map properly

[Namhyung Kim]

On Fri, Jun 23, 2017 at 11:27:25AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Jun 23, 2017 at 02:48:23PM +0900, Namhyung Kim escreveu:
> > When /proc/kallsyms is used for kernel address, addresses in module can
> > be changed when the module is reloaded.  So if one did perf record with
> > some module and then for some reason reload the module.  Then perf
> > report might see a different address for the module and the output can
> > show incorrect symbols.
> 
> If we, as mentioned in another message, make sure that no module reload
> is performed while perf record is running, and then refuse to use
> kallsyms if the buildid for the running module is different from the one
> in the perf.data build id header, then this is solved, no?

No.  This can happen when a (same) module is reload at a different
address.  Even if we refuse the symbols of the module in
dso__split_kallsyms(), the last symbols in the kernel dso was already
fixed up with a different address and then the map end address would
be invalid.  This can make the module map overlapped to the kernel
map, so invisible when it finds maps for samples.

Thanks,
Namhyung


>  
> > For example, let a module XXX was loaded at 0xffffffff8a000000, the
> > /proc/kallsyms might show following:
> > 
> >   ...
> >   0xffffffff81234560 t last_symbol_in_vmlinux
> >   0xffffffff8a000000 t first_symbol_in_XXX
> >   ...
> > 
> > As kallsyms fixs up the symbol and map address by using next address,
> > the end address of last_symbol and kernel map would be start address of
> > XXX (0xffffffff8a000000).  And samples in 0xffffffff8a001234 can be
> > found in a map for XXX module.
> > 
> > But later, XXX was reloaded at 0xffffffff8a007000, slightly higher than
> > before.  Now reading /proc/kallsyms tells that the end address of last
> > symbol would be 0xfffffff8a007000 and so kernel map is same.  Now
> > samples in 0xffffffff8a001234 - formerly in the XXX module - would go to
> > the kernel map and show no symbols.
> > 
> > In this case, perf can know the address of map of XXX from mmap event in
> > perf.data so it can adjust kernel map address using the address of XXX
> > map.  To do that, replace map__fixup_end() by __map_groups__fixup_end().
> > This still have incorrect end address of last symbol in the kernel map
> > but it's ok since samples in that address wouldn't go to the kernel map
> > anyway.
> > 
> > Cc: Adrian Hunter <adrian.hunter@xxxxxxxxx>
> > Cc: Wang Nan <wangnan0@xxxxxxxxxx>
> > Signed-off-by: Namhyung Kim <namhyung@xxxxxxxxxx>
> > ---
> >  tools/perf/util/symbol.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> > index 74078ba595b3..ce79a51f25bf 100644
> > --- a/tools/perf/util/symbol.c
> > +++ b/tools/perf/util/symbol.c
> > @@ -1832,7 +1832,7 @@ static int dso__load_kernel_sym(struct dso *dso, struct map *map)
> >  		dso->binary_type = DSO_BINARY_TYPE__KALLSYMS;
> >  		dso__set_long_name(dso, DSO__NAME_KALLSYMS, false);
> >  		map__fixup_start(map);
> > -		map__fixup_end(map);
> > +		__map_groups__fixup_end(map->groups, map->type);
> >  	}
> >  
> >  	return err;
> > @@ -1880,7 +1880,7 @@ static int dso__load_guest_kernel_sym(struct dso *dso, struct map *map)
> >  		machine__mmap_name(machine, path, sizeof(path));
> >  		dso__set_long_name(dso, strdup(path), true);
> >  		map__fixup_start(map);
> > -		map__fixup_end(map);
> > +		__map_groups__fixup_end(map->groups, map->type);
> >  	}
> >  
> >  	return err;
> > -- 
> > 2.13.1