Em Fri, Jun 23, 2017 at 02:48:27PM +0900, Namhyung Kim escreveu:
> 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
> @@ -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);
> + 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:
diff --git a/kernel/module.c b/kernel/module.c
index 4a3665f8f837..50e9718a141d 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -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
> + }
> +
> + return ret;
> }
> int __weak arch__fix_module_text_start(u64 *start __maybe_unused,
> const char *name __maybe_unused)
> diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
> index 88361eeae813..59370ceb87c4 100644
> --- a/tools/perf/util/symbol.h
> +++ b/tools/perf/util/symbol.h
> @@ -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;
> --
> 2.13.1
