Skip to content

Commit 23106e3

Browse files
captain5050acmel
authored andcommitted
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 <[email protected]> Cc: Adrian Hunter <[email protected]> Cc: Alexander Shishkin <[email protected]> Cc: Athira Rajeev <[email protected]> Cc: Changbin Du <[email protected]> Cc: Ingo Molnar <[email protected]> Cc: Jiri Olsa <[email protected]> Cc: Kan Liang <[email protected]> Cc: Mark Rutland <[email protected]> Cc: Namhyung Kim <[email protected]> Cc: Peter Zijlstra <[email protected]> Cc: Tiezhu Yang <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
1 parent ee5061f commit 23106e3

File tree

1 file changed

+25
-26
lines changed

1 file changed

+25
-26
lines changed

tools/perf/util/symbol-elf.c

Lines changed: 25 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1419,7 +1419,7 @@ void __weak arch__sym_update(struct symbol *s __maybe_unused,
14191419
static int dso__process_kernel_symbol(struct dso *dso, struct map *map,
14201420
GElf_Sym *sym, GElf_Shdr *shdr,
14211421
struct maps *kmaps, struct kmap *kmap,
1422-
struct dso **curr_dsop, struct map **curr_mapp,
1422+
struct dso **curr_dsop,
14231423
const char *section_name,
14241424
bool adjust_kernel_syms, bool kmodule, bool *remap_kernel,
14251425
u64 max_text_sh_offset)
@@ -1470,8 +1470,8 @@ static int dso__process_kernel_symbol(struct dso *dso, struct map *map,
14701470
map__set_pgoff(map, shdr->sh_offset);
14711471
}
14721472

1473-
*curr_mapp = map;
1474-
*curr_dsop = dso;
1473+
dso__put(*curr_dsop);
1474+
*curr_dsop = dso__get(dso);
14751475
return 0;
14761476
}
14771477

@@ -1484,8 +1484,8 @@ static int dso__process_kernel_symbol(struct dso *dso, struct map *map,
14841484
*/
14851485
if (kmodule && adjust_kernel_syms && is_exe_text(shdr->sh_flags) &&
14861486
shdr->sh_offset <= max_text_sh_offset) {
1487-
*curr_mapp = map;
1488-
*curr_dsop = dso;
1487+
dso__put(*curr_dsop);
1488+
*curr_dsop = dso__get(dso);
14891489
return 0;
14901490
}
14911491

@@ -1507,10 +1507,10 @@ static int dso__process_kernel_symbol(struct dso *dso, struct map *map,
15071507
dso__set_binary_type(curr_dso, dso__binary_type(dso));
15081508
dso__set_adjust_symbols(curr_dso, dso__adjust_symbols(dso));
15091509
curr_map = map__new2(start, curr_dso);
1510-
dso__put(curr_dso);
1511-
if (curr_map == NULL)
1510+
if (curr_map == NULL) {
1511+
dso__put(curr_dso);
15121512
return -1;
1513-
1513+
}
15141514
if (dso__kernel(curr_dso))
15151515
map__kmap(curr_map)->kmaps = kmaps;
15161516

@@ -1524,21 +1524,15 @@ static int dso__process_kernel_symbol(struct dso *dso, struct map *map,
15241524
dso__set_symtab_type(curr_dso, dso__symtab_type(dso));
15251525
if (maps__insert(kmaps, curr_map))
15261526
return -1;
1527-
/*
1528-
* Add it before we drop the reference to curr_map, i.e. while
1529-
* we still are sure to have a reference to this DSO via
1530-
* *curr_map->dso.
1531-
*/
15321527
dsos__add(&maps__machine(kmaps)->dsos, curr_dso);
1533-
/* kmaps already got it */
1534-
map__put(curr_map);
15351528
dso__set_loaded(curr_dso);
1536-
*curr_mapp = curr_map;
1529+
dso__put(*curr_dsop);
15371530
*curr_dsop = curr_dso;
15381531
} else {
1539-
*curr_dsop = map__dso(curr_map);
1540-
map__put(curr_map);
1532+
dso__put(*curr_dsop);
1533+
*curr_dsop = dso__get(map__dso(curr_map));
15411534
}
1535+
map__put(curr_map);
15421536

15431537
return 0;
15441538
}
@@ -1549,11 +1543,9 @@ dso__load_sym_internal(struct dso *dso, struct map *map, struct symsrc *syms_ss,
15491543
{
15501544
struct kmap *kmap = dso__kernel(dso) ? map__kmap(map) : NULL;
15511545
struct maps *kmaps = kmap ? map__kmaps(map) : NULL;
1552-
struct map *curr_map = map;
1553-
struct dso *curr_dso = dso;
1546+
struct dso *curr_dso = NULL;
15541547
Elf_Data *symstrs, *secstrs, *secstrs_run, *secstrs_sym;
15551548
uint32_t nr_syms;
1556-
int err = -1;
15571549
uint32_t idx;
15581550
GElf_Ehdr ehdr;
15591551
GElf_Shdr shdr;
@@ -1656,6 +1648,7 @@ dso__load_sym_internal(struct dso *dso, struct map *map, struct symsrc *syms_ss,
16561648
if (kmodule && adjust_kernel_syms)
16571649
max_text_sh_offset = max_text_section(runtime_ss->elf, &runtime_ss->ehdr);
16581650

1651+
curr_dso = dso__get(dso);
16591652
elf_symtab__for_each_symbol(syms, nr_syms, idx, sym) {
16601653
struct symbol *f;
16611654
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,
17441737
--sym.st_value;
17451738

17461739
if (dso__kernel(dso)) {
1747-
if (dso__process_kernel_symbol(dso, map, &sym, &shdr, kmaps, kmap, &curr_dso, &curr_map,
1748-
section_name, adjust_kernel_syms, kmodule,
1749-
&remap_kernel, max_text_sh_offset))
1740+
if (dso__process_kernel_symbol(dso, map, &sym, &shdr,
1741+
kmaps, kmap, &curr_dso,
1742+
section_name,
1743+
adjust_kernel_syms,
1744+
kmodule,
1745+
&remap_kernel,
1746+
max_text_sh_offset))
17501747
goto out_elf_end;
17511748
} else if ((used_opd && runtime_ss->adjust_symbols) ||
17521749
(!used_opd && syms_ss->adjust_symbols)) {
@@ -1795,6 +1792,7 @@ dso__load_sym_internal(struct dso *dso, struct map *map, struct symsrc *syms_ss,
17951792
__symbols__insert(dso__symbols(curr_dso), f, dso__kernel(dso));
17961793
nr++;
17971794
}
1795+
dso__put(curr_dso);
17981796

17991797
/*
18001798
* For misannotated, zeroed, ASM function sizes.
@@ -1810,9 +1808,10 @@ dso__load_sym_internal(struct dso *dso, struct map *map, struct symsrc *syms_ss,
18101808
maps__fixup_end(kmaps);
18111809
}
18121810
}
1813-
err = nr;
1811+
return nr;
18141812
out_elf_end:
1815-
return err;
1813+
dso__put(curr_dso);
1814+
return -1;
18161815
}
18171816

18181817
int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *syms_ss,

0 commit comments

Comments
 (0)