From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754150AbdFWOpb (ORCPT ); Fri, 23 Jun 2017 10:45:31 -0400 Received: from mail.kernel.org ([198.145.29.99]:45788 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751999AbdFWOpa (ORCPT ); Fri, 23 Jun 2017 10:45:30 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org E72642170D Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=acme@kernel.org Date: Fri, 23 Jun 2017 11:45:24 -0300 From: Arnaldo Carvalho de Melo To: Namhyung Kim Cc: Ingo Molnar , Peter Zijlstra , Jiri Olsa , LKML , kernel-team@lge.com, Masami Hiramatsu , Andi Kleen , Adrian Hunter , Wang Nan Subject: Re: [PATCH/RFC 9/9] perf record: Add --module-dir option Message-ID: <20170623144524.GE4622@kernel.org> References: <20170623054827.3828-1-namhyung@kernel.org> <20170623054827.3828-10-namhyung@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170623054827.3828-10-namhyung@kernel.org> X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.8.0 (2017-02-23) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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