1

perf symbol-elf: dso__load_sym_internal() reference count fixes

dso__load_sym_internal() passed curr_mapp as an out argument to
dso__process_kernel_symbol(). The out argument was never used so remove
it to simplify the reference counting logic.

Simplify reference counting issues with curr_dso by ensuring the value
it points to has a +1 reference count, and then putting as
necessary.

This avoids some reference counting games when the dso is created making
the code more obviously correct with some possible introduced overhead
due to the reference counting get/puts.

This, however, silences reference count checking and we can always
optimize from a seemingly correct point.

Signed-off-by: Ian Rogers <irogers@google.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Cc: Changbin Du <changbin.du@huawei.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Tiezhu Yang <yangtiezhu@loongson.cn>
Link: https://lore.kernel.org/r/20240506180104.485674-4-irogers@google.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
This commit is contained in:
Ian Rogers 2024-05-06 11:01:03 -07:00 committed by Arnaldo Carvalho de Melo
parent ee5061f824
commit 23106e3188

View File

@ -1419,7 +1419,7 @@ void __weak arch__sym_update(struct symbol *s __maybe_unused,
static int dso__process_kernel_symbol(struct dso *dso, struct map *map, static int dso__process_kernel_symbol(struct dso *dso, struct map *map,
GElf_Sym *sym, GElf_Shdr *shdr, GElf_Sym *sym, GElf_Shdr *shdr,
struct maps *kmaps, struct kmap *kmap, struct maps *kmaps, struct kmap *kmap,
struct dso **curr_dsop, struct map **curr_mapp, struct dso **curr_dsop,
const char *section_name, const char *section_name,
bool adjust_kernel_syms, bool kmodule, bool *remap_kernel, bool adjust_kernel_syms, bool kmodule, bool *remap_kernel,
u64 max_text_sh_offset) u64 max_text_sh_offset)
@ -1470,8 +1470,8 @@ static int dso__process_kernel_symbol(struct dso *dso, struct map *map,
map__set_pgoff(map, shdr->sh_offset); map__set_pgoff(map, shdr->sh_offset);
} }
*curr_mapp = map; dso__put(*curr_dsop);
*curr_dsop = dso; *curr_dsop = dso__get(dso);
return 0; return 0;
} }
@ -1484,8 +1484,8 @@ static int dso__process_kernel_symbol(struct dso *dso, struct map *map,
*/ */
if (kmodule && adjust_kernel_syms && is_exe_text(shdr->sh_flags) && if (kmodule && adjust_kernel_syms && is_exe_text(shdr->sh_flags) &&
shdr->sh_offset <= max_text_sh_offset) { shdr->sh_offset <= max_text_sh_offset) {
*curr_mapp = map; dso__put(*curr_dsop);
*curr_dsop = dso; *curr_dsop = dso__get(dso);
return 0; return 0;
} }
@ -1507,10 +1507,10 @@ static int dso__process_kernel_symbol(struct dso *dso, struct map *map,
dso__set_binary_type(curr_dso, dso__binary_type(dso)); dso__set_binary_type(curr_dso, dso__binary_type(dso));
dso__set_adjust_symbols(curr_dso, dso__adjust_symbols(dso)); dso__set_adjust_symbols(curr_dso, dso__adjust_symbols(dso));
curr_map = map__new2(start, curr_dso); curr_map = map__new2(start, curr_dso);
if (curr_map == NULL) {
dso__put(curr_dso); dso__put(curr_dso);
if (curr_map == NULL)
return -1; return -1;
}
if (dso__kernel(curr_dso)) if (dso__kernel(curr_dso))
map__kmap(curr_map)->kmaps = kmaps; map__kmap(curr_map)->kmaps = kmaps;
@ -1524,21 +1524,15 @@ static int dso__process_kernel_symbol(struct dso *dso, struct map *map,
dso__set_symtab_type(curr_dso, dso__symtab_type(dso)); dso__set_symtab_type(curr_dso, dso__symtab_type(dso));
if (maps__insert(kmaps, curr_map)) if (maps__insert(kmaps, curr_map))
return -1; return -1;
/*
* Add it before we drop the reference to curr_map, i.e. while
* we still are sure to have a reference to this DSO via
* *curr_map->dso.
*/
dsos__add(&maps__machine(kmaps)->dsos, curr_dso); dsos__add(&maps__machine(kmaps)->dsos, curr_dso);
/* kmaps already got it */
map__put(curr_map);
dso__set_loaded(curr_dso); dso__set_loaded(curr_dso);
*curr_mapp = curr_map; dso__put(*curr_dsop);
*curr_dsop = curr_dso; *curr_dsop = curr_dso;
} else { } else {
*curr_dsop = map__dso(curr_map); dso__put(*curr_dsop);
map__put(curr_map); *curr_dsop = dso__get(map__dso(curr_map));
} }
map__put(curr_map);
return 0; return 0;
} }
@ -1549,11 +1543,9 @@ dso__load_sym_internal(struct dso *dso, struct map *map, struct symsrc *syms_ss,
{ {
struct kmap *kmap = dso__kernel(dso) ? map__kmap(map) : NULL; struct kmap *kmap = dso__kernel(dso) ? map__kmap(map) : NULL;
struct maps *kmaps = kmap ? map__kmaps(map) : NULL; struct maps *kmaps = kmap ? map__kmaps(map) : NULL;
struct map *curr_map = map; struct dso *curr_dso = NULL;
struct dso *curr_dso = dso;
Elf_Data *symstrs, *secstrs, *secstrs_run, *secstrs_sym; Elf_Data *symstrs, *secstrs, *secstrs_run, *secstrs_sym;
uint32_t nr_syms; uint32_t nr_syms;
int err = -1;
uint32_t idx; uint32_t idx;
GElf_Ehdr ehdr; GElf_Ehdr ehdr;
GElf_Shdr shdr; GElf_Shdr shdr;
@ -1656,6 +1648,7 @@ dso__load_sym_internal(struct dso *dso, struct map *map, struct symsrc *syms_ss,
if (kmodule && adjust_kernel_syms) if (kmodule && adjust_kernel_syms)
max_text_sh_offset = max_text_section(runtime_ss->elf, &runtime_ss->ehdr); max_text_sh_offset = max_text_section(runtime_ss->elf, &runtime_ss->ehdr);
curr_dso = dso__get(dso);
elf_symtab__for_each_symbol(syms, nr_syms, idx, sym) { elf_symtab__for_each_symbol(syms, nr_syms, idx, sym) {
struct symbol *f; struct symbol *f;
const char *elf_name = elf_sym__name(&sym, symstrs); const char *elf_name = elf_sym__name(&sym, symstrs);
@ -1744,9 +1737,13 @@ dso__load_sym_internal(struct dso *dso, struct map *map, struct symsrc *syms_ss,
--sym.st_value; --sym.st_value;
if (dso__kernel(dso)) { if (dso__kernel(dso)) {
if (dso__process_kernel_symbol(dso, map, &sym, &shdr, kmaps, kmap, &curr_dso, &curr_map, if (dso__process_kernel_symbol(dso, map, &sym, &shdr,
section_name, adjust_kernel_syms, kmodule, kmaps, kmap, &curr_dso,
&remap_kernel, max_text_sh_offset)) section_name,
adjust_kernel_syms,
kmodule,
&remap_kernel,
max_text_sh_offset))
goto out_elf_end; goto out_elf_end;
} else if ((used_opd && runtime_ss->adjust_symbols) || } else if ((used_opd && runtime_ss->adjust_symbols) ||
(!used_opd && syms_ss->adjust_symbols)) { (!used_opd && syms_ss->adjust_symbols)) {
@ -1795,6 +1792,7 @@ dso__load_sym_internal(struct dso *dso, struct map *map, struct symsrc *syms_ss,
__symbols__insert(dso__symbols(curr_dso), f, dso__kernel(dso)); __symbols__insert(dso__symbols(curr_dso), f, dso__kernel(dso));
nr++; nr++;
} }
dso__put(curr_dso);
/* /*
* For misannotated, zeroed, ASM function sizes. * For misannotated, zeroed, ASM function sizes.
@ -1810,9 +1808,10 @@ dso__load_sym_internal(struct dso *dso, struct map *map, struct symsrc *syms_ss,
maps__fixup_end(kmaps); maps__fixup_end(kmaps);
} }
} }
err = nr; return nr;
out_elf_end: out_elf_end:
return err; dso__put(curr_dso);
return -1;
} }
int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *syms_ss, int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *syms_ss,