[PATCH v9 8/8] perf: proof-of-concept kallmodsyms support

From: Nick Alcock
Date: Thu Oct 27 2022 - 15:59:36 EST

This is only very partial: it adds support to 'perf kallsyms', allowing
you to say, e.g.

% ./perf kallsyms __scsi_device_lookup
__scsi_device_lookup: scsi_mod (built-in) 0xffffffff9b901de0-0xffffffff9b901e30 (0xffffffff9b901de0-0xffffffff9b901e30)

and get told that this is in a built-in module. We also handle symbols
that are in multiple modules at once:

% ./perf kallsyms lio_set_fecparam
lio_set_fecparam: liquidio, liquidio_vf (built-in) 0xffffffff9b934da0-0xffffffff9b934e10 (0xffffffff9b934da0-0xffffffff9b934e10)

We do this the simplistic way, by augmenting symbols with a module array
field, the members of which are atoms in a new machine.modules red-black
tree (structured the same, and managed with the same code, as the
existing transient tree that is used to compare /proc/modules against
each other).

kallsyms symbols no longer carry a [module name] around with them that
needs cutting off whenever it's used (all users that relied on this
adjusted, I hope) but instead have that name hived off into the
per-symbol module array, with a new 'built_in' field to tell users
whether this is a built-in module or not. Since we cannot use the
presence of '[' to detect modules any more, we do it at kallmodsyms read
time by spotting _end and considering it to denote the end of the core
kernel and the start of the modular range. (I *think* this works on all
arches.)

Signed-off-by: Nick Alcock <nick.alcock@xxxxxxxxxx>
Reviewed-by: Kris Van Hees <kris.van.hees@xxxxxxxxxx>
---
tools/perf/builtin-kallsyms.c | 35 +++++-
tools/perf/util/event.c | 14 ++-
tools/perf/util/machine.c | 6 +-
tools/perf/util/machine.h | 1 +
tools/perf/util/symbol.c | 207 +++++++++++++++++++++++++---------
tools/perf/util/symbol.h | 12 +-
6 files changed, 211 insertions(+), 64 deletions(-)

diff --git a/tools/perf/builtin-kallsyms.c b/tools/perf/builtin-kallsyms.c
index c08ee81529e8..6bcec2522d2d 100644
--- a/tools/perf/builtin-kallsyms.c
+++ b/tools/perf/builtin-kallsyms.c
@@ -35,10 +35,37 @@ static int __cmd_kallsyms(int argc, const char **argv)
continue;
}

- printf("%s: %s %s %#" PRIx64 "-%#" PRIx64 " (%#" PRIx64 "-%#" PRIx64")\n",
- symbol->name, map->dso->short_name, map->dso->long_name,
- map->unmap_ip(map, symbol->start), map->unmap_ip(map, symbol->end),
- symbol->start, symbol->end);
+ if (!symbol->modules) {
+ printf("%s: %s %s %#" PRIx64 "-%#" PRIx64 " (%#" PRIx64 "-%#" PRIx64")\n",
+ symbol->name, map->dso->short_name, map->dso->long_name,
+ map->unmap_ip(map, symbol->start), map->unmap_ip(map, symbol->end),
+ symbol->start, symbol->end);
+ } else {
+ if (!symbol->built_in)
+ printf("%s: %s %s %#" PRIx64 "-%#" PRIx64 " (%#" PRIx64 "-%#" PRIx64")\n",
+ symbol->name, map->dso->short_name, map->dso->long_name,
+ map->unmap_ip(map, symbol->start), map->unmap_ip(map, symbol->end),
+ symbol->start, symbol->end);
+ else if (symbol->modules[1] == 0)
+ printf("%s: %s (built-in) %#" PRIx64 "-%#" PRIx64 " (%#" PRIx64 "-%#" PRIx64")\n",
+ symbol->name, symbol->modules[0], map->unmap_ip(map, symbol->start),
+ map->unmap_ip(map, symbol->end), symbol->start, symbol->end);
+ else { /* Symbol in multiple modules at once */
+ char **mod;
+
+ printf("%s: ", symbol->name);
+
+ for (mod = symbol->modules; *mod; mod++) {
+ if (mod != symbol->modules)
+ printf(", ");
+ printf("%s", *mod);
+ }
+
+ printf (" (built-in) %#" PRIx64 "-%#" PRIx64 " (%#" PRIx64 "-%#" PRIx64")\n",
+ map->unmap_ip(map, symbol->start), map->unmap_ip(map, symbol->end),
+ symbol->start, symbol->end);
+ }
+ }
}

machine__delete(machine);
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 1fa14598b916..a344b35f7e38 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -97,16 +97,28 @@ static int find_symbol_cb(void *arg, const char *name, char type,
u64 start)
{
struct process_symbol_args *args = arg;
+ char *chop, *tmp_alloc = NULL;
+ const char *tmp = name;
+
+ if ((chop = strchr(name, '\t')) != NULL) {
+ tmp_alloc = strndup(name, name - chop);
+ if (tmp_alloc == NULL)
+ return -ENOMEM;
+ tmp = tmp_alloc;
+ }

/*
* Must be a function or at least an alias, as in PARISC64, where "_text" is
* an 'A' to the same address as "_stext".
*/
if (!(kallsyms__is_function(type) ||
- type == 'A') || strcmp(name, args->name))
+ type == 'A') || strcmp(tmp, args->name)) {
+ free(tmp_alloc);
return 0;
+ }

args->start = start;
+ free(tmp_alloc);
return 1;
}

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 76316e459c3d..2be5a3c1a267 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -173,7 +173,7 @@ struct machine *machine__new_kallsyms(void)
* ask for not using the kcore parsing code, once this one is fixed
* to create a map per module.
*/
- if (machine && machine__load_kallsyms(machine, "/proc/kallsyms") <= 0) {
+ if (machine && machine__load_kallsyms(machine, "/proc/kallmodsyms") <= 0) {
machine__delete(machine);
machine = NULL;
}
@@ -237,6 +237,7 @@ void machine__exit(struct machine *machine)
zfree(&machine->mmap_name);
zfree(&machine->current_tid);
zfree(&machine->kallsyms_filename);
+ modules__delete_modules(&machine->modules);

for (i = 0; i < THREADS__TABLE_SIZE; i++) {
struct threads *threads = &machine->threads[i];
@@ -1410,7 +1411,8 @@ int machines__create_kernel_maps(struct machines *machines, pid_t pid)
int machine__load_kallsyms(struct machine *machine, const char *filename)
{
struct map *map = machine__kernel_map(machine);
- int ret = __dso__load_kallsyms(map->dso, filename, map, true);
+ int ret = __dso__load_kallsyms(map->dso, filename, map, &machine->modules,
+ true);

if (ret > 0) {
dso__set_loaded(map->dso);
diff --git a/tools/perf/util/machine.h b/tools/perf/util/machine.h
index 74935dfaa937..393063840cd1 100644
--- a/tools/perf/util/machine.h
+++ b/tools/perf/util/machine.h
@@ -55,6 +55,7 @@ struct machine {
struct dsos dsos;
struct maps *kmaps;
struct map *vmlinux_map;
+ struct rb_root modules;
u64 kernel_start;
pid_t *current_tid;
size_t current_tid_sz;
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index a3a165ae933a..aab7ffdd0573 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -41,10 +41,16 @@
#include <symbol/kallsyms.h>
#include <sys/utsname.h>

-static int dso__load_kernel_sym(struct dso *dso, struct map *map);
+static int dso__load_kernel_sym(struct dso *dso, struct map *map,
+ struct rb_root *modules);
static int dso__load_guest_kernel_sym(struct dso *dso, struct map *map);
static bool symbol__is_idle(const char *name);

+static int read_proc_modules(const char *filename, struct rb_root *modules);
+static struct module_info *find_module(const char *name,
+ struct rb_root *modules);
+static void add_module(struct module_info *mi, struct rb_root *modules);
+
int vmlinux_path__nr_entries;
char **vmlinux_path;

@@ -85,6 +91,12 @@ static enum dso_binary_type binary_type_symtab[] = {

#define DSO_BINARY_TYPE__SYMTAB_CNT ARRAY_SIZE(binary_type_symtab)

+struct module_info {
+ struct rb_node rb_node;
+ char *name;
+ u64 start;
+};
+
static bool symbol_type__filter(char symbol_type)
{
symbol_type = toupper(symbol_type);
@@ -234,15 +246,10 @@ void symbols__fixup_end(struct rb_root_cached *symbols, bool is_kallsyms)
* kernel text segment and beginning of first module's text
* segment is very big. Therefore do not fill this gap and do
* not assign it to the kernel dso map (kallsyms).
- *
- * In kallsyms, it determines module symbols using '[' character
- * like in:
- * ffffffffc1937000 T hdmi_driver_init [snd_hda_codec_hdmi]
*/
if (prev->end == prev->start) {
/* Last kernel/module symbol mapped to end of page */
- if (is_kallsyms && (!strchr(prev->name, '[') !=
- !strchr(curr->name, '[')))
+ if (is_kallsyms && prev->built_in != curr->built_in)
prev->end = roundup(prev->end + 4096, 4096);
else
prev->end = curr->start;
@@ -301,6 +308,8 @@ struct symbol *symbol__new(u64 start, u64 len, u8 binding, u8 type, const char *
sym->type = type;
sym->binding = binding;
sym->namelen = namelen - 1;
+ sym->modules = NULL;
+ sym->built_in = 0;

pr_debug4("%s: %s %#" PRIx64 "-%#" PRIx64 "\n",
__func__, name, start, sym->end);
@@ -318,6 +327,7 @@ void symbol__delete(struct symbol *sym)
annotation__exit(notes);
}
}
+ free(sym->modules);
free(((void *)sym) - symbol_conf.priv_size);
}

@@ -716,12 +726,37 @@ static bool symbol__is_idle(const char *name)
return strlist__has_entry(idle_symbols_list, name);
}

-static int map__process_kallsym_symbol(void *arg, const char *name,
+struct process_kallsym_symbol_arg {
+ struct dso *dso;
+ struct rb_root *modules;
+ int seen_end;
+};
+
+static int map__process_kallsym_symbol(void *arg_, const char *name,
char type, u64 start)
{
struct symbol *sym;
- struct dso *dso = arg;
+ struct process_kallsym_symbol_arg *arg = arg_;
+ struct dso *dso = arg->dso;
struct rb_root_cached *root = &dso->symbols;
+ struct rb_root *modules = arg->modules;
+ char *module;
+ const char *modulep;
+ int counting = 1;
+ size_t nmods = 0;
+ char **mods = NULL;
+ char **modp = NULL;
+
+ /*
+ * Split off the modules part.
+ */
+ if ((module = strchr(name, '\t')) != NULL) {
+ *module = 0;
+ module++;
+ }
+
+ if (strcmp(name, "_end") == 0)
+ arg->seen_end = 1;

if (!symbol_type__filter(type))
return 0;
@@ -731,18 +766,88 @@ static int map__process_kallsym_symbol(void *arg, const char *name,
return 0;

/*
- * module symbols are not sorted so we add all
- * symbols, setting length to 0, and rely on
- * symbols__fixup_end() to fix it up.
+ * non-builtin module symbols are not sorted so we add all symbols,
+ * setting length to 0, and rely on symbols__fixup_end() to fix it up.
*/
sym = symbol__new(start, 0, kallsyms2elf_binding(type), kallsyms2elf_type(type), name);
if (sym == NULL)
return -ENOMEM;
+
+ sym->built_in = !arg->seen_end;
+
+ /*
+ * Pass over the modules list twice: once to count the number of
+ * modules this symbol is part of and allocate an array to store their
+ * names, then again to fill it out.
+ *
+ * Arguably inefficient, due to one allocation per built-in symbol, even
+ * though many symbols will have the same mods array. In practice,
+ * it's just too small a waste to matter. The module names are pointers
+ * into the machine->modules rb-tree (lazily populated here).
+ */
+
+fill:
+ modulep = module;
+ while (modulep && (modulep = strchr(modulep, '[')) != NULL) {
+ struct module_info *mi;
+ const char *end_bra = strchr(modulep, ']');
+
+ modulep++;
+ if (end_bra == NULL || end_bra <= modulep)
+ continue;
+
+ if (counting) {
+ nmods++;
+ continue;
+ }
+
+ /*
+ * Fill-out phase.
+ */
+
+ *modp = strndup(modulep, end_bra - modulep);
+ if (*modp == NULL) {
+ free(mods);
+ return -ENOMEM;
+ }
+
+ mi = find_module(*modp, modules);
+ if (!mi) {
+ mi = zalloc(sizeof(struct module_info));
+
+ if (!mi) {
+ free (mods);
+ free (*modp);
+ return -ENOMEM;
+ }
+ mi->name = *modp;
+ }
+ else {
+ free(*modp);
+ *modp = mi->name;
+ }
+
+ modp++;
+ }
+
+ if (counting && nmods > 0) {
+ mods = calloc(nmods + 1, sizeof (char *));
+ if (mods == NULL)
+ return -ENOMEM;
+ modp = mods;
+
+ counting = 0;
+ goto fill;
+ }
+
+ sym->modules = mods;
+
/*
* We will pass the symbols to the filter later, in
- * map__split_kallsyms, when we have split the maps per module
+ * map__split_kallsyms, when we have split the maps per
+ * (non-built-in) module
*/
- __symbols__insert(root, sym, !strchr(name, '['));
+ __symbols__insert(root, sym, !arg->seen_end);

return 0;
}
@@ -752,9 +857,11 @@ static int map__process_kallsym_symbol(void *arg, const char *name,
* so that we can in the next step set the symbol ->end address and then
* call kernel_maps__split_kallsyms.
*/
-static int dso__load_all_kallsyms(struct dso *dso, const char *filename)
+static int dso__load_all_kallsyms(struct dso *dso, const char *filename,
+ struct rb_root *modules)
{
- return kallsyms__parse(filename, dso, map__process_kallsym_symbol);
+ struct process_kallsym_symbol_arg arg = {dso, modules, 0};
+ return kallsyms__parse(filename, &arg, map__process_kallsym_symbol);
}

static int maps__split_kallsyms_for_kcore(struct maps *kmaps, struct dso *dso)
@@ -766,22 +873,14 @@ static int maps__split_kallsyms_for_kcore(struct maps *kmaps, struct dso *dso)
struct rb_root_cached *root = &dso->symbols;
struct rb_node *next = rb_first_cached(root);

- if (!kmaps)
- return -1;
-
*root = RB_ROOT_CACHED;

while (next) {
- char *module;
-
pos = rb_entry(next, struct symbol, rb_node);
next = rb_next(&pos->rb_node);

rb_erase_cached(&pos->rb_node, &old_root);
RB_CLEAR_NODE(&pos->rb_node);
- module = strchr(pos->name, '\t');
- if (module)
- *module = '\0';

curr_map = maps__find(kmaps, pos->start);

@@ -830,19 +929,19 @@ static int maps__split_kallsyms(struct maps *kmaps, struct dso *dso, u64 delta,
x86_64 = machine__is(machine, "x86_64");

while (next) {
- char *module;
-
pos = rb_entry(next, struct symbol, rb_node);
next = rb_next(&pos->rb_node);

- module = strchr(pos->name, '\t');
- if (module) {
+ if (!pos->built_in && pos->modules) {
if (!symbol_conf.use_modules)
goto discard_symbol;

- *module++ = '\0';
-
- if (strcmp(curr_map->dso->short_name, module)) {
+ /*
+ * Non-built-in symbols can only be in one module at
+ * once.
+ */
+ assert(pos->modules[1] == NULL);
+ if (strcmp(curr_map->dso->short_name, pos->modules[0])) {
if (curr_map != initial_map &&
dso->kernel == DSO_SPACE__KERNEL_GUEST &&
machine__is_default_guest(machine)) {
@@ -856,12 +955,12 @@ static int maps__split_kallsyms(struct maps *kmaps, struct dso *dso, u64 delta,
dso__set_loaded(curr_map->dso);
}

- curr_map = maps__find_by_name(kmaps, module);
+ curr_map = maps__find_by_name(kmaps, pos->modules[0]);
if (curr_map == NULL) {
pr_debug("%s/proc/{kallsyms,modules} "
"inconsistency while looking "
"for \"%s\" module!\n",
- machine->root_dir, module);
+ machine->root_dir, pos->modules[0]);
curr_map = initial_map;
goto discard_symbol;
}
@@ -971,12 +1070,6 @@ bool symbol__restricted_filename(const char *filename,
return restricted;
}

-struct module_info {
- struct rb_node rb_node;
- char *name;
- u64 start;
-};
-
static void add_module(struct module_info *mi, struct rb_root *modules)
{
struct rb_node **p = &modules->rb_node;
@@ -995,7 +1088,7 @@ static void add_module(struct module_info *mi, struct rb_root *modules)
rb_insert_color(&mi->rb_node, modules);
}

-static void delete_modules(struct rb_root *modules)
+void modules__delete_modules(struct rb_root *modules)
{
struct module_info *mi;
struct rb_node *next = rb_first(modules);
@@ -1060,7 +1153,7 @@ static int read_proc_modules(const char *filename, struct rb_root *modules)
return -1;

if (modules__parse(filename, modules, __read_proc_modules)) {
- delete_modules(modules);
+ modules__delete_modules(modules);
return -1;
}

@@ -1101,9 +1194,9 @@ int compare_proc_modules(const char *from, const char *to)
if (!from_node && !to_node)
ret = 0;

- delete_modules(&to_modules);
+ modules__delete_modules(&to_modules);
out_delete_from:
- delete_modules(&from_modules);
+ modules__delete_modules(&from_modules);

return ret;
}
@@ -1133,7 +1226,7 @@ static int do_validate_kcore_modules(const char *filename, struct maps *kmaps)
}
}
out:
- delete_modules(&modules);
+ modules__delete_modules(&modules);
return err;
}

@@ -1467,18 +1560,20 @@ static int kallsyms__delta(struct kmap *kmap, const char *filename, u64 *delta)
}

int __dso__load_kallsyms(struct dso *dso, const char *filename,
- struct map *map, bool no_kcore)
+ struct map *map, struct rb_root *modules,
+ bool no_kcore)
{
struct kmap *kmap = map__kmap(map);
u64 delta = 0;

- if (symbol__restricted_filename(filename, "/proc/kallsyms"))
+ if (symbol__restricted_filename(filename, "/proc/kallsyms") &&
+ symbol__restricted_filename(filename, "/proc/kallmodsyms"))
return -1;

if (!kmap || !kmap->kmaps)
return -1;

- if (dso__load_all_kallsyms(dso, filename) < 0)
+ if (dso__load_all_kallsyms(dso, filename, modules) < 0)
return -1;

if (kallsyms__delta(kmap, filename, &delta))
@@ -1499,9 +1594,9 @@ int __dso__load_kallsyms(struct dso *dso, const char *filename,
}

int dso__load_kallsyms(struct dso *dso, const char *filename,
- struct map *map)
+ struct map *map, struct rb_root *modules)
{
- return __dso__load_kallsyms(dso, filename, map, false);
+ return __dso__load_kallsyms(dso, filename, map, modules, false);
}

static int dso__load_perf_map(const char *map_path, struct dso *dso)
@@ -1814,12 +1909,13 @@ int dso__load(struct dso *dso, struct map *map)
dso->symtab_type == DSO_BINARY_TYPE__GUEST_KMODULE_COMP;

if (dso->kernel && !kmod) {
+ machine = map__kmaps(map)->machine;
+
if (dso->kernel == DSO_SPACE__KERNEL)
- ret = dso__load_kernel_sym(dso, map);
+ ret = dso__load_kernel_sym(dso, map, &machine->modules);
else if (dso->kernel == DSO_SPACE__KERNEL_GUEST)
ret = dso__load_guest_kernel_sym(dso, map);

- machine = map__kmaps(map)->machine;
if (machine__is(machine, "x86_64"))
machine__map_x86_64_entry_trampolines(machine, dso);
goto out;
@@ -2220,7 +2316,8 @@ static char *dso__find_kallsyms(struct dso *dso, struct map *map)
return strdup(path);
}

-static int dso__load_kernel_sym(struct dso *dso, struct map *map)
+static int dso__load_kernel_sym(struct dso *dso, struct map *map,
+ struct rb_root *modules)
{
int err;
const char *kallsyms_filename = NULL;
@@ -2282,7 +2379,7 @@ static int dso__load_kernel_sym(struct dso *dso, struct map *map)
kallsyms_filename = kallsyms_allocated_filename;

do_kallsyms:
- err = dso__load_kallsyms(dso, kallsyms_filename, map);
+ err = dso__load_kallsyms(dso, kallsyms_filename, map, modules);
if (err > 0)
pr_debug("Using %s for symbols\n", kallsyms_filename);
free(kallsyms_allocated_filename);
@@ -2323,11 +2420,11 @@ static int dso__load_guest_kernel_sym(struct dso *dso, struct map *map)
if (!kallsyms_filename)
return -1;
} else {
- sprintf(path, "%s/proc/kallsyms", machine->root_dir);
+ sprintf(path, "%s/proc/kallmodsyms", machine->root_dir);
kallsyms_filename = path;
}

- err = dso__load_kallsyms(dso, kallsyms_filename, map);
+ err = dso__load_kallsyms(dso, kallsyms_filename, map, &machine->modules);
if (err > 0)
pr_debug("Using %s for symbols\n", kallsyms_filename);
if (err > 0 && !dso__is_kcore(dso)) {
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 0b893dcc8ea6..9ca218e09acf 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -66,6 +66,11 @@ struct symbol {
u8 annotate2:1;
/** Architecture specific. Unused except on PPC where it holds st_other. */
u8 arch_sym;
+ /** Null-terminated array of pointers to names of containing modules in the
+ modules red-black tree. May be NULL for none. */
+ char **modules;
+ /** Set if this symbol is built in to the core kernel. */
+ int built_in;
/** The name of length namelen associated with the symbol. */
char name[];
};
@@ -137,8 +142,9 @@ int dso__load_vmlinux(struct dso *dso, struct map *map,
const char *vmlinux, bool vmlinux_allocated);
int dso__load_vmlinux_path(struct dso *dso, struct map *map);
int __dso__load_kallsyms(struct dso *dso, const char *filename, struct map *map,
- bool no_kcore);
-int dso__load_kallsyms(struct dso *dso, const char *filename, struct map *map);
+ struct rb_root *modules, bool no_kcore);
+int dso__load_kallsyms(struct dso *dso, const char *filename, struct map *map,
+ struct rb_root *modules);

void dso__insert_symbol(struct dso *dso,
struct symbol *sym);
@@ -161,6 +167,8 @@ int sysfs__read_build_id(const char *filename, struct build_id *bid);
int modules__parse(const char *filename, void *arg,
int (*process_module)(void *arg, const char *name,
u64 start, u64 size));
+void modules__delete_modules(struct rb_root *modules);
+
int filename__read_debuglink(const char *filename, char *debuglink,
size_t size);

--
2.38.0.266.g481848f278