perf tools: Support out-of-tree modules

If a module is already loaded, it should have symbols and no need to load new symbols from kallsyms. Actually kallsyms can have different addresses if the module was reloaded.

Current code just discards the first symbols only, but it should do the same for all symbols in the module. Note that the kernel doesn't set the dso->loaded bit so simply checking it would do the job IMHO.

Cc Adrian Hunter <[email protected]>

Cc Wang Nan <[email protected]>

Signed-off-by Namhyung Kim <[email protected]>

---
 tools/perf/util/symbol.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

tools/perf/util/symbol.c +5 -4

index e7a98dbd2aed..74078ba595b3 100644
@@ -787,11 +787,12 @@ static int dso__split_kallsyms(struct dso *dso, struct map *map, u64 delta)
 					curr_map = map;
 					goto discard_symbol;
 				}
-
-				if (curr_map->dso->loaded &&
-				    !machine__is_default_guest(machine))
-					goto discard_symbol;
 			}
+
+			if (curr_map->dso->loaded &&
+			    !machine__is_default_guest(machine))
+				goto discard_symbol;
+
 			/*
 			 * So that we look just like we get from .ko files,
 			 * i.e. not prelinked, relative to map->start.

Signature · 1 line

2.13.1

Em Fri, Jun 23, 2017 at 02:48:21PM +0900, Namhyung Kim escreveu:

Quoted 3 lines

If a module is already loaded, it should have symbols and no need to load new symbols from kallsyms. Actually kallsyms can have different addresses if the module was reloaded.

Well, if it is loaded, then it should match what is in kallsyms, no?

Quoted 4 lines

Current code just discards the first symbols only, but it should do the same for all symbols in the module. Note that the kernel doesn't set the dso->loaded bit so simply checking it would do the job IMHO.

The kernel sets dso->loaded? Can you rephrase this part? And if this procedure doesn't set dso->loaded how can simply checking it do the job?

Can you clarify the above sentence? Please elaborate the scenario more fully.

- Arnaldo

Quoted 30 lines

Cc Adrian Hunter <[email protected]>

Cc Wang Nan <[email protected]>

Signed-off-by Namhyung Kim <[email protected]>

---
 tools/perf/util/symbol.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

tools/perf/util/symbol.c +5 -4

index e7a98dbd2aed..74078ba595b3 100644
@@ -787,11 +787,12 @@ static int dso__split_kallsyms(struct dso *dso, struct map *map, u64 delta)
 					curr_map = map;
 					goto discard_symbol;
 				}
-
-				if (curr_map->dso->loaded &&
-				    !machine__is_default_guest(machine))
-					goto discard_symbol;
 			}
+
+			if (curr_map->dso->loaded &&
+			    !machine__is_default_guest(machine))
+				goto discard_symbol;
+
 			/*
 			 * So that we look just like we get from .ko files,
 			 * i.e. not prelinked, relative to map->start.

Signature · 1 line

2.13.1

When loading kernel symbols from /proc/kallsyms, it might have different addresses for modules. We should honor the mmap event recorded in a perf.data so load the module symbols when it sees the event so that it cannot be overridden by symbols in /proc/kallsyms later.

Cc Adrian Hunter <[email protected]>

Cc Wang Nan <[email protected]>

Signed-off-by Namhyung Kim <[email protected]>

---
 tools/perf/util/machine.c | 3 +++
 1 file changed, 3 insertions(+)

tools/perf/util/machine.c +0 -0

index d838f893e639..799efe920f0c 100644

tools/perf/util/machine.c +1 -0

+
 	/* Put the map here because map_groups__insert alread got it */
 	map__put(map);
 out:

Signature · 1 line

2.13.1

Em Fri, Jun 23, 2017 at 02:48:22PM +0900, Namhyung Kim escreveu:

Quoted 17 lines

When loading kernel symbols from /proc/kallsyms, it might have different addresses for modules. We should honor the mmap event recorded in a perf.data so load the module symbols when it sees the event so that it cannot be overridden by symbols in /proc/kallsyms later.

Cc Adrian Hunter <[email protected]>

Cc Wang Nan <[email protected]>

Signed-off-by Namhyung Kim <[email protected]>

---
 tools/perf/util/machine.c | 3 +++
 1 file changed, 3 insertions(+)

tools/perf/util/machine.c +0 -0

index d838f893e639..799efe920f0c 100644

tools/perf/util/machine.c struct map *machine__findnew_module_map(struct machine *machine, u64 start, · line 660

Quoted 5 lines

tools/perf/util/machine.c +2 -0

@@ -656,6 +656,9 @@ struct map *machine__findnew_module_map(struct machine *machine, u64 start,
 
 	map_groups__insert(&machine->kmaps, map);
 
+	if (dso->has_build_id)
+		map__load(map);

How could this be set at this point? I just tried processing a PERF_RECORD_MMAP event for a kernel module and at this point it is set to false :-\

Humm, but I did it for video.ko, that has no hits in this case, so this must be coming from reading the perf.data build-id table... /me tries that... yeah, when processing the buildid table in perf.data we populate the machine->dsos list and set the dso->has_build_id flag:

[root@jouet ~]# perf buildid-list | grep .ko
57a47c2f0d85ef5726b80e8f55403c3f508a4eac /lib/modules/4.12.0-rc4+/kernel/drivers/net/wireless/intel/iwlwifi/iwlwifi.ko
[root@jouet ~]#
(gdb) bt
#0 machine__findnew_module_map (machine=0x21c6c78, start=18446744072647282688, filename=0x7fd8fc057a00 "/lib/modules/4.12.0-rc4+/kernel/drivers/net/wireless/intel/iwlwifi/iwlwifi.ko")

    at util/machine.c:650
#1  0x00000000005094a3 in machine__process_kernel_mmap_event (machine=0x21c6c78, event=0x7fd8fc0579d8) at util/machine.c:1274
#2  0x00000000005099de in machine__process_mmap_event (machine=0x21c6c78, event=0x7fd8fc0579d8, sample=0x7fffffffbef0) at util/machine.c:1433
#3  0x00000000004cc021 in perf_event__process_mmap (tool=0x7fffffffc440, event=0x7fd8fc0579d8, sample=0x7fffffffbef0, machine=0x21c6c78) at util/event.c:1265
#4  0x0000000000512686 in machines__deliver_event (machines=0x21c6c78, evlist=0x21c6f30, event=0x7fd8fc0579d8, sample=0x7fffffffbef0, tool=0x7fffffffc440, file_offset=10712)
    at util/session.c:1250
#5  0x00000000005129ce in perf_session__deliver_event (session=0x21c6b90, event=0x7fd8fc0579d8, sample=0x7fffffffbef0, tool=0x7fffffffc440, file_offset=10712) at util/session.c:1310
#6  0x0000000000513255 in perf_session__process_event (session=0x21c6b90, event=0x7fd8fc0579d8, file_offset=10712) at util/session.c:1490
#7  0x0000000000513ff4 in __perf_session__process_events (session=0x21c6b90, data_offset=232, data_size=23120, file_size=23352) at util/session.c:1867
#8  0x00000000005141f2 in perf_session__process_events (session=0x21c6b90) at util/session.c:1921
#9  0x00000000004427c9 in __cmd_report (rep=0x7fffffffc440) at builtin-report.c:575

#10 0x000000000044427c in cmd_report (argc=0, argv=0x7fffffffe160) at builtin-report.c:1054 #11 0x00000000004bad91 in run_builtin (p=0xa28fd0 <commands+240>, argc=2, argv=0x7fffffffe160) at perf.c:296 #12 0x00000000004baffe in handle_internal_command (argc=2, argv=0x7fffffffe160) at perf.c:348 #13 0x00000000004bb150 in run_argv (argcp=0x7fffffffdfbc, argv=0x7fffffffdfb0) at perf.c:392 #14 0x00000000004bb52a in main (argc=2, argv=0x7fffffffe160) at perf.c:530
(gdb) p dso
$127 = (struct dso *) 0x21c7dc0
(gdb) p dso->has_build_id
$128 = 1 '\001'
(gdb)

So what you're doing is to load it here... perhaps we should not do that, and instead in the kallsyms loading code, when processing the symbols for this specific module, discard them and leave its symbol table unpopulated, then, later, when resolving a sample in this module, it will see that it is not loaded, will see that it has a build-id and will use that, and _refuse_ to get it from anywhere else than an ELF file with that build-id.

I.e. in general, when we have a build-id for a given DSO, we should not read it from kallsyms or any other file that don't have a matching build-id.

The only possibility would be: hey, I have a build id, and reading the currently loaded module (/sys/module/e1000e/notes/.note.gnu.build-id) I see it _still_ has that same build-id, ok, I can read it from kallsyms.

What do you think?

When /proc/kallsyms is used for kernel address, addresses in module can be changed when the module is reloaded. So if one did perf record with some module and then for some reason reload the module. Then perf report might see a different address for the module and the output can show incorrect symbols.

For example, let a module XXX was loaded at 0xffffffff8a000000, the /proc/kallsyms might show following:

...
0xffffffff81234560 t last_symbol_in_vmlinux
0xffffffff8a000000 t first_symbol_in_XXX
...

As kallsyms fixs up the symbol and map address by using next address, the end address of last_symbol and kernel map would be start address of XXX (0xffffffff8a000000). And samples in 0xffffffff8a001234 can be found in a map for XXX module.

But later, XXX was reloaded at 0xffffffff8a007000, slightly higher than before. Now reading /proc/kallsyms tells that the end address of last symbol would be 0xfffffff8a007000 and so kernel map is same. Now samples in 0xffffffff8a001234 - formerly in the XXX module - would go to the kernel map and show no symbols.

In this case, perf can know the address of map of XXX from mmap event in perf.data so it can adjust kernel map address using the address of XXX map. To do that, replace map__fixup_end() by __map_groups__fixup_end(). This still have incorrect end address of last symbol in the kernel map but it's ok since samples in that address wouldn't go to the kernel map anyway.

Cc Adrian Hunter <[email protected]>

Cc Wang Nan <[email protected]>

Signed-off-by Namhyung Kim <[email protected]>

---
 tools/perf/util/symbol.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

tools/perf/util/symbol.c +2 -2

index 74078ba595b3..ce79a51f25bf 100644
@@ -1832,7 +1832,7 @@ static int dso__load_kernel_sym(struct dso *dso, struct map *map)
 		dso->binary_type = DSO_BINARY_TYPE__KALLSYMS;
 		dso__set_long_name(dso, DSO__NAME_KALLSYMS, false);
 		map__fixup_start(map);
-		map__fixup_end(map);
+		__map_groups__fixup_end(map->groups, map->type);
 	}
 
 	return err;

@@ -1880,7 +1880,7 @@ static int dso__load_guest_kernel_sym(struct dso *dso, struct map *map)
 		machine__mmap_name(machine, path, sizeof(path));
 		dso__set_long_name(dso, strdup(path), true);
 		map__fixup_start(map);
-		map__fixup_end(map);
+		__map_groups__fixup_end(map->groups, map->type);
 	}
 
 	return err;

Signature · 1 line

2.13.1

Em Fri, Jun 23, 2017 at 02:48:23PM +0900, Namhyung Kim escreveu:

Quoted 5 lines

When /proc/kallsyms is used for kernel address, addresses in module can be changed when the module is reloaded. So if one did perf record with some module and then for some reason reload the module. Then perf report might see a different address for the module and the output can show incorrect symbols.

If we, as mentioned in another message, make sure that no module reload is performed while perf record is running, and then refuse to use kallsyms if the buildid for the running module is different from the one in the perf.data build id header, then this is solved, no?

Quoted 58 lines

For example, let a module XXX was loaded at 0xffffffff8a000000, the /proc/kallsyms might show following:

...
0xffffffff81234560 t last_symbol_in_vmlinux
0xffffffff8a000000 t first_symbol_in_XXX
...

As kallsyms fixs up the symbol and map address by using next address, the end address of last_symbol and kernel map would be start address of XXX (0xffffffff8a000000). And samples in 0xffffffff8a001234 can be found in a map for XXX module.

But later, XXX was reloaded at 0xffffffff8a007000, slightly higher than before. Now reading /proc/kallsyms tells that the end address of last symbol would be 0xfffffff8a007000 and so kernel map is same. Now samples in 0xffffffff8a001234 - formerly in the XXX module - would go to the kernel map and show no symbols.

In this case, perf can know the address of map of XXX from mmap event in perf.data so it can adjust kernel map address using the address of XXX map. To do that, replace map__fixup_end() by __map_groups__fixup_end(). This still have incorrect end address of last symbol in the kernel map but it's ok since samples in that address wouldn't go to the kernel map anyway.

Cc Adrian Hunter <[email protected]>

Cc Wang Nan <[email protected]>

Signed-off-by Namhyung Kim <[email protected]>

---
 tools/perf/util/symbol.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

tools/perf/util/symbol.c +2 -2

index 74078ba595b3..ce79a51f25bf 100644
@@ -1832,7 +1832,7 @@ static int dso__load_kernel_sym(struct dso *dso, struct map *map)
 		dso->binary_type = DSO_BINARY_TYPE__KALLSYMS;
 		dso__set_long_name(dso, DSO__NAME_KALLSYMS, false);
 		map__fixup_start(map);
-		map__fixup_end(map);
+		__map_groups__fixup_end(map->groups, map->type);
 	}
 
 	return err;

@@ -1880,7 +1880,7 @@ static int dso__load_guest_kernel_sym(struct dso *dso, struct map *map)
 		machine__mmap_name(machine, path, sizeof(path));
 		dso__set_long_name(dso, strdup(path), true);
 		map__fixup_start(map);
-		map__fixup_end(map);
+		__map_groups__fixup_end(map->groups, map->type);
 	}
 
 	return err;

Signature · 1 line

2.13.1

Even every module has loaded onto same addresses, some modules can be changed and reloaded.

In that case it needs to access to the old module in the build-id cache.

Cc Adrian Hunter <[email protected]>

Cc Wang Nan <[email protected]>

Signed-off-by Namhyung Kim <[email protected]>

---
 tools/perf/util/symbol.c | 45 +++++++++++++++++++++++----------------------
 1 file changed, 23 insertions(+), 22 deletions(-)

tools/perf/util/symbol.c +23 -22

index ce79a51f25bf..fe46eb782297 100644
@@ -1155,6 +1155,7 @@ static int dso__load_kcore(struct dso *dso, struct map *map,
 	int err, fd;
 	char kcore_filename[PATH_MAX];
 	struct symbol *sym;
+	struct map_groups old_mg;
 
 	if (!kmaps)
 		return -EINVAL;

@@ -1196,15 +1197,8 @@ static int dso__load_kcore(struct dso *dso, struct map *map,
 		goto out_err;
 	}
 
-	/* Remove old maps */
-	old_map = map_groups__first(kmaps, map->type);
-	while (old_map) {
-		struct map *next = map_groups__next(old_map);
-
-		if (old_map != map)
-			map_groups__remove(kmaps, old_map);
-		old_map = next;
-	}
+	old_mg = *kmaps;
+	kmaps->maps[map->type].entries = RB_ROOT;
 
 	/* Find the kernel map using the first symbol */
 	sym = dso__first_symbol(dso, map->type);

@@ -1223,24 +1217,31 @@ static int dso__load_kcore(struct dso *dso, struct map *map,
 	while (!list_empty(&md.maps)) {
 		new_map = list_entry(md.maps.next, struct map, node);
 		list_del_init(&new_map->node);
-		if (new_map == replacement_map) {
-			map->start	= new_map->start;
-			map->end	= new_map->end;
-			map->pgoff	= new_map->pgoff;
-			map->map_ip	= new_map->map_ip;
-			map->unmap_ip	= new_map->unmap_ip;
-			/* Ensure maps are correctly ordered */
-			map__get(map);
-			map_groups__remove(kmaps, map);
-			map_groups__insert(kmaps, map);
-			map__put(map);
-		} else {
-			map_groups__insert(kmaps, new_map);
+
+		map_groups__insert(kmaps, new_map);
+
+		if (new_map != replacement_map) {
+			old_map = map_groups__find(&old_mg, new_map->type, new_map->start);
+			if (old_map && dso__loaded(old_map->dso, old_map->type)) {
+				new_map->pgoff = old_map->pgoff;
+
+				dso__put(new_map->dso);
+				new_map->dso = dso__get(old_map->dso);
+			}
 		}
 
 		map__put(new_map);
 	}
 
+	/* Remove old maps */
+	old_map = map_groups__first(&old_mg, map->type);
+	while (old_map) {
+		struct map *next = map_groups__next(old_map);
+
+		map_groups__remove(&old_mg, old_map);
+		old_map = next;
+	}
+
 	/*
 	 * Set the data type and long name so that kcore can be read via
 	 * dso__data_read_addr().

Signature · 1 line

2.13.1

Em Fri, Jun 23, 2017 at 02:48:24PM +0900, Namhyung Kim escreveu:

Quoted 2 lines

Even every module has loaded onto same addresses, some modules can be changed and reloaded.

Can you rephrase the above statement?

You mean even if a module is reloaded at the same address it can have a different symtab and thus misresolve symbols if we get a sample for one module version and try to resolve symbols using a symtab for another module version?

Quoted 3 lines

In that case it needs to access to the old module in the build-id cache.

Right, we need to make sure that during a 'perf record' session a módule isn't reloaded, because at 'perf record' exit we will record that module build-id in a perf.data header and grab a copy (or a hardlink, if possible) into the ~/.debug build-id cache, then, at 'perf report' time we will read that build-id perf.data table and lookup in the build-id cache to get the matching symtab (and full elf .ko file to do things like annotation).

Quoted 85 lines

Cc Adrian Hunter <[email protected]>

Cc Wang Nan <[email protected]>

Signed-off-by Namhyung Kim <[email protected]>

---
 tools/perf/util/symbol.c | 45 +++++++++++++++++++++++----------------------
 1 file changed, 23 insertions(+), 22 deletions(-)

tools/perf/util/symbol.c +23 -22

index ce79a51f25bf..fe46eb782297 100644
@@ -1155,6 +1155,7 @@ static int dso__load_kcore(struct dso *dso, struct map *map,
 	int err, fd;
 	char kcore_filename[PATH_MAX];
 	struct symbol *sym;
+	struct map_groups old_mg;
 
 	if (!kmaps)
 		return -EINVAL;

@@ -1196,15 +1197,8 @@ static int dso__load_kcore(struct dso *dso, struct map *map,
 		goto out_err;
 	}
 
-	/* Remove old maps */
-	old_map = map_groups__first(kmaps, map->type);
-	while (old_map) {
-		struct map *next = map_groups__next(old_map);
-
-		if (old_map != map)
-			map_groups__remove(kmaps, old_map);
-		old_map = next;
-	}
+	old_mg = *kmaps;
+	kmaps->maps[map->type].entries = RB_ROOT;
 
 	/* Find the kernel map using the first symbol */
 	sym = dso__first_symbol(dso, map->type);

@@ -1223,24 +1217,31 @@ static int dso__load_kcore(struct dso *dso, struct map *map,
 	while (!list_empty(&md.maps)) {
 		new_map = list_entry(md.maps.next, struct map, node);
 		list_del_init(&new_map->node);
-		if (new_map == replacement_map) {
-			map->start	= new_map->start;
-			map->end	= new_map->end;
-			map->pgoff	= new_map->pgoff;
-			map->map_ip	= new_map->map_ip;
-			map->unmap_ip	= new_map->unmap_ip;
-			/* Ensure maps are correctly ordered */
-			map__get(map);
-			map_groups__remove(kmaps, map);
-			map_groups__insert(kmaps, map);
-			map__put(map);
-		} else {
-			map_groups__insert(kmaps, new_map);
+
+		map_groups__insert(kmaps, new_map);
+
+		if (new_map != replacement_map) {
+			old_map = map_groups__find(&old_mg, new_map->type, new_map->start);
+			if (old_map && dso__loaded(old_map->dso, old_map->type)) {
+				new_map->pgoff = old_map->pgoff;
+
+				dso__put(new_map->dso);
+				new_map->dso = dso__get(old_map->dso);
+			}
 		}
 
 		map__put(new_map);
 	}
 
+	/* Remove old maps */
+	old_map = map_groups__first(&old_mg, map->type);
+	while (old_map) {
+		struct map *next = map_groups__next(old_map);
+
+		map_groups__remove(&old_mg, old_map);
+		old_map = next;
+	}
+
 	/*
 	 * Set the data type and long name so that kcore can be read via
 	 * dso__data_read_addr().

Signature · 1 line

2.13.1

Currently perf only searches module binaries on the canonical directory (/lib/modules/uname -r). But sometimes user needs to load local modules. These cannot be copied to the build-id cache since long name (i.e. real path) of DSOs was not set.

This patch fixes the problem by adding a new --module-dir option so that perf can record modules in the directory.

+++ b/tools/perf/Documentation/perf-record.txt

(quoted diff) +6 -1

@@ -480,6 +480,9 @@ Implies --tail-synthesize.
+--module-dir=PATH::
+Directory name where extra modules are located.
 
+++ b/tools/perf/builtin-record.c

@@ -1671,6 +1671,8 @@ static struct option __record_options[] = {
 		    "Parse options then exit"),
 	OPT_BOOLEAN(0, "use-kcore", &symbol_conf.use_kcore,
 		    "Use /proc/kcore for object code"),
+	OPT_STRING(0, "module-dir", &symbol_conf.extra_module_path, "path",
+		   "directory name where extra modules are located"),
 	OPT_END()
 };
 
+++ b/tools/perf/util/machine.c

@@ -1117,7 +1118,19 @@ static int machine__set_modules_path(struct machine *machine)
 		 machine->root_dir, version);
 	free(version);
 
-	return map_groups__set_modules_path_dir(&machine->kmaps, modules_path, 0);

(quoted diff) +3 -0

+	}
+
+	return ret;
 }
 int __weak arch__fix_module_text_start(u64 *start __maybe_unused,
 				const char *name __maybe_unused)

tools/perf/util/symbol.h +2 -1

index 88361eeae813..59370ceb87c4 100644
@@ -124,7 +124,8 @@ struct symbol_conf {
 	const char	*vmlinux_name,
 			*kallsyms_name,
 			*source_prefix,
-			*field_sep;
+			*field_sep,
+			*extra_module_path;
 	const char	*default_guest_vmlinux_name,
 			*default_guest_kallsyms,
 			*default_guest_modules;

Signature · 1 line

2.13.1

Em Fri, Jun 23, 2017 at 02:48:27PM +0900, Namhyung Kim escreveu:

Quoted 29 lines

Currently perf only searches module binaries on the canonical directory (/lib/modules/uname -r). But sometimes user needs to load local modules. These cannot be copied to the build-id cache since long name (i.e. real path) of DSOs was not set.

This patch fixes the problem by adding a new --module-dir option so that perf can record modules in the directory.

+++ b/tools/perf/Documentation/perf-record.txt

(quoted diff) +6 -1

@@ -480,6 +480,9 @@ Implies --tail-synthesize.
+--module-dir=PATH::
+Directory name where extra modules are located.
 
+++ b/tools/perf/builtin-record.c

@@ -1671,6 +1671,8 @@ static struct option __record_options[] = {
 		    "Parse options then exit"),
 	OPT_BOOLEAN(0, "use-kcore", &symbol_conf.use_kcore,
 		    "Use /proc/kcore for object code"),
+	OPT_STRING(0, "module-dir", &symbol_conf.extra_module_path, "path",
+		   "directory name where extra modules are located"),
 	OPT_END()
 };
 
+++ b/tools/perf/util/machine.c

@@ -1117,7 +1118,19 @@ static int machine__set_modules_path(struct machine *machine)
 		 machine->root_dir, version);
 	free(version);
 
-	return map_groups__set_modules_path_dir(&machine->kmaps, modules_path, 0);

Review static int machine__set_modules_path(struct machine *machine) · line 1130

Quoted 10 lines

(quoted diff) +10 -0

+	ret = map_groups__set_modules_path_dir(&machine->kmaps, modules_path, 0);
+	if (ret < 0)
+		return ret;
+
+	if (symbol_conf.extra_module_path) {
+		snprintf(modules_path, sizeof(modules_path), "%s/%s",
+			 machine->root_dir, symbol_conf.extra_module_path);
+
+		ret = map_groups__set_modules_path_dir(&machine->kmaps,
+						       modules_path, 0);

What if we have samples in a module that is in the canonical dir _and_ samples in a module in this extra_module_path? Shouldn't we have something like vmlinux_path, but for modules? Where you can add entries in that path?

I'm ok with adding entries to where module files are looked up, with semantics similar to $PATH in a shell, i.e. entries added via --module-path (rename of your --module-dir) will take precedence over the canonical dir (/lib/modules/uname -r).

But for the future I think we should try to get a PERF_RECORD_MMAP that will allow reloading modules during a 'perf record' session when a module gets reloaded, I thought about using existing tracepoints, but...

[root@jouet ~]# cat /sys/kernel/debug/tracing/events/module/module_load/format
name: module_load
ID: 370
format:

	field:unsigned short common_type;	offset:0;	size:2;	signed:0;
	field:unsigned char common_flags;	offset:2;	size:1;	signed:0;
	field:unsigned char common_preempt_count;	offset:3;	size:1;	signed:0;
	field:int common_pid;	offset:4;	size:4;	signed:1;

	field:unsigned int taints;	offset:8;	size:4;	signed:0;
	field:__data_loc char[] name;	offset:12;	size:4;	signed:1;

print fmt: "%s %s", __get_str(name), __print_flags(REC->taints, "", { (1UL << 0), "P" }, { (1UL << 12), "O" }, { (1UL << 1), "F" }, { (1UL << 10), "C" }, { (1UL << 13), "E" }) [root@jouet ~]# perf trace --no-syscalls --event module:module_load modprobe ppp
modprobe: FATAL: Module ppp not found in directory /lib/modules/4.12.0-rc4+
[root@jouet ~]# perf trace --no-syscalls --event module:module_load modprobe e1000

     0.000 module:module_load:e1000 )

[root@jouet ~]#

It just gets us the module name, not the file from what it is loaded, bummer :-\

Something like:

kernel/module.c +1 -0

index 4a3665f8f837..50e9718a141d 100644
@@ -3739,6 +3739,7 @@ static int load_module(struct load_info *info, const char __user *uargs,
 
 	/* Done! */
 	trace_module_load(mod);
+	perf_event_mmap_mod(mod);
 
 	return do_init_module(mod);
 

----------

struct module *mod has:

mod->name: "e1000"
mod->debug->filename: /lib/modules/4.11.0-rc8+/kernel/drivers/net/ethernet/intel/e1000/e1000.ko

and at that point we also have load_info, htat has ELF headers where we could extract the build-id and insert it in a field in a new PERF_RECORD_MMAP3 record :-)

perf_event_mmap_mod() would be modelled after perf_event_mmap(vma), but getting what it needs not from the vma present in a sys_mmap() but from struct module and load_info.

- Arnaldo

Quoted 22 lines

(quoted diff) +3 -0

+	}
+
+	return ret;
 }
 int __weak arch__fix_module_text_start(u64 *start __maybe_unused,
 				const char *name __maybe_unused)

tools/perf/util/symbol.h +2 -1

index 88361eeae813..59370ceb87c4 100644
@@ -124,7 +124,8 @@ struct symbol_conf {
 	const char	*vmlinux_name,
 			*kallsyms_name,
 			*source_prefix,
-			*field_sep;
+			*field_sep,
+			*extra_module_path;
 	const char	*default_guest_vmlinux_name,
 			*default_guest_kallsyms,
 			*default_guest_modules;

Signature · 1 line

2.13.1