On 08/02/2017 08:42 PM, Arnaldo Carvalho de Melo wrote:

Em Mon, Jul 24, 2017 at 04:35:13PM +0200, Thomas Richter escreveu:

quoted

+++ b/tools/perf/util/symbol-elf.c

@@ -793,6 +793,12 @@ static u64 ref_reloc(struct kmap *kmap)
 void __weak arch__sym_update(struct symbol *s __maybe_unused,
 		GElf_Sym *sym __maybe_unused) { }
 
+u64 __weak arch__sym_set_address(GElf_Sym *sym, GElf_Shdr *shdr,
+				 struct map *map __maybe_unused)
+{
+	return sym->st_value - (shdr->sh_addr - shdr->sh_offset);
+}
+
 int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *syms_ss,
 		  struct symsrc *runtime_ss, int kmodule)
 {

@@ -973,7 +979,7 @@ int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *syms_ss,
 
 			/* Adjust symbol to map to file offset */
 			if (adjust_kernel_syms)
-				sym.st_value -= shdr.sh_addr - shdr.sh_offset;
+				sym.st_value = arch__sym_set_address(&sym, &shdr, map);

Since this is just used when "adjusting" the symbol wrt file offset,
can we have a more descriptive name? Like:

void __weak arch__adjust_sym_map_offset(GElf_Sym *sym, GElf_Shdr *shdr,
					struct map *map __maybe_unused)
{
	sym->st_value -= shdr->sh_addr - shdr->sh_offset;
}

Then have it as:

-			/* Adjust symbol to map to file offset */
 			if (adjust_kernel_syms)
-				sym.st_value -= shdr.sh_addr - shdr.sh_offset;
+				arch__adjust_sym_map_offset(&sym, &shdr, map);

I.e. the weak function name becomes the comment :-)

Ok , I will provide a version 2 with your findings fixed. I'll include a 
Reviewed-by:
if this is ok with you.

-- 
Thomas Richter, Dept 3303, IBM LTC Boeblingen Germany
--
Vorsitzende des Aufsichtsrats: Martina Koederitz 
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294