Ping again? On 2015/4/15 9:27, Wang Nan wrote: > Ping? > > On 2015/4/10 11:53, Wang Nan wrote: >> perf report and perf annotate are easy to trigger segfault if trace data >> contain kernel module information like this: >> >> # perf report -D -i ./perf.data >> ... >> 0 0 0x188 [0x50]: PERF_RECORD_MMAP -1/0: [0xffffffbff1018000(0xf068000) @ >> 0]: x [test_module] >> ... >> >> # perf report -i ./perf.data --objdump=/path/to/objdump >> --kallsyms=/path/to/kallsyms >> >> perf: Segmentation fault >> -------- backtrace -------- >> /path/to/perf[0x503478] >> /lib64/libc.so.6(+0x3545f)[0x7fb201f3745f] >> /path/to/perf[0x499b56] >> /path/to/perf(dso__load_kallsyms+0x13c)[0x49b56c] >> /path/to/perf(dso__load+0x72e)[0x49c21e] >> /path/to/perf(map__load+0x6e)[0x4ae9ee] >> /path/to/perf(thread__find_addr_map+0x24c)[0x47deec] >> /path/to/perf(perf_event__preprocess_sample+0x88)[0x47e238] >> /path/to/perf[0x43ad02] >> /path/to/perf[0x4b55bc] >> /path/to/perf(ordered_events__flush+0xca)[0x4b57ea] >> /path/to/perf[0x4b1a01] >> /path/to/perf(perf_session__process_events+0x3be)[0x4b428e] >> /path/to/perf(cmd_report+0xf11)[0x43bfc1] >> /path/to/perf[0x474702] >> /path/to/perf(main+0x5f5)[0x42de95] >> /lib64/libc.so.6(__libc_start_main+0xf4)[0x7fb201f23bd4] >> /path/to/perf[0x42dfc4] >> >> This is because __kmod_path__parse regard '[' leading name as kernel >> instead of kernel module. If perf.data contain build information and >> the buildid of such modules can be found, the DSO of it will be treated >> as kernel, not kernel module. It will then be passed to >> dso__load_kernel_sym() then dso__load_kcore() because of --kallsyms >> argument. The segfault is triggered because the kmap structure is not >> initialized. >> >> Although in --vmlinux case such segfault can be avoided, the symbols in >> the kernel module are unable to be retrived since the attribute of DSO >> is incorrect. >> >> This patch fixes __kmod_path__parse, make it to treat names like >> '[test_module]' as kernel modules. >> >> kmod-path.c is also update to reflect the above changes. >> >> Signed-off-by: Wang Nan <[email protected]> >> --- >> >> Different from v4: checks cpumode in is_kernel_module(), makes code simpler. >> Appends tests of is_kernel_module(). >> --- >> tools/perf/tests/kmod-path.c | 72 >> ++++++++++++++++++++++++++++++++++++++++++++ >> tools/perf/util/dso.c | 42 +++++++++++++++++++++++--- >> tools/perf/util/dso.h | 2 +- >> tools/perf/util/header.c | 8 ++--- >> tools/perf/util/machine.c | 16 +++++++++- >> 5 files changed, 130 insertions(+), 10 deletions(-) >> >> diff --git a/tools/perf/tests/kmod-path.c b/tools/perf/tests/kmod-path.c >> index e8d7cbb..08c433b 100644 >> --- a/tools/perf/tests/kmod-path.c >> +++ b/tools/perf/tests/kmod-path.c >> @@ -34,9 +34,21 @@ static int test(const char *path, bool alloc_name, bool >> alloc_ext, >> return 0; >> } >> >> +static int test_is_kernel_module(const char *path, int cpumode, bool expect) >> +{ >> + TEST_ASSERT_VAL("is_kernel_module", >> + (!!is_kernel_module(path, cpumode)) == (!!expect)); >> + pr_debug("%s (cpumode: %d) - is_kernel_module: %s\n", >> + path, cpumode, expect ? "true" : "false"); >> + return 0; >> +} >> + >> #define T(path, an, ae, k, c, n, e) \ >> TEST_ASSERT_VAL("failed", !test(path, an, ae, k, c, n, e)) >> >> +#define M(path, c, e) \ >> + TEST_ASSERT_VAL("failed", !test_is_kernel_module(path, c, e)) >> + >> int test__kmod_path__parse(void) >> { >> /* path alloc_name alloc_ext kmod comp name >> ext */ >> @@ -44,30 +56,90 @@ int test__kmod_path__parse(void) >> T("/xxxx/xxxx/x-x.ko", false , true , true, false, NULL , >> NULL); >> T("/xxxx/xxxx/x-x.ko", true , false , true, false, "[x_x]", >> NULL); >> T("/xxxx/xxxx/x-x.ko", false , false , true, false, NULL , >> NULL); >> + M("/xxxx/xxxx/x-x.ko", PERF_RECORD_MISC_CPUMODE_UNKNOWN, true); >> + M("/xxxx/xxxx/x-x.ko", PERF_RECORD_MISC_KERNEL, true); >> + M("/xxxx/xxxx/x-x.ko", PERF_RECORD_MISC_USER, false); >> >> /* path alloc_name alloc_ext kmod comp name ext */ >> T("/xxxx/xxxx/x.ko.gz", true , true , true, true, "[x]", "gz"); >> T("/xxxx/xxxx/x.ko.gz", false , true , true, true, NULL , "gz"); >> T("/xxxx/xxxx/x.ko.gz", true , false , true, true, "[x]", NULL); >> T("/xxxx/xxxx/x.ko.gz", false , false , true, true, NULL , NULL); >> + M("/xxxx/xxxx/x.ko.gz", PERF_RECORD_MISC_CPUMODE_UNKNOWN, true); >> + M("/xxxx/xxxx/x.ko.gz", PERF_RECORD_MISC_KERNEL, true); >> + M("/xxxx/xxxx/x.ko.gz", PERF_RECORD_MISC_USER, false); >> >> /* path alloc_name alloc_ext kmod comp name ext */ >> T("/xxxx/xxxx/x.gz", true , true , false, true, "x.gz" ,"gz"); >> T("/xxxx/xxxx/x.gz", false , true , false, true, NULL ,"gz"); >> T("/xxxx/xxxx/x.gz", true , false , false, true, "x.gz" , NULL); >> T("/xxxx/xxxx/x.gz", false , false , false, true, NULL , NULL); >> + M("/xxxx/xxxx/x.gz", PERF_RECORD_MISC_CPUMODE_UNKNOWN, false); >> + M("/xxxx/xxxx/x.gz", PERF_RECORD_MISC_KERNEL, false); >> + M("/xxxx/xxxx/x.gz", PERF_RECORD_MISC_USER, false); >> >> /* path alloc_name alloc_ext kmod comp name ext */ >> T("x.gz", true , true , false, true, "x.gz", "gz"); >> T("x.gz", false , true , false, true, NULL , "gz"); >> T("x.gz", true , false , false, true, "x.gz", NULL); >> T("x.gz", false , false , false, true, NULL , NULL); >> + M("x.gz", PERF_RECORD_MISC_CPUMODE_UNKNOWN, false); >> + M("x.gz", PERF_RECORD_MISC_KERNEL, false); >> + M("x.gz", PERF_RECORD_MISC_USER, false); >> >> /* path alloc_name alloc_ext kmod comp name ext */ >> T("x.ko.gz", true , true , true, true, "[x]", "gz"); >> T("x.ko.gz", false , true , true, true, NULL , "gz"); >> T("x.ko.gz", true , false , true, true, "[x]", NULL); >> T("x.ko.gz", false , false , true, true, NULL , NULL); >> + M("x.ko.gz", PERF_RECORD_MISC_CPUMODE_UNKNOWN, true); >> + M("x.ko.gz", PERF_RECORD_MISC_KERNEL, true); >> + M("x.ko.gz", PERF_RECORD_MISC_USER, false); >> + >> + /* path alloc_name alloc_ext kmod comp name >> ext */ >> + T("[test_module]", true , true , true, false, "[test_module]", >> NULL); >> + T("[test_module]", false , true , true, false, NULL , >> NULL); >> + T("[test_module]", true , false , true, false, "[test_module]", >> NULL); >> + T("[test_module]", false , false , true, false, NULL , >> NULL); >> + M("[test_module]", PERF_RECORD_MISC_CPUMODE_UNKNOWN, true); >> + M("[test_module]", PERF_RECORD_MISC_KERNEL, true); >> + M("[test_module]", PERF_RECORD_MISC_USER, false); >> + >> + /* path alloc_name alloc_ext kmod comp name >> ext */ >> + T("[test.module]", true , true , true, false, "[test.module]", >> NULL); >> + T("[test.module]", false , true , true, false, NULL , >> NULL); >> + T("[test.module]", true , false , true, false, "[test.module]", >> NULL); >> + T("[test.module]", false , false , true, false, NULL , >> NULL); >> + M("[test.module]", PERF_RECORD_MISC_CPUMODE_UNKNOWN, true); >> + M("[test.module]", PERF_RECORD_MISC_KERNEL, true); >> + M("[test.module]", PERF_RECORD_MISC_USER, false); >> + >> + /* path alloc_name alloc_ext kmod comp name ext */ >> + T("[vdso]", true , true , false, false, "[vdso]", NULL); >> + T("[vdso]", false , true , false, false, NULL , NULL); >> + T("[vdso]", true , false , false, false, "[vdso]", NULL); >> + T("[vdso]", false , false , false, false, NULL , NULL); >> + M("[vdso]", PERF_RECORD_MISC_CPUMODE_UNKNOWN, false); >> + M("[vdso]", PERF_RECORD_MISC_KERNEL, false); >> + M("[vdso]", PERF_RECORD_MISC_USER, false); >> + >> + /* path alloc_name alloc_ext kmod comp name ext >> */ >> + T("[vsyscall]", true , true , false, false, "[vsyscall]", >> NULL); >> + T("[vsyscall]", false , true , false, false, NULL , >> NULL); >> + T("[vsyscall]", true , false , false, false, "[vsyscall]", >> NULL); >> + T("[vsyscall]", false , false , false, false, NULL , >> NULL); >> + M("[vsyscall]", PERF_RECORD_MISC_CPUMODE_UNKNOWN, false); >> + M("[vsyscall]", PERF_RECORD_MISC_KERNEL, false); >> + M("[vsyscall]", PERF_RECORD_MISC_USER, false); >> + >> + /* path alloc_name alloc_ext kmod comp name >> ext */ >> + T("[kernel.kallsyms]", true , true , false, false, >> "[kernel.kallsyms]", NULL); >> + T("[kernel.kallsyms]", false , true , false, false, NULL >> , NULL); >> + T("[kernel.kallsyms]", true , false , false, false, >> "[kernel.kallsyms]", NULL); >> + T("[kernel.kallsyms]", false , false , false, false, NULL >> , NULL); >> + M("[kernel.kallsyms]", PERF_RECORD_MISC_CPUMODE_UNKNOWN, false); >> + M("[kernel.kallsyms]", PERF_RECORD_MISC_KERNEL, false); >> + M("[kernel.kallsyms]", PERF_RECORD_MISC_USER, false); >> >> return 0; >> } >> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c >> index fc0ddd5..e9d4ae4 100644 >> --- a/tools/perf/util/dso.c >> +++ b/tools/perf/util/dso.c >> @@ -165,13 +165,25 @@ bool is_supported_compression(const char *ext) >> return false; >> } >> >> -bool is_kernel_module(const char *pathname) >> +bool is_kernel_module(const char *pathname, int cpumode) >> { >> struct kmod_path m; >> >> - if (kmod_path__parse(&m, pathname)) >> - return NULL; >> + /* caller should pass a masked cpumode. Mask again for safety. */ >> + switch (cpumode & PERF_RECORD_MISC_CPUMODE_MASK) { >> + case PERF_RECORD_MISC_USER: >> + case PERF_RECORD_MISC_HYPERVISOR: >> + case PERF_RECORD_MISC_GUEST_USER: >> + return false; >> + /* Regard PERF_RECORD_MISC_CPUMODE_UNKNOWN as kernel */ >> + default: >> + if (kmod_path__parse(&m, pathname)) { >> + pr_err("Failed to check whether %s is a kernel module >> or not. Assume it is.", >> + pathname); >> >> + return true; >> + } >> + } >> return m.kmod; >> } >> >> @@ -214,12 +226,34 @@ int __kmod_path__parse(struct kmod_path *m, const char >> *path, >> { >> const char *name = strrchr(path, '/'); >> const char *ext = strrchr(path, '.'); >> + bool is_simple_name = false; >> >> memset(m, 0x0, sizeof(*m)); >> name = name ? name + 1 : path; >> >> + /* >> + * '.' is also a valid character for module name. For example: >> + * [aaa.bbb] is a valid module name. '[' should have higher >> + * priority than '.ko' suffix. >> + * >> + * The kernel names are from machine__mmap_name. Such >> + * name should belong to kernel itself, not kernel module. >> + */ >> + if (name[0] == '[') { >> + is_simple_name = true; >> + if ((strncmp(name, "[kernel.kallsyms]", 17) == 0) || >> + (strncmp(name, "[guest.kernel.kallsyms", 22) == 0) || >> + (strncmp(name, "[vdso]", 6) == 0) || >> + (strncmp(name, "[vsyscall]", 10) == 0)) { >> + m->kmod = false; >> + >> + } else >> + m->kmod = true; >> + } >> + >> + >> /* No extension, just return name. */ >> - if (ext == NULL) { >> + if ((ext == NULL) || is_simple_name) { >> if (alloc_name) { >> m->name = strdup(name); >> return m->name ? 0 : -ENOMEM; >> diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h >> index e0901b4..cc3797c 100644 >> --- a/tools/perf/util/dso.h >> +++ b/tools/perf/util/dso.h >> @@ -216,7 +216,7 @@ char dso__symtab_origin(const struct dso *dso); >> int dso__read_binary_type_filename(const struct dso *dso, enum >> dso_binary_type type, >> char *root_dir, char *filename, size_t size); >> bool is_supported_compression(const char *ext); >> -bool is_kernel_module(const char *pathname); >> +bool is_kernel_module(const char *pathname, int cpumode); >> bool decompress_to_file(const char *ext, const char *filename, int >> output_fd); >> bool dso__needs_decompress(struct dso *dso); >> >> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c >> index fb43215..8c76a23 100644 >> --- a/tools/perf/util/header.c >> +++ b/tools/perf/util/header.c >> @@ -1232,7 +1232,7 @@ static int __event_process_build_id(struct >> build_id_event *bev, >> int err = -1; >> struct dsos *dsos; >> struct machine *machine; >> - u16 misc; >> + u16 cpumode; >> struct dso *dso; >> enum dso_kernel_type dso_type; >> >> @@ -1240,9 +1240,9 @@ static int __event_process_build_id(struct >> build_id_event *bev, >> if (!machine) >> goto out; >> >> - misc = bev->header.misc & PERF_RECORD_MISC_CPUMODE_MASK; >> + cpumode = bev->header.misc & PERF_RECORD_MISC_CPUMODE_MASK; >> >> - switch (misc) { >> + switch (cpumode) { >> case PERF_RECORD_MISC_KERNEL: >> dso_type = DSO_TYPE_KERNEL; >> dsos = &machine->kernel_dsos; >> @@ -1266,7 +1266,7 @@ static int __event_process_build_id(struct >> build_id_event *bev, >> >> dso__set_build_id(dso, &bev->build_id); >> >> - if (!is_kernel_module(filename)) >> + if (!is_kernel_module(filename, cpumode)) >> dso->kernel = dso_type; >> >> build_id__sprintf(dso->build_id, sizeof(dso->build_id), >> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c >> index e335330..3769009 100644 >> --- a/tools/perf/util/machine.c >> +++ b/tools/perf/util/machine.c >> @@ -1109,7 +1109,21 @@ static int machine__process_kernel_mmap_event(struct >> machine *machine, >> struct dso *dso; >> >> list_for_each_entry(dso, &machine->kernel_dsos.head, node) { >> - if (is_kernel_module(dso->long_name)) >> + /* >> + * cpumode passed to is_kernel_module is not the >> + * cpumode of *this* event. If we insist on passing >> + * correct cpumode to is_kernel_module, we should record >> + * the cpumode when we adding this dso to the linked >> list. >> + * >> + * However we don't really need passing correct cpumode. >> + * We know the correct cpumode must be kernel mode >> + * (if not, we should not link it onto kernel_dsos >> list). >> + * >> + * Therefore, we pass PERF_RECORD_MISC_CPUMODE_UNKNOWN. >> + * is_kernel_module() treat it as a kernel cpumode. >> + */ >> + if (is_kernel_module(dso->long_name, >> + PERF_RECORD_MISC_CPUMODE_UNKNOWN)) >> continue; >> >> kernel = dso; >> > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to [email protected] > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ >
-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
