diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c index da1ad58758af..1ea06fde14e0 100644 --- a/tools/perf/util/machine.c +++ b/tools/perf/util/machine.c @@ -1539,22 +1539,30 @@ static void machine__set_kernel_mmap(struct machine *machine, map__set_end(machine->vmlinux_map, ~0ULL); } -static int machine__update_kernel_mmap(struct machine *machine, - u64 start, u64 end) +struct kernel_mmap_mutation_ctx { + u64 start; + u64 end; +}; + +static int kernel_mmap_mutate_cb(struct map *map, void *data) { - struct map *orig, *updated; - int err; + struct kernel_mmap_mutation_ctx *ctx = data; - orig = machine->vmlinux_map; - updated = map__get(orig); + map__set_start(map, ctx->start); + map__set_end(map, ctx->end); + if (ctx->start == 0 && ctx->end == 0) + map__set_end(map, ~0ULL); + return 0; +} - machine->vmlinux_map = updated; - maps__remove(machine__kernel_maps(machine), orig); - machine__set_kernel_mmap(machine, start, end); - err = maps__insert(machine__kernel_maps(machine), updated); - map__put(orig); +static int machine__update_kernel_mmap(struct machine *machine, + u64 start, u64 end) +{ + struct kernel_mmap_mutation_ctx ctx = { .start = start, .end = end }; - return err; + return maps__mutate_mapping(machine__kernel_maps(machine), + machine->vmlinux_map, + kernel_mmap_mutate_cb, &ctx); } int machine__create_kernel_maps(struct machine *machine) diff --git a/tools/perf/util/maps.c b/tools/perf/util/maps.c index 923935ee21b6..f808df2fe77b 100644 --- a/tools/perf/util/maps.c +++ b/tools/perf/util/maps.c @@ -576,6 +576,48 @@ void maps__remove(struct maps *maps, struct map *map) #endif } +/** + * maps__mutate_mapping - Apply write-protected mutations to a map. + * @maps: The maps collection containing the map. + * @map: The map to mutate. + * @mutate_cb: Callback function that performs the actual mutations. + * @data: Private data passed to the callback. + * + * This acquires the write lock on the maps semaphore to safely protect + * concurrent readers from seeing partially mutated or unsorted map boundaries. + * + * WARNING: Acquiring down_write() here can trigger a recursive self-deadlock if + * the caller already holds the read lock (e.g., during maps__for_each_map() or + * maps__find() iteration paths that trigger lazy symbol loading). To completely + * avoid this deadlock, all kernel/module maps must be pre-loaded up-front (via + * maps__load_maps()) under a clean, single-threaded context before entering + * multi-threaded event processing loops. + */ +int maps__mutate_mapping(struct maps *maps, struct map *map, + int (*mutate_cb)(struct map *map, void *data), void *data) +{ + int err = 0; + + if (maps) { + down_write(maps__lock(maps)); + + err = mutate_cb(map, data); + + RC_CHK_ACCESS(maps)->maps_by_address_sorted = false; + RC_CHK_ACCESS(maps)->maps_by_name_sorted = false; + + up_write(maps__lock(maps)); + +#ifdef HAVE_LIBDW_SUPPORT + libdw__invalidate_dwfl(maps, maps__libdw_addr_space_dwfl(maps)); +#endif + } else { + err = mutate_cb(map, data); + } + + return err; +} + bool maps__empty(struct maps *maps) { bool res; @@ -626,6 +668,41 @@ int maps__for_each_map(struct maps *maps, int (*cb)(struct map *map, void *data) return ret; } +int maps__load_maps(struct maps *maps) +{ + struct map **maps_copy; + unsigned int nr_maps; + int err = 0; + + if (!maps) + return 0; + + down_read(maps__lock(maps)); + nr_maps = maps__nr_maps(maps); + if (nr_maps == 0) { + up_read(maps__lock(maps)); + return 0; + } + maps_copy = calloc(nr_maps, sizeof(*maps_copy)); + if (!maps_copy) { + up_read(maps__lock(maps)); + return -ENOMEM; + } + for (unsigned int i = 0; i < nr_maps; i++) + maps_copy[i] = map__get(maps__maps_by_address(maps)[i]); + up_read(maps__lock(maps)); + + for (unsigned int i = 0; i < nr_maps; i++) { + if (map__load(maps_copy[i]) < 0) { + pr_warning("Failed to load map %s\n", dso__name(map__dso(maps_copy[i]))); + err = -1; + } + map__put(maps_copy[i]); + } + free(maps_copy); + return err; +} + void maps__remove_maps(struct maps *maps, bool (*cb)(struct map *map, void *data), void *data) { struct map **maps_by_address; @@ -668,40 +745,57 @@ struct symbol *maps__find_symbol(struct maps *maps, u64 addr, struct map **mapp) return result; } -struct maps__find_symbol_by_name_args { - struct map **mapp; - const char *name; - struct symbol *sym; -}; - -static int maps__find_symbol_by_name_cb(struct map *map, void *data) -{ - struct maps__find_symbol_by_name_args *args = data; - - args->sym = map__find_symbol_by_name(map, args->name); - if (!args->sym) - return 0; - - if (!map__contains_symbol(map, args->sym)) { - args->sym = NULL; - return 0; - } - - if (args->mapp != NULL) - *args->mapp = map__get(map); - return 1; -} - struct symbol *maps__find_symbol_by_name(struct maps *maps, const char *name, struct map **mapp) { - struct maps__find_symbol_by_name_args args = { - .mapp = mapp, - .name = name, - .sym = NULL, - }; + struct map **maps_copy; + unsigned int nr_maps; + struct symbol *sym = NULL; - maps__for_each_map(maps, maps__find_symbol_by_name_cb, &args); - return args.sym; + if (!maps) + return NULL; + + /* + * First, ensure all maps are loaded. We pre-load them outside of any + * read-to-write locks to avoid deadlocks. Even if some fail, we proceed. + */ + maps__load_maps(maps); + + /* + * Create a local snapshot of the maps while holding the read lock. + * This prevents deadlocking if iteration triggers further map insertions. + */ + down_read(maps__lock(maps)); + nr_maps = maps__nr_maps(maps); + maps_copy = calloc(nr_maps, sizeof(*maps_copy)); + if (maps_copy) { + for (unsigned int i = 0; i < nr_maps; i++) { + struct map *map = maps__maps_by_address(maps)[i]; + + maps_copy[i] = map__get(map); + } + } + up_read(maps__lock(maps)); + + if (!maps_copy) + return NULL; + + for (unsigned int i = 0; i < nr_maps; i++) { + struct map *map = maps_copy[i]; + + sym = map__find_symbol_by_name(map, name); + if (sym && map__contains_symbol(map, sym)) { + if (mapp) + *mapp = map__get(map); + break; + } + sym = NULL; + } + + for (unsigned int i = 0; i < nr_maps; i++) + map__put(maps_copy[i]); + + free(maps_copy); + return sym; } int maps__find_ams(struct maps *maps, struct addr_map_symbol *ams) diff --git a/tools/perf/util/maps.h b/tools/perf/util/maps.h index 5b80b199685e..4ec9b7453a3b 100644 --- a/tools/perf/util/maps.h +++ b/tools/perf/util/maps.h @@ -59,8 +59,11 @@ void maps__set_libdw_addr_space_dwfl(struct maps *maps, void *dwfl); size_t maps__fprintf(struct maps *maps, FILE *fp); +int maps__load_maps(struct maps *maps); int maps__insert(struct maps *maps, struct map *map); void maps__remove(struct maps *maps, struct map *map); +int maps__mutate_mapping(struct maps *maps, struct map *map, + int (*mutate_cb)(struct map *map, void *data), void *data); struct map *maps__find(struct maps *maps, u64 addr); struct symbol *maps__find_symbol(struct maps *maps, u64 addr, struct map **mapp); diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c index d84e2e031d43..c301c298ded9 100644 --- a/tools/perf/util/symbol-elf.c +++ b/tools/perf/util/symbol-elf.c @@ -1373,6 +1373,24 @@ static u64 ref_reloc(struct kmap *kmap) void __weak arch__sym_update(struct symbol *s __maybe_unused, GElf_Sym *sym __maybe_unused) { } +struct remap_kernel_ctx { + u64 sh_addr; + u64 sh_size; + u64 sh_offset; + struct kmap *kmap; +}; + +static int remap_kernel_cb(struct map *map, void *data) +{ + struct remap_kernel_ctx *ctx = data; + + map__set_start(map, ctx->sh_addr + ref_reloc(ctx->kmap)); + map__set_end(map, map__start(map) + ctx->sh_size); + map__set_pgoff(map, ctx->sh_offset); + map__set_mapping_type(map, MAPPING_TYPE__DSO); + return 0; +} + static int dso__process_kernel_symbol(struct dso *dso, struct map *map, GElf_Sym *sym, GElf_Shdr *shdr, struct maps *kmaps, struct kmap *kmap, @@ -1403,22 +1421,15 @@ static int dso__process_kernel_symbol(struct dso *dso, struct map *map, * map to the kernel dso. */ if (*remap_kernel && dso__kernel(dso) && !kmodule) { - *remap_kernel = false; - map__set_start(map, shdr->sh_addr + ref_reloc(kmap)); - map__set_end(map, map__start(map) + shdr->sh_size); - map__set_pgoff(map, shdr->sh_offset); - map__set_mapping_type(map, MAPPING_TYPE__DSO); - /* Ensure maps are correctly ordered */ - if (kmaps) { - int err; - struct map *tmp = map__get(map); + struct remap_kernel_ctx ctx = { + .sh_addr = shdr->sh_addr, + .sh_size = shdr->sh_size, + .sh_offset = shdr->sh_offset, + .kmap = kmap + }; - maps__remove(kmaps, map); - err = maps__insert(kmaps, map); - map__put(tmp); - if (err) - return err; - } + *remap_kernel = false; + maps__mutate_mapping(kmaps, map, remap_kernel_cb, &ctx); } /* diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c index 0c46b24ee098..2cc911af8c81 100644 --- a/tools/perf/util/symbol.c +++ b/tools/perf/util/symbol.c @@ -48,6 +48,13 @@ #include #include +static int map_fixup_cb(struct map *map, void *data __maybe_unused) +{ + map__fixup_start(map); + map__fixup_end(map); + return 0; +} + static int dso__load_kernel_sym(struct dso *dso, struct map *map); static int dso__load_guest_kernel_sym(struct dso *dso, struct map *map); @@ -2240,10 +2247,11 @@ static int dso__load_kernel_sym(struct dso *dso, struct map *map) free(kallsyms_allocated_filename); if (err > 0 && !dso__is_kcore(dso)) { + struct maps *kmaps = map__kmaps(map); + dso__set_binary_type(dso, DSO_BINARY_TYPE__KALLSYMS); dso__set_long_name(dso, DSO__NAME_KALLSYMS, false); - map__fixup_start(map); - map__fixup_end(map); + maps__mutate_mapping(kmaps, map, map_fixup_cb, NULL); } return err; @@ -2283,10 +2291,11 @@ static int dso__load_guest_kernel_sym(struct dso *dso, struct map *map) if (err > 0) pr_debug("Using %s for symbols\n", kallsyms_filename); if (err > 0 && !dso__is_kcore(dso)) { + struct maps *kmaps = map__kmaps(map); + dso__set_binary_type(dso, DSO_BINARY_TYPE__GUEST_KALLSYMS); dso__set_long_name(dso, machine->mmap_name, false); - map__fixup_start(map); - map__fixup_end(map); + maps__mutate_mapping(kmaps, map, map_fixup_cb, NULL); } return err;