linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 0/4] perf probe: Introduce remote cross-arch probes
@ 2016-08-24  5:57 Masami Hiramatsu
  2016-08-24  5:57 ` [RFC PATCH 1/4] perf-probe: Remove unused tracing_dir variable Masami Hiramatsu
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Masami Hiramatsu @ 2016-08-24  5:57 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Masami Hiramatsu, linux-kernel, Peter Zijlstra, Ingo Molnar, Jiri Olsa

Hi,

Here is an RFC series for remote cross-arch probe support on perf-probe.

I've made a perf-probe for remote arch (currently arm on x86-64) for
helping debugging and performance analysis.

Currently perf-probe doesn't supoort cross/remote target. This means
we have to cross-build the perf-tools including libraries (elfutils,
libelf etc.), and to prepare vmlinux with debuginfo which can be 
accessed from the target machine.
This requires too much resource for a small embededd device.

If we can analyze the debuginfo by perf-probe on host machine,
we do not need to cross-build perf-tools, nor copy vmlinux on the
device. :)

This series introduces such features on perf-probe.

To use this, on host machine (with cross build kernel image),
below command outputs the target-machine's kprobe-events definition
in <output-directory>/kprobe_events.

  perf probe -k <cross-vmlinux> --outdir=<output-directory> \
                <probe-point> <arguments>

Perf analyzes the given vmlinux and get the architecture and
switch the dwarf-register mappings.

Here is an example:
  -----
  $ mkdir tracing
  $ sudo perf probe --outdir=./tracing --vmlinux=./vmlinux-arm \
    do_sys_open '$vars'
  Added new event:
  probe:do_sys_open    (on do_sys_open with $vars)

  You can now use it in all perf tools, such as:

        perf record -e probe:do_sys_open -aR sleep 1

  $ cat tracing/kprobe_events
  p:probe/do_sys_open _text+1282292 dfd=%r5:s32 filename=%r1:u32
   flags=%r6:s32 mode=%r3:u16 op=-60(%sp) fd=%r4:s32 tmp=%r7:u32
  -----
Here, we can get probe/do_sys_open event by "copy & paste" the
definition to target-machine's debugfs/tracing/kprobe_events.

To make sure it is correct:
  -----
  $ nm vmlinux-arm | grep "T _text"
  80008000 T _text
  $ nm vmlinux-arm | grep "T do_sys_open"
  801410f4 T do_sys_open
  $ expr `printf "%d + %d" 0x80008000 1282292`
  2148798708
  $ printf "%x\n" 2148798708
  801410f4
  -----
So "_text+12882292" indicates do_sys_open on the target binary correctly.

Thanks,
---

Masami Hiramatsu (4):
      perf-probe: Remove unused tracing_dir variable
      perf-probe: Add offline output directory option
      perf-probe: Ignore vmlinux buildid if offline kernel is given
      perf-probe: Support probing on offline cross-arch binary


 tools/perf/arch/arm/include/dwarf-regs-table.h     |    9 +++
 tools/perf/arch/arm64/include/dwarf-regs-table.h   |   13 +++++
 tools/perf/arch/powerpc/include/dwarf-regs-table.h |   27 ++++++++++
 tools/perf/arch/s390/include/dwarf-regs-table.h    |    8 +++
 tools/perf/arch/sh/include/dwarf-regs-table.h      |   25 +++++++++
 tools/perf/arch/sparc/include/dwarf-regs-table.h   |   18 +++++++
 tools/perf/arch/x86/include/dwarf-regs-table.h     |   14 +++++
 tools/perf/arch/xtensa/include/dwarf-regs-table.h  |    8 +++
 tools/perf/builtin-probe.c                         |    8 +++
 tools/perf/util/Build                              |    1 
 tools/perf/util/dwarf-regs.c                       |   55 ++++++++++++++++++++
 tools/perf/util/include/dwarf-regs.h               |    6 ++
 tools/perf/util/probe-event.h                      |    1 
 tools/perf/util/probe-file.c                       |   22 ++++++--
 tools/perf/util/probe-finder.c                     |   27 ++++++----
 tools/perf/util/probe-finder.h                     |    1 
 tools/perf/util/symbol-elf.c                       |    2 -
 17 files changed, 228 insertions(+), 17 deletions(-)
 create mode 100644 tools/perf/arch/arm/include/dwarf-regs-table.h
 create mode 100644 tools/perf/arch/arm64/include/dwarf-regs-table.h
 create mode 100644 tools/perf/arch/powerpc/include/dwarf-regs-table.h
 create mode 100644 tools/perf/arch/s390/include/dwarf-regs-table.h
 create mode 100644 tools/perf/arch/sh/include/dwarf-regs-table.h
 create mode 100644 tools/perf/arch/sparc/include/dwarf-regs-table.h
 create mode 100644 tools/perf/arch/x86/include/dwarf-regs-table.h
 create mode 100644 tools/perf/arch/xtensa/include/dwarf-regs-table.h
 create mode 100644 tools/perf/util/dwarf-regs.c

--
Masami Hiramatsu (Linaro Ltd.) <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFC PATCH 1/4] perf-probe: Remove unused tracing_dir variable
  2016-08-24  5:57 [RFC PATCH 0/4] perf probe: Introduce remote cross-arch probes Masami Hiramatsu
@ 2016-08-24  5:57 ` Masami Hiramatsu
  2016-09-05 13:19   ` [tip:perf/core] perf probe: " tip-bot for Masami Hiramatsu
  2016-08-24  5:58 ` [RFC PATCH 2/4] perf-probe: Add offline output directory option Masami Hiramatsu
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Masami Hiramatsu @ 2016-08-24  5:57 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Masami Hiramatsu, linux-kernel, Peter Zijlstra, Ingo Molnar, Jiri Olsa

Remove unused tracing_dir variable from open_probe_events().

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 tools/perf/util/probe-file.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index 697ef66..6f931e4 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -73,11 +73,10 @@ static void print_both_open_warning(int kerr, int uerr)
 static int open_probe_events(const char *trace_file, bool readwrite)
 {
 	char buf[PATH_MAX];
-	const char *tracing_dir = "";
 	int ret;
 
-	ret = e_snprintf(buf, PATH_MAX, "%s/%s%s",
-			 tracing_path, tracing_dir, trace_file);
+	ret = e_snprintf(buf, PATH_MAX, "%s/%s",
+			 tracing_path, trace_file);
 	if (ret >= 0) {
 		pr_debug("Opening %s write=%d\n", buf, readwrite);
 		if (readwrite && !probe_event_dry_run)

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RFC PATCH 2/4] perf-probe: Add offline output directory option
  2016-08-24  5:57 [RFC PATCH 0/4] perf probe: Introduce remote cross-arch probes Masami Hiramatsu
  2016-08-24  5:57 ` [RFC PATCH 1/4] perf-probe: Remove unused tracing_dir variable Masami Hiramatsu
@ 2016-08-24  5:58 ` Masami Hiramatsu
  2016-08-24 12:58   ` Arnaldo Carvalho de Melo
  2016-08-24  5:58 ` [RFC PATCH 3/4] perf-probe: Ignore vmlinux buildid if offline kernel is given Masami Hiramatsu
  2016-08-24  5:58 ` [RFC PATCH 4/4] perf-probe: Support probing on offline cross-arch binary Masami Hiramatsu
  3 siblings, 1 reply; 12+ messages in thread
From: Masami Hiramatsu @ 2016-08-24  5:58 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Masami Hiramatsu, linux-kernel, Peter Zijlstra, Ingo Molnar, Jiri Olsa

Add offline output direcrtory option. This allows user to
store probe event definition in offline output directory.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 tools/perf/builtin-probe.c    |    2 ++
 tools/perf/util/probe-event.h |    1 +
 tools/perf/util/probe-file.c  |   19 ++++++++++++++++---
 3 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index ee5b421..5b45ec8 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -517,6 +517,8 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 		"Show variables location range in scope (with --vars only)"),
 	OPT_STRING('k', "vmlinux", &symbol_conf.vmlinux_name,
 		   "file", "vmlinux pathname"),
+	OPT_STRING(0, "outdir", &probe_conf.output_dir,
+		"directory", "path to offline output directory"),
 	OPT_STRING('s', "source", &symbol_conf.source_prefix,
 		   "directory", "path to kernel source"),
 	OPT_BOOLEAN('\0', "no-inlines", &probe_conf.no_inlines,
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index f4f45db..75b572a 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -14,6 +14,7 @@ struct probe_conf {
 	bool	no_inlines;
 	bool	cache;
 	int	max_probes;
+	const char	*output_dir;
 };
 extern struct probe_conf probe_conf;
 extern bool probe_event_dry_run;
diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index 6f931e4..41db430 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -73,16 +73,25 @@ static void print_both_open_warning(int kerr, int uerr)
 static int open_probe_events(const char *trace_file, bool readwrite)
 {
 	char buf[PATH_MAX];
+	const char *tracing_dir = tracing_path;
+	int oflag = 0;
+	mode_t mode = 0;
 	int ret;
 
+	if (probe_conf.output_dir) {
+		tracing_dir = probe_conf.output_dir;
+		oflag = O_CREAT;
+		mode = 0644;
+	}
+
 	ret = e_snprintf(buf, PATH_MAX, "%s/%s",
-			 tracing_path, trace_file);
+			 tracing_dir, trace_file);
 	if (ret >= 0) {
 		pr_debug("Opening %s write=%d\n", buf, readwrite);
 		if (readwrite && !probe_event_dry_run)
-			ret = open(buf, O_RDWR | O_APPEND, 0);
+			ret = open(buf, O_RDWR | O_APPEND | oflag, mode);
 		else
-			ret = open(buf, O_RDONLY, 0);
+			ret = open(buf, O_RDONLY | oflag, mode);
 
 		if (ret < 0)
 			ret = -errno;
@@ -242,6 +251,8 @@ int probe_file__add_event(int fd, struct probe_trace_event *tev)
 			pr_warning("Failed to write event: %s\n",
 				   str_error_r(errno, sbuf, sizeof(sbuf)));
 		}
+		if (probe_conf.output_dir)
+			ret = write(fd, "\n", 1) == 1 ? 0 : -errno;
 	}
 	free(buf);
 
@@ -274,6 +285,8 @@ static int __del_trace_probe_event(int fd, struct str_node *ent)
 		ret = -errno;
 		goto error;
 	}
+	if (probe_conf.output_dir)
+		ret = write(fd, "\n", 1) == 1 ? 0 : -errno;
 
 	return 0;
 error:

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RFC PATCH 3/4] perf-probe: Ignore vmlinux buildid if offline kernel is given
  2016-08-24  5:57 [RFC PATCH 0/4] perf probe: Introduce remote cross-arch probes Masami Hiramatsu
  2016-08-24  5:57 ` [RFC PATCH 1/4] perf-probe: Remove unused tracing_dir variable Masami Hiramatsu
  2016-08-24  5:58 ` [RFC PATCH 2/4] perf-probe: Add offline output directory option Masami Hiramatsu
@ 2016-08-24  5:58 ` Masami Hiramatsu
  2016-08-24  5:58 ` [RFC PATCH 4/4] perf-probe: Support probing on offline cross-arch binary Masami Hiramatsu
  3 siblings, 0 replies; 12+ messages in thread
From: Masami Hiramatsu @ 2016-08-24  5:58 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Masami Hiramatsu, linux-kernel, Peter Zijlstra, Ingo Molnar, Jiri Olsa

Ignore the buildid of running kernel when both --outdir and
--vmlinux is given and --outdir is not tracing_path,
because that kernel should be off-line.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 tools/perf/builtin-probe.c   |    6 ++++++
 tools/perf/util/symbol-elf.c |    2 +-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 5b45ec8..773cea0 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -602,6 +602,12 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 	 */
 	symbol_conf.try_vmlinux_path = (symbol_conf.vmlinux_name == NULL);
 
+	/* If user gives output directory and offline vmlinux, ignore buildid */
+	if (probe_conf.output_dir && symbol_conf.vmlinux_name &&
+	    strcmp(probe_conf.output_dir, tracing_path) != 0) {
+		symbol_conf.ignore_vmlinux_buildid = true;
+	}
+
 	switch (params.command) {
 	case 'l':
 		if (params.uprobes) {
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index a811c13..013cebf 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -685,7 +685,7 @@ int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
 	}
 
 	/* Always reject images with a mismatched build-id: */
-	if (dso->has_build_id) {
+	if (dso->has_build_id && !symbol_conf.ignore_vmlinux_buildid) {
 		u8 build_id[BUILD_ID_SIZE];
 
 		if (elf_read_build_id(elf, build_id, BUILD_ID_SIZE) < 0) {

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RFC PATCH 4/4] perf-probe: Support probing on offline cross-arch binary
  2016-08-24  5:57 [RFC PATCH 0/4] perf probe: Introduce remote cross-arch probes Masami Hiramatsu
                   ` (2 preceding siblings ...)
  2016-08-24  5:58 ` [RFC PATCH 3/4] perf-probe: Ignore vmlinux buildid if offline kernel is given Masami Hiramatsu
@ 2016-08-24  5:58 ` Masami Hiramatsu
  2016-08-24 13:03   ` Arnaldo Carvalho de Melo
  3 siblings, 1 reply; 12+ messages in thread
From: Masami Hiramatsu @ 2016-08-24  5:58 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Masami Hiramatsu, linux-kernel, Peter Zijlstra, Ingo Molnar, Jiri Olsa

Support probing on offline cross-architecture binary by
adding getting the target machine arch from ELF and
choose correct register string for the machine.

Here is an example:
  -----
  $ mkdir tracing
  $ sudo perf probe --outdir=./tracing --vmlinux=./vmlinux-arm \
    do_sys_open '$vars'
  Added new event:
  probe:do_sys_open    (on do_sys_open with $vars)

  You can now use it in all perf tools, such as:

  	perf record -e probe:do_sys_open -aR sleep 1

  $ cat tracing/kprobe_events
  p:probe/do_sys_open _text+1282292 dfd=%r5:s32 filename=%r1:u32
   flags=%r6:s32 mode=%r3:u16 op=-60(%sp) fd=%r4:s32 tmp=%r7:u32
  -----
Here, we can get probe/do_sys_open event by "copy & paste" from
./tracing/kprobe_events to target-machine's debugfs/tracing/kprobe_events

To make sure it is correct:
  -----
  $ nm vmlinux-arm | grep "T _text"
  80008000 T _text
  $ nm vmlinux-arm | grep "T do_sys_open"
  801410f4 T do_sys_open
  $ expr `printf "%d + %d" 0x80008000 1282292`
  2148798708
  $ printf "%x\n" 2148798708
  801410f4
  -----
So "_text+12882292" indicates do_sys_open on the target binary correctly.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 tools/perf/arch/arm/include/dwarf-regs-table.h     |    9 +++
 tools/perf/arch/arm64/include/dwarf-regs-table.h   |   13 +++++
 tools/perf/arch/powerpc/include/dwarf-regs-table.h |   27 ++++++++++
 tools/perf/arch/s390/include/dwarf-regs-table.h    |    8 +++
 tools/perf/arch/sh/include/dwarf-regs-table.h      |   25 +++++++++
 tools/perf/arch/sparc/include/dwarf-regs-table.h   |   18 +++++++
 tools/perf/arch/x86/include/dwarf-regs-table.h     |   14 +++++
 tools/perf/arch/xtensa/include/dwarf-regs-table.h  |    8 +++
 tools/perf/util/Build                              |    1 
 tools/perf/util/dwarf-regs.c                       |   55 ++++++++++++++++++++
 tools/perf/util/include/dwarf-regs.h               |    6 ++
 tools/perf/util/probe-finder.c                     |   27 ++++++----
 tools/perf/util/probe-finder.h                     |    1 
 13 files changed, 201 insertions(+), 11 deletions(-)
 create mode 100644 tools/perf/arch/arm/include/dwarf-regs-table.h
 create mode 100644 tools/perf/arch/arm64/include/dwarf-regs-table.h
 create mode 100644 tools/perf/arch/powerpc/include/dwarf-regs-table.h
 create mode 100644 tools/perf/arch/s390/include/dwarf-regs-table.h
 create mode 100644 tools/perf/arch/sh/include/dwarf-regs-table.h
 create mode 100644 tools/perf/arch/sparc/include/dwarf-regs-table.h
 create mode 100644 tools/perf/arch/x86/include/dwarf-regs-table.h
 create mode 100644 tools/perf/arch/xtensa/include/dwarf-regs-table.h
 create mode 100644 tools/perf/util/dwarf-regs.c

diff --git a/tools/perf/arch/arm/include/dwarf-regs-table.h b/tools/perf/arch/arm/include/dwarf-regs-table.h
new file mode 100644
index 0000000..f298d03
--- /dev/null
+++ b/tools/perf/arch/arm/include/dwarf-regs-table.h
@@ -0,0 +1,9 @@
+#ifdef DEFINE_DWARF_REGSTR_TABLE
+/* This is included in perf/util/dwarf-regs.c */
+
+static const char * const arm_regstr_tbl[] = {
+	"%r0", "%r1", "%r2", "%r3", "%r4",
+	"%r5", "%r6", "%r7", "%r8", "%r9", "%r10",
+	"%fp", "%ip", "%sp", "%lr", "%pc",
+};
+#endif
diff --git a/tools/perf/arch/arm64/include/dwarf-regs-table.h b/tools/perf/arch/arm64/include/dwarf-regs-table.h
new file mode 100644
index 0000000..2675936
--- /dev/null
+++ b/tools/perf/arch/arm64/include/dwarf-regs-table.h
@@ -0,0 +1,13 @@
+#ifdef DEFINE_DWARF_REGSTR_TABLE
+/* This is included in perf/util/dwarf-regs.c */
+
+static const char * const aarch64_regstr_tbl[] = {
+	"%r0", "%r1", "%r2", "%r3", "%r4",
+	"%r5", "%r6", "%r7", "%r8", "%r9",
+	"%r10", "%r11", "%r12", "%r13", "%r14",
+	"%r15", "%r16", "%r17", "%r18", "%r19",
+	"%r20", "%r21", "%r22", "%r23", "%r24",
+	"%r25", "%r26", "%r27", "%r28", "%r29",
+	"%lr", "%sp",
+};
+#endif
diff --git a/tools/perf/arch/powerpc/include/dwarf-regs-table.h b/tools/perf/arch/powerpc/include/dwarf-regs-table.h
new file mode 100644
index 0000000..db4730f
--- /dev/null
+++ b/tools/perf/arch/powerpc/include/dwarf-regs-table.h
@@ -0,0 +1,27 @@
+#ifdef DEFINE_DWARF_REGSTR_TABLE
+/* This is included in perf/util/dwarf-regs.c */
+
+/*
+ * Reference:
+ * http://refspecs.linuxfoundation.org/ELF/ppc64/PPC-elf64abi-1.9.html
+ * http://refspecs.linux-foundation.org/elf/elfspec_ppc.pdf
+ */
+#define REG_DWARFNUM_NAME(reg, idx)	[idx] = "%" #reg
+
+static const char * const powerpc_regstr_tbl[] = {
+	"%gpr0", "%gpr1", "%gpr2", "%gpr3", "%gpr4",
+	"%gpr5", "%gpr6", "%gpr7", "%gpr8", "%gpr9",
+	"%gpr10", "%gpr11", "%gpr12", "%gpr13", "%gpr14",
+	"%gpr15", "%gpr16", "%gpr17", "%gpr18", "%gpr19",
+	"%gpr20", "%gpr21", "%gpr22", "%gpr23", "%gpr24",
+	"%gpr25", "%gpr26", "%gpr27", "%gpr28", "%gpr29",
+	"%gpr30", "%gpr31",
+	REG_DWARFNUM_NAME(msr,   66),
+	REG_DWARFNUM_NAME(ctr,   109),
+	REG_DWARFNUM_NAME(link,  108),
+	REG_DWARFNUM_NAME(xer,   101),
+	REG_DWARFNUM_NAME(dar,   119),
+	REG_DWARFNUM_NAME(dsisr, 118),
+};
+
+#endif
diff --git a/tools/perf/arch/s390/include/dwarf-regs-table.h b/tools/perf/arch/s390/include/dwarf-regs-table.h
new file mode 100644
index 0000000..9da74a9
--- /dev/null
+++ b/tools/perf/arch/s390/include/dwarf-regs-table.h
@@ -0,0 +1,8 @@
+#ifdef DEFINE_DWARF_REGSTR_TABLE
+/* This is included in perf/util/dwarf-regs.c */
+
+static const char * const s390_regstr_tbl[] = {
+	"%r0", "%r1",  "%r2",  "%r3",  "%r4",  "%r5",  "%r6",  "%r7",
+	"%r8", "%r9", "%r10", "%r11", "%r12", "%r13", "%r14", "%r15",
+};
+#endif
diff --git a/tools/perf/arch/sh/include/dwarf-regs-table.h b/tools/perf/arch/sh/include/dwarf-regs-table.h
new file mode 100644
index 0000000..3a2deaf
--- /dev/null
+++ b/tools/perf/arch/sh/include/dwarf-regs-table.h
@@ -0,0 +1,25 @@
+#ifdef DEFINE_DWARF_REGSTR_TABLE
+/* This is included in perf/util/dwarf-regs.c */
+
+const char * const sh_regstr_tbl[] = {
+	"r0",
+	"r1",
+	"r2",
+	"r3",
+	"r4",
+	"r5",
+	"r6",
+	"r7",
+	"r8",
+	"r9",
+	"r10",
+	"r11",
+	"r12",
+	"r13",
+	"r14",
+	"r15",
+	"pc",
+	"pr",
+};
+
+#endif
diff --git a/tools/perf/arch/sparc/include/dwarf-regs-table.h b/tools/perf/arch/sparc/include/dwarf-regs-table.h
new file mode 100644
index 0000000..12c0761
--- /dev/null
+++ b/tools/perf/arch/sparc/include/dwarf-regs-table.h
@@ -0,0 +1,18 @@
+#ifdef DEFINE_DWARF_REGSTR_TABLE
+/* This is included in perf/util/dwarf-regs.c */
+
+static const char * const sparc_regstr_tbl[] = {
+	"%g0", "%g1", "%g2", "%g3", "%g4", "%g5", "%g6", "%g7",
+	"%o0", "%o1", "%o2", "%o3", "%o4", "%o5", "%sp", "%o7",
+	"%l0", "%l1", "%l2", "%l3", "%l4", "%l5", "%l6", "%l7",
+	"%i0", "%i1", "%i2", "%i3", "%i4", "%i5", "%fp", "%i7",
+	"%f0", "%f1", "%f2", "%f3", "%f4", "%f5", "%f6", "%f7",
+	"%f8", "%f9", "%f10", "%f11", "%f12", "%f13", "%f14", "%f15",
+	"%f16", "%f17", "%f18", "%f19", "%f20", "%f21", "%f22", "%f23",
+	"%f24", "%f25", "%f26", "%f27", "%f28", "%f29", "%f30", "%f31",
+	"%f32", "%f33", "%f34", "%f35", "%f36", "%f37", "%f38", "%f39",
+	"%f40", "%f41", "%f42", "%f43", "%f44", "%f45", "%f46", "%f47",
+	"%f48", "%f49", "%f50", "%f51", "%f52", "%f53", "%f54", "%f55",
+	"%f56", "%f57", "%f58", "%f59", "%f60", "%f61", "%f62", "%f63",
+};
+#endif
diff --git a/tools/perf/arch/x86/include/dwarf-regs-table.h b/tools/perf/arch/x86/include/dwarf-regs-table.h
new file mode 100644
index 0000000..39ac7cb
--- /dev/null
+++ b/tools/perf/arch/x86/include/dwarf-regs-table.h
@@ -0,0 +1,14 @@
+#ifdef DEFINE_DWARF_REGSTR_TABLE
+/* This is included in perf/util/dwarf-regs.c */
+
+static const char * const x86_32_regstr_tbl[] = {
+	"%ax", "%cx", "%dx", "%bx", "$stack",/* Stack address instead of %sp */
+	"%bp", "%si", "%di",
+};
+
+static const char * const x86_64_regstr_tbl[] = {
+	"%ax", "dx", "%cx", "%bx", "%si", "%di",
+	"%bp", "%sp", "%r8", "%r9", "%r10", "%r11",
+	"%r12", "%r13", "%r14", "%r15",
+};
+#endif
diff --git a/tools/perf/arch/xtensa/include/dwarf-regs-table.h b/tools/perf/arch/xtensa/include/dwarf-regs-table.h
new file mode 100644
index 0000000..aa0444a
--- /dev/null
+++ b/tools/perf/arch/xtensa/include/dwarf-regs-table.h
@@ -0,0 +1,8 @@
+#ifdef DEFINE_DWARF_REGSTR_TABLE
+/* This is included in perf/util/dwarf-regs.c */
+
+static const char * const xtensa_regstr_tbl[] = {
+	"a0", "a1", "a2", "a3", "a4", "a5", "a6", "a7",
+	"a8", "a9", "a10", "a11", "a12", "a13", "a14", "a15",
+};
+#endif
diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 91c5f6e..f1a6d17 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -98,6 +98,7 @@ endif
 
 libperf-$(CONFIG_DWARF) += probe-finder.o
 libperf-$(CONFIG_DWARF) += dwarf-aux.o
+libperf-$(CONFIG_DWARF) += dwarf-regs.o
 
 libperf-$(CONFIG_LIBDW_DWARF_UNWIND) += unwind-libdw.o
 libperf-$(CONFIG_LOCAL_LIBUNWIND)    += unwind-libunwind-local.o
diff --git a/tools/perf/util/dwarf-regs.c b/tools/perf/util/dwarf-regs.c
new file mode 100644
index 0000000..ce15816
--- /dev/null
+++ b/tools/perf/util/dwarf-regs.c
@@ -0,0 +1,55 @@
+/*
+ * dwarf-regs.c : Mapping of DWARF debug register numbers into register names.
+ *
+ * Written by: Masami Hiramatsu <mhiramat@kernel.org>
+ */
+
+#include <util.h>
+#include <debug.h>
+#include <dwarf-regs.h>
+#include <elf.h>
+
+/* Define const char * {arch}_register_tbl[] */
+#define DEFINE_DWARF_REGSTR_TABLE
+#include "../arch/x86/include/dwarf-regs-table.h"
+#include "../arch/arm/include/dwarf-regs-table.h"
+#include "../arch/arm64/include/dwarf-regs-table.h"
+#include "../arch/sh/include/dwarf-regs-table.h"
+#include "../arch/powerpc/include/dwarf-regs-table.h"
+#include "../arch/s390/include/dwarf-regs-table.h"
+#include "../arch/sparc/include/dwarf-regs-table.h"
+#include "../arch/xtensa/include/dwarf-regs-table.h"
+
+#define __get_dwarf_regstr(tbl, n) (((n) < ARRAY_SIZE(tbl)) ? (tbl)[(n)] : NULL)
+
+/* Return architecture dependent register string (for kprobe-tracer) */
+const char *get_dwarf_regstr(unsigned int n, unsigned int machine)
+{
+	switch (machine) {
+	case EM_NONE:	/* Generic arch - use host arch */
+		return get_arch_regstr(n);
+	case EM_386:
+		return __get_dwarf_regstr(x86_32_regstr_tbl, n);
+	case EM_X86_64:
+		return __get_dwarf_regstr(x86_64_regstr_tbl, n);
+	case EM_ARM:
+		return __get_dwarf_regstr(arm_regstr_tbl, n);
+	case EM_AARCH64:
+		return __get_dwarf_regstr(aarch64_regstr_tbl, n);
+	case EM_SH:
+		return __get_dwarf_regstr(sh_regstr_tbl, n);
+	case EM_S390:
+		return __get_dwarf_regstr(s390_regstr_tbl, n);
+	case EM_PPC:
+	case EM_PPC64:
+		return __get_dwarf_regstr(powerpc_regstr_tbl, n);
+	case EM_SPARC:
+	case EM_SPARCV9:
+		return __get_dwarf_regstr(sparc_regstr_tbl, n);
+	case EM_XTENSA:
+		return __get_dwarf_regstr(xtensa_regstr_tbl, n);
+	default:
+		pr_err("ELF MACHINE %x is not supported.\n", machine);
+	}
+	return NULL;
+}
diff --git a/tools/perf/util/include/dwarf-regs.h b/tools/perf/util/include/dwarf-regs.h
index 07c644e..43bfd8d 100644
--- a/tools/perf/util/include/dwarf-regs.h
+++ b/tools/perf/util/include/dwarf-regs.h
@@ -3,6 +3,12 @@
 
 #ifdef HAVE_DWARF_SUPPORT
 const char *get_arch_regstr(unsigned int n);
+/*
+ * get_dwarf_regstr - Returns ftrace register string from DWARF regnum
+ * n: DWARF register number
+ * machine: ELF machine signature (EM_*)
+ */
+const char *get_dwarf_regstr(unsigned int n, unsigned int machine);
 #endif
 
 #ifdef HAVE_ARCH_REGS_QUERY_REGISTER_OFFSET
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index ac4740f..508b61c 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -171,6 +171,7 @@ static struct probe_trace_arg_ref *alloc_trace_arg_ref(long offs)
  */
 static int convert_variable_location(Dwarf_Die *vr_die, Dwarf_Addr addr,
 				     Dwarf_Op *fb_ops, Dwarf_Die *sp_die,
+				     unsigned int machine,
 				     struct probe_trace_arg *tvar)
 {
 	Dwarf_Attribute attr;
@@ -266,7 +267,7 @@ static_var:
 	if (!tvar)
 		return ret2;
 
-	regs = get_arch_regstr(regn);
+	regs = get_dwarf_regstr(regn, machine);
 	if (!regs) {
 		/* This should be a bug in DWARF or this tool */
 		pr_warning("Mapping for the register number %u "
@@ -543,7 +544,7 @@ static int convert_variable(Dwarf_Die *vr_die, struct probe_finder *pf)
 		 dwarf_diename(vr_die));
 
 	ret = convert_variable_location(vr_die, pf->addr, pf->fb_ops,
-					&pf->sp_die, pf->tvar);
+					&pf->sp_die, pf->machine, pf->tvar);
 	if (ret == -ENOENT || ret == -EINVAL) {
 		pr_err("Failed to find the location of the '%s' variable at this address.\n"
 		       " Perhaps it has been optimized out.\n"
@@ -1106,11 +1107,8 @@ static int debuginfo__find_probes(struct debuginfo *dbg,
 				  struct probe_finder *pf)
 {
 	int ret = 0;
-
-#if _ELFUTILS_PREREQ(0, 142)
 	Elf *elf;
 	GElf_Ehdr ehdr;
-	GElf_Shdr shdr;
 
 	if (pf->cfi_eh || pf->cfi_dbg)
 		return debuginfo__find_probe_location(dbg, pf);
@@ -1123,11 +1121,18 @@ static int debuginfo__find_probes(struct debuginfo *dbg,
 	if (gelf_getehdr(elf, &ehdr) == NULL)
 		return -EINVAL;
 
-	if (elf_section_by_name(elf, &ehdr, &shdr, ".eh_frame", NULL) &&
-	    shdr.sh_type == SHT_PROGBITS)
-		pf->cfi_eh = dwarf_getcfi_elf(elf);
+	pf->machine = ehdr.e_machine;
+
+#if _ELFUTILS_PREREQ(0, 142)
+	do {
+		GElf_Shdr shdr;
+
+		if (elf_section_by_name(elf, &ehdr, &shdr, ".eh_frame", NULL) &&
+		    shdr.sh_type == SHT_PROGBITS)
+			pf->cfi_eh = dwarf_getcfi_elf(elf);
 
-	pf->cfi_dbg = dwarf_getcfi(dbg->dbg);
+		pf->cfi_dbg = dwarf_getcfi(dbg->dbg);
+	} while (0);
 #endif
 
 	ret = debuginfo__find_probe_location(dbg, pf);
@@ -1155,7 +1160,7 @@ static int copy_variables_cb(Dwarf_Die *die_mem, void *data)
 	    (tag == DW_TAG_variable && vf->vars)) {
 		if (convert_variable_location(die_mem, vf->pf->addr,
 					      vf->pf->fb_ops, &pf->sp_die,
-					      NULL) == 0) {
+					      pf->machine, NULL) == 0) {
 			vf->args[vf->nargs].var = (char *)dwarf_diename(die_mem);
 			if (vf->args[vf->nargs].var == NULL) {
 				vf->ret = -ENOMEM;
@@ -1318,7 +1323,7 @@ static int collect_variables_cb(Dwarf_Die *die_mem, void *data)
 	    tag == DW_TAG_variable) {
 		ret = convert_variable_location(die_mem, af->pf.addr,
 						af->pf.fb_ops, &af->pf.sp_die,
-						NULL);
+						af->pf.machine, NULL);
 		if (ret == 0 || ret == -ERANGE) {
 			int ret2;
 			bool externs = !af->child;
diff --git a/tools/perf/util/probe-finder.h b/tools/perf/util/probe-finder.h
index 51137fc..f1d8558 100644
--- a/tools/perf/util/probe-finder.h
+++ b/tools/perf/util/probe-finder.h
@@ -80,6 +80,7 @@ struct probe_finder {
 	Dwarf_CFI		*cfi_dbg;
 #endif
 	Dwarf_Op		*fb_ops;	/* Frame base attribute */
+	unsigned int		machine;	/* Target machine arch */
 	struct perf_probe_arg	*pvar;		/* Current target variable */
 	struct probe_trace_arg	*tvar;		/* Current result variable */
 };

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH 2/4] perf-probe: Add offline output directory option
  2016-08-24  5:58 ` [RFC PATCH 2/4] perf-probe: Add offline output directory option Masami Hiramatsu
@ 2016-08-24 12:58   ` Arnaldo Carvalho de Melo
  2016-08-25  6:37     ` Masami Hiramatsu
  2016-08-25 10:48     ` Masami Hiramatsu
  0 siblings, 2 replies; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-08-24 12:58 UTC (permalink / raw)
  To: Masami Hiramatsu; +Cc: linux-kernel, Peter Zijlstra, Ingo Molnar, Jiri Olsa

Em Wed, Aug 24, 2016 at 02:58:12PM +0900, Masami Hiramatsu escreveu:
> Add offline output direcrtory option. This allows user to
> store probe event definition in offline output directory.

In such cases you should show it in action, like I am doing now to test
this patch:

  [root@jouet ~]# perf probe --outdir=my_probes do_open
  kprobe_events file does not exist - please rebuild kernel with CONFIG_KPROBE_EVENTS.
    Error: Failed to add events.
  [root@jouet ~]# 

Oops, misleading error message, probably I need to first do a mkdir:

  [root@jouet ~]# mkdir my_probes
  [root@jouet ~]# perf probe --outdir=my_probes do_open
  Added new event:
    probe:do_open        (on do_open)

  You can now use it in all perf tools, such as:

	  perf record -e probe:do_open -aR sleep 1

  [root@jouet ~]# ls -la my_probes
  total 12
  drwxr-xr-x.  2 root root 4096 Aug 24 09:47 .
  dr-xr-x---. 31 root root 4096 Aug 24 09:47 ..
  -rw-r--r--.  1 root root   30 Aug 24 09:47 kprobe_events
  [root@jouet ~]# cat my_probes/kprobe_events 
  p:probe/do_open _text+3481584
  [root@jouet ~]# 

Wouldn't it be better to call it some other name and imply --dry-run? "outdir"
seems too vague. Perhaps --definition and instead have it run like:

  # perf probe --definition do_open
  p:probe/do_open _text+3481584
  #

Then one could even use it to redirect it to a file, tee + file and even test it as:

  # perf probe --definition do_open > /sys/kernel/debug/tracing/kprobe_events
  #

For debugging:

  # perf probe --definition do_open | tee /sys/kernel/debug/tracing/kprobe_events

:-)

I picked "--kprobes" first, but that would left out e.g. uprobes, then I
thought about --event, but also sounds vague, read
Documentation/trace/kprobetrace.txt and saw this bit:

 -------
Usage examples
--------------
To add a probe as a new event, write a new definition to kprobe_events
as below.

  echo 'p:myprobe do_sys_open dfd=%ax filename=%dx flags=%cx mode=+4($stack)' > /sys/kernel/debug/tracing/kprobe_events
 -------

So you used "definition" for that string, might as well be consistent and use
it here as well.

Also please fix the OPT_STRING string, it should start with a capital
letter:

        --max-probes <n>  Set how many probe points can be found for a probe.
        --no-inlines      Don't search inlined functions
        --outdir <directory>
                          path to offline output directory
        --range           Show variables location range in scope (with --vars only)


See how it stands out? All the others start with a capital letter.

I applied the first patch, totally trivial.

- Arnaldo
 
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
>  tools/perf/builtin-probe.c    |    2 ++
>  tools/perf/util/probe-event.h |    1 +
>  tools/perf/util/probe-file.c  |   19 ++++++++++++++++---
>  3 files changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
> index ee5b421..5b45ec8 100644
> --- a/tools/perf/builtin-probe.c
> +++ b/tools/perf/builtin-probe.c
> @@ -517,6 +517,8 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
>  		"Show variables location range in scope (with --vars only)"),
>  	OPT_STRING('k', "vmlinux", &symbol_conf.vmlinux_name,
>  		   "file", "vmlinux pathname"),
> +	OPT_STRING(0, "outdir", &probe_conf.output_dir,
> +		"directory", "path to offline output directory"),
>  	OPT_STRING('s', "source", &symbol_conf.source_prefix,
>  		   "directory", "path to kernel source"),
>  	OPT_BOOLEAN('\0', "no-inlines", &probe_conf.no_inlines,
> diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
> index f4f45db..75b572a 100644
> --- a/tools/perf/util/probe-event.h
> +++ b/tools/perf/util/probe-event.h
> @@ -14,6 +14,7 @@ struct probe_conf {
>  	bool	no_inlines;
>  	bool	cache;
>  	int	max_probes;
> +	const char	*output_dir;
>  };
>  extern struct probe_conf probe_conf;
>  extern bool probe_event_dry_run;
> diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
> index 6f931e4..41db430 100644
> --- a/tools/perf/util/probe-file.c
> +++ b/tools/perf/util/probe-file.c
> @@ -73,16 +73,25 @@ static void print_both_open_warning(int kerr, int uerr)
>  static int open_probe_events(const char *trace_file, bool readwrite)
>  {
>  	char buf[PATH_MAX];
> +	const char *tracing_dir = tracing_path;
> +	int oflag = 0;
> +	mode_t mode = 0;
>  	int ret;
>  
> +	if (probe_conf.output_dir) {
> +		tracing_dir = probe_conf.output_dir;
> +		oflag = O_CREAT;
> +		mode = 0644;
> +	}
> +
>  	ret = e_snprintf(buf, PATH_MAX, "%s/%s",
> -			 tracing_path, trace_file);
> +			 tracing_dir, trace_file);
>  	if (ret >= 0) {
>  		pr_debug("Opening %s write=%d\n", buf, readwrite);
>  		if (readwrite && !probe_event_dry_run)
> -			ret = open(buf, O_RDWR | O_APPEND, 0);
> +			ret = open(buf, O_RDWR | O_APPEND | oflag, mode);
>  		else
> -			ret = open(buf, O_RDONLY, 0);
> +			ret = open(buf, O_RDONLY | oflag, mode);
>  
>  		if (ret < 0)
>  			ret = -errno;
> @@ -242,6 +251,8 @@ int probe_file__add_event(int fd, struct probe_trace_event *tev)
>  			pr_warning("Failed to write event: %s\n",
>  				   str_error_r(errno, sbuf, sizeof(sbuf)));
>  		}
> +		if (probe_conf.output_dir)
> +			ret = write(fd, "\n", 1) == 1 ? 0 : -errno;
>  	}
>  	free(buf);
>  
> @@ -274,6 +285,8 @@ static int __del_trace_probe_event(int fd, struct str_node *ent)
>  		ret = -errno;
>  		goto error;
>  	}
> +	if (probe_conf.output_dir)
> +		ret = write(fd, "\n", 1) == 1 ? 0 : -errno;
>  
>  	return 0;
>  error:

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH 4/4] perf-probe: Support probing on offline cross-arch binary
  2016-08-24  5:58 ` [RFC PATCH 4/4] perf-probe: Support probing on offline cross-arch binary Masami Hiramatsu
@ 2016-08-24 13:03   ` Arnaldo Carvalho de Melo
  2016-08-25  6:57     ` Masami Hiramatsu
  0 siblings, 1 reply; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-08-24 13:03 UTC (permalink / raw)
  To: Masami Hiramatsu; +Cc: linux-kernel, Peter Zijlstra, Ingo Molnar, Jiri Olsa

Em Wed, Aug 24, 2016 at 02:58:41PM +0900, Masami Hiramatsu escreveu:
> Support probing on offline cross-architecture binary by
> adding getting the target machine arch from ELF and
> choose correct register string for the machine.
> 
> Here is an example:
>   -----
>   $ mkdir tracing

No need for the sudo here, right?

>   $ sudo perf probe --outdir=./tracing --vmlinux=./vmlinux-arm \
>     do_sys_open '$vars'

Going with the --definition (or any other name that spits out what needs
to go to kprobes_events, this could be further short circuited, ie.:

  $ perf probe --vmlinux=./vmlinux-arm do_sys_open '$vars'
  p:probe/do_sys_open _text+1282292 dfd=%r5:s32 filename=%r1:u32 flags=%r6:s32 mode=%r3:u16 op=-60(%sp) fd=%r4:s32 tmp=%r7:u32
  $

Because in this case clearly the probe is not for the running kernel,
thus we could imply that this is to generate probes for running on
some other place.

Also perhaps even this could be made shorter:

  $ perf probe vmlinux-arm do_sys_open '$vars'
  p:probe/do_sys_open _text+1282292 dfd=%r5:s32 filename=%r1:u32 flags=%r6:s32 mode=%r3:u16 op=-60(%sp) fd=%r4:s32 tmp=%r7:u32
  $

As it can easily figure out the first argument _is_ a vmlinux :-)

- Arnaldo
        

>   Added new event:
>   probe:do_sys_open    (on do_sys_open with $vars)
> 
>   You can now use it in all perf tools, such as:
> 
>   	perf record -e probe:do_sys_open -aR sleep 1
> 
>   $ cat tracing/kprobe_events
>   p:probe/do_sys_open _text+1282292 dfd=%r5:s32 filename=%r1:u32
>    flags=%r6:s32 mode=%r3:u16 op=-60(%sp) fd=%r4:s32 tmp=%r7:u32
>   -----
> Here, we can get probe/do_sys_open event by "copy & paste" from
> ./tracing/kprobe_events to target-machine's debugfs/tracing/kprobe_events
> 
> To make sure it is correct:
>   -----
>   $ nm vmlinux-arm | grep "T _text"
>   80008000 T _text
>   $ nm vmlinux-arm | grep "T do_sys_open"
>   801410f4 T do_sys_open
>   $ expr `printf "%d + %d" 0x80008000 1282292`
>   2148798708
>   $ printf "%x\n" 2148798708
>   801410f4
>   -----
> So "_text+12882292" indicates do_sys_open on the target binary correctly.
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
>  tools/perf/arch/arm/include/dwarf-regs-table.h     |    9 +++
>  tools/perf/arch/arm64/include/dwarf-regs-table.h   |   13 +++++
>  tools/perf/arch/powerpc/include/dwarf-regs-table.h |   27 ++++++++++
>  tools/perf/arch/s390/include/dwarf-regs-table.h    |    8 +++
>  tools/perf/arch/sh/include/dwarf-regs-table.h      |   25 +++++++++
>  tools/perf/arch/sparc/include/dwarf-regs-table.h   |   18 +++++++
>  tools/perf/arch/x86/include/dwarf-regs-table.h     |   14 +++++
>  tools/perf/arch/xtensa/include/dwarf-regs-table.h  |    8 +++
>  tools/perf/util/Build                              |    1 
>  tools/perf/util/dwarf-regs.c                       |   55 ++++++++++++++++++++
>  tools/perf/util/include/dwarf-regs.h               |    6 ++
>  tools/perf/util/probe-finder.c                     |   27 ++++++----
>  tools/perf/util/probe-finder.h                     |    1 
>  13 files changed, 201 insertions(+), 11 deletions(-)
>  create mode 100644 tools/perf/arch/arm/include/dwarf-regs-table.h
>  create mode 100644 tools/perf/arch/arm64/include/dwarf-regs-table.h
>  create mode 100644 tools/perf/arch/powerpc/include/dwarf-regs-table.h
>  create mode 100644 tools/perf/arch/s390/include/dwarf-regs-table.h
>  create mode 100644 tools/perf/arch/sh/include/dwarf-regs-table.h
>  create mode 100644 tools/perf/arch/sparc/include/dwarf-regs-table.h
>  create mode 100644 tools/perf/arch/x86/include/dwarf-regs-table.h
>  create mode 100644 tools/perf/arch/xtensa/include/dwarf-regs-table.h
>  create mode 100644 tools/perf/util/dwarf-regs.c
> 
> diff --git a/tools/perf/arch/arm/include/dwarf-regs-table.h b/tools/perf/arch/arm/include/dwarf-regs-table.h
> new file mode 100644
> index 0000000..f298d03
> --- /dev/null
> +++ b/tools/perf/arch/arm/include/dwarf-regs-table.h
> @@ -0,0 +1,9 @@
> +#ifdef DEFINE_DWARF_REGSTR_TABLE
> +/* This is included in perf/util/dwarf-regs.c */
> +
> +static const char * const arm_regstr_tbl[] = {
> +	"%r0", "%r1", "%r2", "%r3", "%r4",
> +	"%r5", "%r6", "%r7", "%r8", "%r9", "%r10",
> +	"%fp", "%ip", "%sp", "%lr", "%pc",
> +};
> +#endif
> diff --git a/tools/perf/arch/arm64/include/dwarf-regs-table.h b/tools/perf/arch/arm64/include/dwarf-regs-table.h
> new file mode 100644
> index 0000000..2675936
> --- /dev/null
> +++ b/tools/perf/arch/arm64/include/dwarf-regs-table.h
> @@ -0,0 +1,13 @@
> +#ifdef DEFINE_DWARF_REGSTR_TABLE
> +/* This is included in perf/util/dwarf-regs.c */
> +
> +static const char * const aarch64_regstr_tbl[] = {
> +	"%r0", "%r1", "%r2", "%r3", "%r4",
> +	"%r5", "%r6", "%r7", "%r8", "%r9",
> +	"%r10", "%r11", "%r12", "%r13", "%r14",
> +	"%r15", "%r16", "%r17", "%r18", "%r19",
> +	"%r20", "%r21", "%r22", "%r23", "%r24",
> +	"%r25", "%r26", "%r27", "%r28", "%r29",
> +	"%lr", "%sp",
> +};
> +#endif
> diff --git a/tools/perf/arch/powerpc/include/dwarf-regs-table.h b/tools/perf/arch/powerpc/include/dwarf-regs-table.h
> new file mode 100644
> index 0000000..db4730f
> --- /dev/null
> +++ b/tools/perf/arch/powerpc/include/dwarf-regs-table.h
> @@ -0,0 +1,27 @@
> +#ifdef DEFINE_DWARF_REGSTR_TABLE
> +/* This is included in perf/util/dwarf-regs.c */
> +
> +/*
> + * Reference:
> + * http://refspecs.linuxfoundation.org/ELF/ppc64/PPC-elf64abi-1.9.html
> + * http://refspecs.linux-foundation.org/elf/elfspec_ppc.pdf
> + */
> +#define REG_DWARFNUM_NAME(reg, idx)	[idx] = "%" #reg
> +
> +static const char * const powerpc_regstr_tbl[] = {
> +	"%gpr0", "%gpr1", "%gpr2", "%gpr3", "%gpr4",
> +	"%gpr5", "%gpr6", "%gpr7", "%gpr8", "%gpr9",
> +	"%gpr10", "%gpr11", "%gpr12", "%gpr13", "%gpr14",
> +	"%gpr15", "%gpr16", "%gpr17", "%gpr18", "%gpr19",
> +	"%gpr20", "%gpr21", "%gpr22", "%gpr23", "%gpr24",
> +	"%gpr25", "%gpr26", "%gpr27", "%gpr28", "%gpr29",
> +	"%gpr30", "%gpr31",
> +	REG_DWARFNUM_NAME(msr,   66),
> +	REG_DWARFNUM_NAME(ctr,   109),
> +	REG_DWARFNUM_NAME(link,  108),
> +	REG_DWARFNUM_NAME(xer,   101),
> +	REG_DWARFNUM_NAME(dar,   119),
> +	REG_DWARFNUM_NAME(dsisr, 118),
> +};
> +
> +#endif
> diff --git a/tools/perf/arch/s390/include/dwarf-regs-table.h b/tools/perf/arch/s390/include/dwarf-regs-table.h
> new file mode 100644
> index 0000000..9da74a9
> --- /dev/null
> +++ b/tools/perf/arch/s390/include/dwarf-regs-table.h
> @@ -0,0 +1,8 @@
> +#ifdef DEFINE_DWARF_REGSTR_TABLE
> +/* This is included in perf/util/dwarf-regs.c */
> +
> +static const char * const s390_regstr_tbl[] = {
> +	"%r0", "%r1",  "%r2",  "%r3",  "%r4",  "%r5",  "%r6",  "%r7",
> +	"%r8", "%r9", "%r10", "%r11", "%r12", "%r13", "%r14", "%r15",
> +};
> +#endif
> diff --git a/tools/perf/arch/sh/include/dwarf-regs-table.h b/tools/perf/arch/sh/include/dwarf-regs-table.h
> new file mode 100644
> index 0000000..3a2deaf
> --- /dev/null
> +++ b/tools/perf/arch/sh/include/dwarf-regs-table.h
> @@ -0,0 +1,25 @@
> +#ifdef DEFINE_DWARF_REGSTR_TABLE
> +/* This is included in perf/util/dwarf-regs.c */
> +
> +const char * const sh_regstr_tbl[] = {
> +	"r0",
> +	"r1",
> +	"r2",
> +	"r3",
> +	"r4",
> +	"r5",
> +	"r6",
> +	"r7",
> +	"r8",
> +	"r9",
> +	"r10",
> +	"r11",
> +	"r12",
> +	"r13",
> +	"r14",
> +	"r15",
> +	"pc",
> +	"pr",
> +};
> +
> +#endif
> diff --git a/tools/perf/arch/sparc/include/dwarf-regs-table.h b/tools/perf/arch/sparc/include/dwarf-regs-table.h
> new file mode 100644
> index 0000000..12c0761
> --- /dev/null
> +++ b/tools/perf/arch/sparc/include/dwarf-regs-table.h
> @@ -0,0 +1,18 @@
> +#ifdef DEFINE_DWARF_REGSTR_TABLE
> +/* This is included in perf/util/dwarf-regs.c */
> +
> +static const char * const sparc_regstr_tbl[] = {
> +	"%g0", "%g1", "%g2", "%g3", "%g4", "%g5", "%g6", "%g7",
> +	"%o0", "%o1", "%o2", "%o3", "%o4", "%o5", "%sp", "%o7",
> +	"%l0", "%l1", "%l2", "%l3", "%l4", "%l5", "%l6", "%l7",
> +	"%i0", "%i1", "%i2", "%i3", "%i4", "%i5", "%fp", "%i7",
> +	"%f0", "%f1", "%f2", "%f3", "%f4", "%f5", "%f6", "%f7",
> +	"%f8", "%f9", "%f10", "%f11", "%f12", "%f13", "%f14", "%f15",
> +	"%f16", "%f17", "%f18", "%f19", "%f20", "%f21", "%f22", "%f23",
> +	"%f24", "%f25", "%f26", "%f27", "%f28", "%f29", "%f30", "%f31",
> +	"%f32", "%f33", "%f34", "%f35", "%f36", "%f37", "%f38", "%f39",
> +	"%f40", "%f41", "%f42", "%f43", "%f44", "%f45", "%f46", "%f47",
> +	"%f48", "%f49", "%f50", "%f51", "%f52", "%f53", "%f54", "%f55",
> +	"%f56", "%f57", "%f58", "%f59", "%f60", "%f61", "%f62", "%f63",
> +};
> +#endif
> diff --git a/tools/perf/arch/x86/include/dwarf-regs-table.h b/tools/perf/arch/x86/include/dwarf-regs-table.h
> new file mode 100644
> index 0000000..39ac7cb
> --- /dev/null
> +++ b/tools/perf/arch/x86/include/dwarf-regs-table.h
> @@ -0,0 +1,14 @@
> +#ifdef DEFINE_DWARF_REGSTR_TABLE
> +/* This is included in perf/util/dwarf-regs.c */
> +
> +static const char * const x86_32_regstr_tbl[] = {
> +	"%ax", "%cx", "%dx", "%bx", "$stack",/* Stack address instead of %sp */
> +	"%bp", "%si", "%di",
> +};
> +
> +static const char * const x86_64_regstr_tbl[] = {
> +	"%ax", "dx", "%cx", "%bx", "%si", "%di",
> +	"%bp", "%sp", "%r8", "%r9", "%r10", "%r11",
> +	"%r12", "%r13", "%r14", "%r15",
> +};
> +#endif
> diff --git a/tools/perf/arch/xtensa/include/dwarf-regs-table.h b/tools/perf/arch/xtensa/include/dwarf-regs-table.h
> new file mode 100644
> index 0000000..aa0444a
> --- /dev/null
> +++ b/tools/perf/arch/xtensa/include/dwarf-regs-table.h
> @@ -0,0 +1,8 @@
> +#ifdef DEFINE_DWARF_REGSTR_TABLE
> +/* This is included in perf/util/dwarf-regs.c */
> +
> +static const char * const xtensa_regstr_tbl[] = {
> +	"a0", "a1", "a2", "a3", "a4", "a5", "a6", "a7",
> +	"a8", "a9", "a10", "a11", "a12", "a13", "a14", "a15",
> +};
> +#endif
> diff --git a/tools/perf/util/Build b/tools/perf/util/Build
> index 91c5f6e..f1a6d17 100644
> --- a/tools/perf/util/Build
> +++ b/tools/perf/util/Build
> @@ -98,6 +98,7 @@ endif
>  
>  libperf-$(CONFIG_DWARF) += probe-finder.o
>  libperf-$(CONFIG_DWARF) += dwarf-aux.o
> +libperf-$(CONFIG_DWARF) += dwarf-regs.o
>  
>  libperf-$(CONFIG_LIBDW_DWARF_UNWIND) += unwind-libdw.o
>  libperf-$(CONFIG_LOCAL_LIBUNWIND)    += unwind-libunwind-local.o
> diff --git a/tools/perf/util/dwarf-regs.c b/tools/perf/util/dwarf-regs.c
> new file mode 100644
> index 0000000..ce15816
> --- /dev/null
> +++ b/tools/perf/util/dwarf-regs.c
> @@ -0,0 +1,55 @@
> +/*
> + * dwarf-regs.c : Mapping of DWARF debug register numbers into register names.
> + *
> + * Written by: Masami Hiramatsu <mhiramat@kernel.org>
> + */
> +
> +#include <util.h>
> +#include <debug.h>
> +#include <dwarf-regs.h>
> +#include <elf.h>
> +
> +/* Define const char * {arch}_register_tbl[] */
> +#define DEFINE_DWARF_REGSTR_TABLE
> +#include "../arch/x86/include/dwarf-regs-table.h"
> +#include "../arch/arm/include/dwarf-regs-table.h"
> +#include "../arch/arm64/include/dwarf-regs-table.h"
> +#include "../arch/sh/include/dwarf-regs-table.h"
> +#include "../arch/powerpc/include/dwarf-regs-table.h"
> +#include "../arch/s390/include/dwarf-regs-table.h"
> +#include "../arch/sparc/include/dwarf-regs-table.h"
> +#include "../arch/xtensa/include/dwarf-regs-table.h"
> +
> +#define __get_dwarf_regstr(tbl, n) (((n) < ARRAY_SIZE(tbl)) ? (tbl)[(n)] : NULL)
> +
> +/* Return architecture dependent register string (for kprobe-tracer) */
> +const char *get_dwarf_regstr(unsigned int n, unsigned int machine)
> +{
> +	switch (machine) {
> +	case EM_NONE:	/* Generic arch - use host arch */
> +		return get_arch_regstr(n);
> +	case EM_386:
> +		return __get_dwarf_regstr(x86_32_regstr_tbl, n);
> +	case EM_X86_64:
> +		return __get_dwarf_regstr(x86_64_regstr_tbl, n);
> +	case EM_ARM:
> +		return __get_dwarf_regstr(arm_regstr_tbl, n);
> +	case EM_AARCH64:
> +		return __get_dwarf_regstr(aarch64_regstr_tbl, n);
> +	case EM_SH:
> +		return __get_dwarf_regstr(sh_regstr_tbl, n);
> +	case EM_S390:
> +		return __get_dwarf_regstr(s390_regstr_tbl, n);
> +	case EM_PPC:
> +	case EM_PPC64:
> +		return __get_dwarf_regstr(powerpc_regstr_tbl, n);
> +	case EM_SPARC:
> +	case EM_SPARCV9:
> +		return __get_dwarf_regstr(sparc_regstr_tbl, n);
> +	case EM_XTENSA:
> +		return __get_dwarf_regstr(xtensa_regstr_tbl, n);
> +	default:
> +		pr_err("ELF MACHINE %x is not supported.\n", machine);
> +	}
> +	return NULL;
> +}
> diff --git a/tools/perf/util/include/dwarf-regs.h b/tools/perf/util/include/dwarf-regs.h
> index 07c644e..43bfd8d 100644
> --- a/tools/perf/util/include/dwarf-regs.h
> +++ b/tools/perf/util/include/dwarf-regs.h
> @@ -3,6 +3,12 @@
>  
>  #ifdef HAVE_DWARF_SUPPORT
>  const char *get_arch_regstr(unsigned int n);
> +/*
> + * get_dwarf_regstr - Returns ftrace register string from DWARF regnum
> + * n: DWARF register number
> + * machine: ELF machine signature (EM_*)
> + */
> +const char *get_dwarf_regstr(unsigned int n, unsigned int machine);
>  #endif
>  
>  #ifdef HAVE_ARCH_REGS_QUERY_REGISTER_OFFSET
> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index ac4740f..508b61c 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -171,6 +171,7 @@ static struct probe_trace_arg_ref *alloc_trace_arg_ref(long offs)
>   */
>  static int convert_variable_location(Dwarf_Die *vr_die, Dwarf_Addr addr,
>  				     Dwarf_Op *fb_ops, Dwarf_Die *sp_die,
> +				     unsigned int machine,
>  				     struct probe_trace_arg *tvar)
>  {
>  	Dwarf_Attribute attr;
> @@ -266,7 +267,7 @@ static_var:
>  	if (!tvar)
>  		return ret2;
>  
> -	regs = get_arch_regstr(regn);
> +	regs = get_dwarf_regstr(regn, machine);
>  	if (!regs) {
>  		/* This should be a bug in DWARF or this tool */
>  		pr_warning("Mapping for the register number %u "
> @@ -543,7 +544,7 @@ static int convert_variable(Dwarf_Die *vr_die, struct probe_finder *pf)
>  		 dwarf_diename(vr_die));
>  
>  	ret = convert_variable_location(vr_die, pf->addr, pf->fb_ops,
> -					&pf->sp_die, pf->tvar);
> +					&pf->sp_die, pf->machine, pf->tvar);
>  	if (ret == -ENOENT || ret == -EINVAL) {
>  		pr_err("Failed to find the location of the '%s' variable at this address.\n"
>  		       " Perhaps it has been optimized out.\n"
> @@ -1106,11 +1107,8 @@ static int debuginfo__find_probes(struct debuginfo *dbg,
>  				  struct probe_finder *pf)
>  {
>  	int ret = 0;
> -
> -#if _ELFUTILS_PREREQ(0, 142)
>  	Elf *elf;
>  	GElf_Ehdr ehdr;
> -	GElf_Shdr shdr;
>  
>  	if (pf->cfi_eh || pf->cfi_dbg)
>  		return debuginfo__find_probe_location(dbg, pf);
> @@ -1123,11 +1121,18 @@ static int debuginfo__find_probes(struct debuginfo *dbg,
>  	if (gelf_getehdr(elf, &ehdr) == NULL)
>  		return -EINVAL;
>  
> -	if (elf_section_by_name(elf, &ehdr, &shdr, ".eh_frame", NULL) &&
> -	    shdr.sh_type == SHT_PROGBITS)
> -		pf->cfi_eh = dwarf_getcfi_elf(elf);
> +	pf->machine = ehdr.e_machine;
> +
> +#if _ELFUTILS_PREREQ(0, 142)
> +	do {
> +		GElf_Shdr shdr;
> +
> +		if (elf_section_by_name(elf, &ehdr, &shdr, ".eh_frame", NULL) &&
> +		    shdr.sh_type == SHT_PROGBITS)
> +			pf->cfi_eh = dwarf_getcfi_elf(elf);
>  
> -	pf->cfi_dbg = dwarf_getcfi(dbg->dbg);
> +		pf->cfi_dbg = dwarf_getcfi(dbg->dbg);
> +	} while (0);
>  #endif
>  
>  	ret = debuginfo__find_probe_location(dbg, pf);
> @@ -1155,7 +1160,7 @@ static int copy_variables_cb(Dwarf_Die *die_mem, void *data)
>  	    (tag == DW_TAG_variable && vf->vars)) {
>  		if (convert_variable_location(die_mem, vf->pf->addr,
>  					      vf->pf->fb_ops, &pf->sp_die,
> -					      NULL) == 0) {
> +					      pf->machine, NULL) == 0) {
>  			vf->args[vf->nargs].var = (char *)dwarf_diename(die_mem);
>  			if (vf->args[vf->nargs].var == NULL) {
>  				vf->ret = -ENOMEM;
> @@ -1318,7 +1323,7 @@ static int collect_variables_cb(Dwarf_Die *die_mem, void *data)
>  	    tag == DW_TAG_variable) {
>  		ret = convert_variable_location(die_mem, af->pf.addr,
>  						af->pf.fb_ops, &af->pf.sp_die,
> -						NULL);
> +						af->pf.machine, NULL);
>  		if (ret == 0 || ret == -ERANGE) {
>  			int ret2;
>  			bool externs = !af->child;
> diff --git a/tools/perf/util/probe-finder.h b/tools/perf/util/probe-finder.h
> index 51137fc..f1d8558 100644
> --- a/tools/perf/util/probe-finder.h
> +++ b/tools/perf/util/probe-finder.h
> @@ -80,6 +80,7 @@ struct probe_finder {
>  	Dwarf_CFI		*cfi_dbg;
>  #endif
>  	Dwarf_Op		*fb_ops;	/* Frame base attribute */
> +	unsigned int		machine;	/* Target machine arch */
>  	struct perf_probe_arg	*pvar;		/* Current target variable */
>  	struct probe_trace_arg	*tvar;		/* Current result variable */
>  };

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH 2/4] perf-probe: Add offline output directory option
  2016-08-24 12:58   ` Arnaldo Carvalho de Melo
@ 2016-08-25  6:37     ` Masami Hiramatsu
  2016-08-25 10:48     ` Masami Hiramatsu
  1 sibling, 0 replies; 12+ messages in thread
From: Masami Hiramatsu @ 2016-08-25  6:37 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Peter Zijlstra, Ingo Molnar, Jiri Olsa

On Wed, 24 Aug 2016 09:58:45 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Em Wed, Aug 24, 2016 at 02:58:12PM +0900, Masami Hiramatsu escreveu:
> > Add offline output direcrtory option. This allows user to
> > store probe event definition in offline output directory.
> 
> In such cases you should show it in action, like I am doing now to test
> this patch:

OK, I'll add it.

> 
>   [root@jouet ~]# perf probe --outdir=my_probes do_open
>   kprobe_events file does not exist - please rebuild kernel with CONFIG_KPROBE_EVENTS.
>     Error: Failed to add events.
>   [root@jouet ~]# 
> 
> Oops, misleading error message, probably I need to first do a mkdir:
> 
>   [root@jouet ~]# mkdir my_probes
>   [root@jouet ~]# perf probe --outdir=my_probes do_open
>   Added new event:
>     probe:do_open        (on do_open)
> 
>   You can now use it in all perf tools, such as:
> 
> 	  perf record -e probe:do_open -aR sleep 1
> 
>   [root@jouet ~]# ls -la my_probes
>   total 12
>   drwxr-xr-x.  2 root root 4096 Aug 24 09:47 .
>   dr-xr-x---. 31 root root 4096 Aug 24 09:47 ..
>   -rw-r--r--.  1 root root   30 Aug 24 09:47 kprobe_events
>   [root@jouet ~]# cat my_probes/kprobe_events 
>   p:probe/do_open _text+3481584
>   [root@jouet ~]# 
> 
> Wouldn't it be better to call it some other name and imply --dry-run? "outdir"
> seems too vague. Perhaps --definition and instead have it run like:
> 
>   # perf probe --definition do_open
>   p:probe/do_open _text+3481584
>   #

Yeah, it looks good especially we don't have to think about --del or
--list with that...

> 
> Then one could even use it to redirect it to a file, tee + file and even test it as:
> 
>   # perf probe --definition do_open > /sys/kernel/debug/tracing/kprobe_events
>   #
> 
> For debugging:
> 
>   # perf probe --definition do_open | tee /sys/kernel/debug/tracing/kprobe_events
> 
> :-)
> 

Yeah, it also makes use perf-probe in shell script easier.


> I picked "--kprobes" first, but that would left out e.g. uprobes, then I
> thought about --event, but also sounds vague, read
> Documentation/trace/kprobetrace.txt and saw this bit:
> 
>  -------
> Usage examples
> --------------
> To add a probe as a new event, write a new definition to kprobe_events
> as below.
> 
>   echo 'p:myprobe do_sys_open dfd=%ax filename=%dx flags=%cx mode=+4($stack)' > /sys/kernel/debug/tracing/kprobe_events
>  -------
> 
> So you used "definition" for that string, might as well be consistent and use
> it here as well.

OK, I couldn't get better name. --definition or -D will be good. 

 
> Also please fix the OPT_STRING string, it should start with a capital
> letter:

Ah, I see :)

> 
>         --max-probes <n>  Set how many probe points can be found for a probe.
>         --no-inlines      Don't search inlined functions
>         --outdir <directory>
>                           path to offline output directory
>         --range           Show variables location range in scope (with --vars only)
> 
> 
> See how it stands out? All the others start with a capital letter.
> 
> I applied the first patch, totally trivial.

Thanks!

> 
> - Arnaldo
>  
> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > ---
> >  tools/perf/builtin-probe.c    |    2 ++
> >  tools/perf/util/probe-event.h |    1 +
> >  tools/perf/util/probe-file.c  |   19 ++++++++++++++++---
> >  3 files changed, 19 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
> > index ee5b421..5b45ec8 100644
> > --- a/tools/perf/builtin-probe.c
> > +++ b/tools/perf/builtin-probe.c
> > @@ -517,6 +517,8 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
> >  		"Show variables location range in scope (with --vars only)"),
> >  	OPT_STRING('k', "vmlinux", &symbol_conf.vmlinux_name,
> >  		   "file", "vmlinux pathname"),
> > +	OPT_STRING(0, "outdir", &probe_conf.output_dir,
> > +		"directory", "path to offline output directory"),
> >  	OPT_STRING('s', "source", &symbol_conf.source_prefix,
> >  		   "directory", "path to kernel source"),
> >  	OPT_BOOLEAN('\0', "no-inlines", &probe_conf.no_inlines,
> > diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
> > index f4f45db..75b572a 100644
> > --- a/tools/perf/util/probe-event.h
> > +++ b/tools/perf/util/probe-event.h
> > @@ -14,6 +14,7 @@ struct probe_conf {
> >  	bool	no_inlines;
> >  	bool	cache;
> >  	int	max_probes;
> > +	const char	*output_dir;
> >  };
> >  extern struct probe_conf probe_conf;
> >  extern bool probe_event_dry_run;
> > diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
> > index 6f931e4..41db430 100644
> > --- a/tools/perf/util/probe-file.c
> > +++ b/tools/perf/util/probe-file.c
> > @@ -73,16 +73,25 @@ static void print_both_open_warning(int kerr, int uerr)
> >  static int open_probe_events(const char *trace_file, bool readwrite)
> >  {
> >  	char buf[PATH_MAX];
> > +	const char *tracing_dir = tracing_path;
> > +	int oflag = 0;
> > +	mode_t mode = 0;
> >  	int ret;
> >  
> > +	if (probe_conf.output_dir) {
> > +		tracing_dir = probe_conf.output_dir;
> > +		oflag = O_CREAT;
> > +		mode = 0644;
> > +	}
> > +
> >  	ret = e_snprintf(buf, PATH_MAX, "%s/%s",
> > -			 tracing_path, trace_file);
> > +			 tracing_dir, trace_file);
> >  	if (ret >= 0) {
> >  		pr_debug("Opening %s write=%d\n", buf, readwrite);
> >  		if (readwrite && !probe_event_dry_run)
> > -			ret = open(buf, O_RDWR | O_APPEND, 0);
> > +			ret = open(buf, O_RDWR | O_APPEND | oflag, mode);
> >  		else
> > -			ret = open(buf, O_RDONLY, 0);
> > +			ret = open(buf, O_RDONLY | oflag, mode);
> >  
> >  		if (ret < 0)
> >  			ret = -errno;
> > @@ -242,6 +251,8 @@ int probe_file__add_event(int fd, struct probe_trace_event *tev)
> >  			pr_warning("Failed to write event: %s\n",
> >  				   str_error_r(errno, sbuf, sizeof(sbuf)));
> >  		}
> > +		if (probe_conf.output_dir)
> > +			ret = write(fd, "\n", 1) == 1 ? 0 : -errno;
> >  	}
> >  	free(buf);
> >  
> > @@ -274,6 +285,8 @@ static int __del_trace_probe_event(int fd, struct str_node *ent)
> >  		ret = -errno;
> >  		goto error;
> >  	}
> > +	if (probe_conf.output_dir)
> > +		ret = write(fd, "\n", 1) == 1 ? 0 : -errno;
> >  
> >  	return 0;
> >  error:


-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH 4/4] perf-probe: Support probing on offline cross-arch binary
  2016-08-24 13:03   ` Arnaldo Carvalho de Melo
@ 2016-08-25  6:57     ` Masami Hiramatsu
  0 siblings, 0 replies; 12+ messages in thread
From: Masami Hiramatsu @ 2016-08-25  6:57 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Peter Zijlstra, Ingo Molnar, Jiri Olsa

On Wed, 24 Aug 2016 10:03:08 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Em Wed, Aug 24, 2016 at 02:58:41PM +0900, Masami Hiramatsu escreveu:
> > Support probing on offline cross-architecture binary by
> > adding getting the target machine arch from ELF and
> > choose correct register string for the machine.
> > 
> > Here is an example:
> >   -----
> >   $ mkdir tracing
> 
> No need for the sudo here, right?
> 
> >   $ sudo perf probe --outdir=./tracing --vmlinux=./vmlinux-arm \
> >     do_sys_open '$vars'

Ah, actually, there might be something wrong processing path around
kernel_get_ref_reloc_sym(), it calls map__load() to load symbols in the
kernel, but it actually loads /proc/kallsyms, and if kptr_restrict is
enabled fails to find reloc_sym, because all addresses are 0.
I think it should not load the kallsyms if symbol_conf.vmlinux_name is
given. But I'm not sure how.
(Moreover, it is very strange why we could get the correct ref_reloc_sym
address(_text) from kallsyms, I guess map__load() may load both of the
kallsyms and given vmlinux)
BTW, actually, the dso already loaded given vmlinux, so I would like
to know how to load a map from dso.

> Going with the --definition (or any other name that spits out what needs
> to go to kprobes_events, this could be further short circuited, ie.:
> 
>   $ perf probe --vmlinux=./vmlinux-arm do_sys_open '$vars'
>   p:probe/do_sys_open _text+1282292 dfd=%r5:s32 filename=%r1:u32 flags=%r6:s32 mode=%r3:u16 op=-60(%sp) fd=%r4:s32 tmp=%r7:u32
>   $
> 
> Because in this case clearly the probe is not for the running kernel,
> thus we could imply that this is to generate probes for running on
> some other place.

Hmm, but if user gives a file what he think it is running kernel vmlinux to
--vmlinux, he might be upset. I would like to see the Buildid error in that case.

If he gives --definition, we may be sure that user is done with his intention.

Thanks,

> Also perhaps even this could be made shorter:
> 
>   $ perf probe vmlinux-arm do_sys_open '$vars'
>   p:probe/do_sys_open _text+1282292 dfd=%r5:s32 filename=%r1:u32 flags=%r6:s32 mode=%r3:u16 op=-60(%sp) fd=%r4:s32 tmp=%r7:u32
>   $
> 
> As it can easily figure out the first argument _is_ a vmlinux :-)
> 
> - Arnaldo
>         
> 
> >   Added new event:
> >   probe:do_sys_open    (on do_sys_open with $vars)
> > 
> >   You can now use it in all perf tools, such as:
> > 
> >   	perf record -e probe:do_sys_open -aR sleep 1
> > 
> >   $ cat tracing/kprobe_events
> >   p:probe/do_sys_open _text+1282292 dfd=%r5:s32 filename=%r1:u32
> >    flags=%r6:s32 mode=%r3:u16 op=-60(%sp) fd=%r4:s32 tmp=%r7:u32
> >   -----
> > Here, we can get probe/do_sys_open event by "copy & paste" from
> > ./tracing/kprobe_events to target-machine's debugfs/tracing/kprobe_events
> > 
> > To make sure it is correct:
> >   -----
> >   $ nm vmlinux-arm | grep "T _text"
> >   80008000 T _text
> >   $ nm vmlinux-arm | grep "T do_sys_open"
> >   801410f4 T do_sys_open
> >   $ expr `printf "%d + %d" 0x80008000 1282292`
> >   2148798708
> >   $ printf "%x\n" 2148798708
> >   801410f4
> >   -----
> > So "_text+12882292" indicates do_sys_open on the target binary correctly.
> > 
> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > ---
> >  tools/perf/arch/arm/include/dwarf-regs-table.h     |    9 +++
> >  tools/perf/arch/arm64/include/dwarf-regs-table.h   |   13 +++++
> >  tools/perf/arch/powerpc/include/dwarf-regs-table.h |   27 ++++++++++
> >  tools/perf/arch/s390/include/dwarf-regs-table.h    |    8 +++
> >  tools/perf/arch/sh/include/dwarf-regs-table.h      |   25 +++++++++
> >  tools/perf/arch/sparc/include/dwarf-regs-table.h   |   18 +++++++
> >  tools/perf/arch/x86/include/dwarf-regs-table.h     |   14 +++++
> >  tools/perf/arch/xtensa/include/dwarf-regs-table.h  |    8 +++
> >  tools/perf/util/Build                              |    1 
> >  tools/perf/util/dwarf-regs.c                       |   55 ++++++++++++++++++++
> >  tools/perf/util/include/dwarf-regs.h               |    6 ++
> >  tools/perf/util/probe-finder.c                     |   27 ++++++----
> >  tools/perf/util/probe-finder.h                     |    1 
> >  13 files changed, 201 insertions(+), 11 deletions(-)
> >  create mode 100644 tools/perf/arch/arm/include/dwarf-regs-table.h
> >  create mode 100644 tools/perf/arch/arm64/include/dwarf-regs-table.h
> >  create mode 100644 tools/perf/arch/powerpc/include/dwarf-regs-table.h
> >  create mode 100644 tools/perf/arch/s390/include/dwarf-regs-table.h
> >  create mode 100644 tools/perf/arch/sh/include/dwarf-regs-table.h
> >  create mode 100644 tools/perf/arch/sparc/include/dwarf-regs-table.h
> >  create mode 100644 tools/perf/arch/x86/include/dwarf-regs-table.h
> >  create mode 100644 tools/perf/arch/xtensa/include/dwarf-regs-table.h
> >  create mode 100644 tools/perf/util/dwarf-regs.c
> > 
> > diff --git a/tools/perf/arch/arm/include/dwarf-regs-table.h b/tools/perf/arch/arm/include/dwarf-regs-table.h
> > new file mode 100644
> > index 0000000..f298d03
> > --- /dev/null
> > +++ b/tools/perf/arch/arm/include/dwarf-regs-table.h
> > @@ -0,0 +1,9 @@
> > +#ifdef DEFINE_DWARF_REGSTR_TABLE
> > +/* This is included in perf/util/dwarf-regs.c */
> > +
> > +static const char * const arm_regstr_tbl[] = {
> > +	"%r0", "%r1", "%r2", "%r3", "%r4",
> > +	"%r5", "%r6", "%r7", "%r8", "%r9", "%r10",
> > +	"%fp", "%ip", "%sp", "%lr", "%pc",
> > +};
> > +#endif
> > diff --git a/tools/perf/arch/arm64/include/dwarf-regs-table.h b/tools/perf/arch/arm64/include/dwarf-regs-table.h
> > new file mode 100644
> > index 0000000..2675936
> > --- /dev/null
> > +++ b/tools/perf/arch/arm64/include/dwarf-regs-table.h
> > @@ -0,0 +1,13 @@
> > +#ifdef DEFINE_DWARF_REGSTR_TABLE
> > +/* This is included in perf/util/dwarf-regs.c */
> > +
> > +static const char * const aarch64_regstr_tbl[] = {
> > +	"%r0", "%r1", "%r2", "%r3", "%r4",
> > +	"%r5", "%r6", "%r7", "%r8", "%r9",
> > +	"%r10", "%r11", "%r12", "%r13", "%r14",
> > +	"%r15", "%r16", "%r17", "%r18", "%r19",
> > +	"%r20", "%r21", "%r22", "%r23", "%r24",
> > +	"%r25", "%r26", "%r27", "%r28", "%r29",
> > +	"%lr", "%sp",
> > +};
> > +#endif
> > diff --git a/tools/perf/arch/powerpc/include/dwarf-regs-table.h b/tools/perf/arch/powerpc/include/dwarf-regs-table.h
> > new file mode 100644
> > index 0000000..db4730f
> > --- /dev/null
> > +++ b/tools/perf/arch/powerpc/include/dwarf-regs-table.h
> > @@ -0,0 +1,27 @@
> > +#ifdef DEFINE_DWARF_REGSTR_TABLE
> > +/* This is included in perf/util/dwarf-regs.c */
> > +
> > +/*
> > + * Reference:
> > + * http://refspecs.linuxfoundation.org/ELF/ppc64/PPC-elf64abi-1.9.html
> > + * http://refspecs.linux-foundation.org/elf/elfspec_ppc.pdf
> > + */
> > +#define REG_DWARFNUM_NAME(reg, idx)	[idx] = "%" #reg
> > +
> > +static const char * const powerpc_regstr_tbl[] = {
> > +	"%gpr0", "%gpr1", "%gpr2", "%gpr3", "%gpr4",
> > +	"%gpr5", "%gpr6", "%gpr7", "%gpr8", "%gpr9",
> > +	"%gpr10", "%gpr11", "%gpr12", "%gpr13", "%gpr14",
> > +	"%gpr15", "%gpr16", "%gpr17", "%gpr18", "%gpr19",
> > +	"%gpr20", "%gpr21", "%gpr22", "%gpr23", "%gpr24",
> > +	"%gpr25", "%gpr26", "%gpr27", "%gpr28", "%gpr29",
> > +	"%gpr30", "%gpr31",
> > +	REG_DWARFNUM_NAME(msr,   66),
> > +	REG_DWARFNUM_NAME(ctr,   109),
> > +	REG_DWARFNUM_NAME(link,  108),
> > +	REG_DWARFNUM_NAME(xer,   101),
> > +	REG_DWARFNUM_NAME(dar,   119),
> > +	REG_DWARFNUM_NAME(dsisr, 118),
> > +};
> > +
> > +#endif
> > diff --git a/tools/perf/arch/s390/include/dwarf-regs-table.h b/tools/perf/arch/s390/include/dwarf-regs-table.h
> > new file mode 100644
> > index 0000000..9da74a9
> > --- /dev/null
> > +++ b/tools/perf/arch/s390/include/dwarf-regs-table.h
> > @@ -0,0 +1,8 @@
> > +#ifdef DEFINE_DWARF_REGSTR_TABLE
> > +/* This is included in perf/util/dwarf-regs.c */
> > +
> > +static const char * const s390_regstr_tbl[] = {
> > +	"%r0", "%r1",  "%r2",  "%r3",  "%r4",  "%r5",  "%r6",  "%r7",
> > +	"%r8", "%r9", "%r10", "%r11", "%r12", "%r13", "%r14", "%r15",
> > +};
> > +#endif
> > diff --git a/tools/perf/arch/sh/include/dwarf-regs-table.h b/tools/perf/arch/sh/include/dwarf-regs-table.h
> > new file mode 100644
> > index 0000000..3a2deaf
> > --- /dev/null
> > +++ b/tools/perf/arch/sh/include/dwarf-regs-table.h
> > @@ -0,0 +1,25 @@
> > +#ifdef DEFINE_DWARF_REGSTR_TABLE
> > +/* This is included in perf/util/dwarf-regs.c */
> > +
> > +const char * const sh_regstr_tbl[] = {
> > +	"r0",
> > +	"r1",
> > +	"r2",
> > +	"r3",
> > +	"r4",
> > +	"r5",
> > +	"r6",
> > +	"r7",
> > +	"r8",
> > +	"r9",
> > +	"r10",
> > +	"r11",
> > +	"r12",
> > +	"r13",
> > +	"r14",
> > +	"r15",
> > +	"pc",
> > +	"pr",
> > +};
> > +
> > +#endif
> > diff --git a/tools/perf/arch/sparc/include/dwarf-regs-table.h b/tools/perf/arch/sparc/include/dwarf-regs-table.h
> > new file mode 100644
> > index 0000000..12c0761
> > --- /dev/null
> > +++ b/tools/perf/arch/sparc/include/dwarf-regs-table.h
> > @@ -0,0 +1,18 @@
> > +#ifdef DEFINE_DWARF_REGSTR_TABLE
> > +/* This is included in perf/util/dwarf-regs.c */
> > +
> > +static const char * const sparc_regstr_tbl[] = {
> > +	"%g0", "%g1", "%g2", "%g3", "%g4", "%g5", "%g6", "%g7",
> > +	"%o0", "%o1", "%o2", "%o3", "%o4", "%o5", "%sp", "%o7",
> > +	"%l0", "%l1", "%l2", "%l3", "%l4", "%l5", "%l6", "%l7",
> > +	"%i0", "%i1", "%i2", "%i3", "%i4", "%i5", "%fp", "%i7",
> > +	"%f0", "%f1", "%f2", "%f3", "%f4", "%f5", "%f6", "%f7",
> > +	"%f8", "%f9", "%f10", "%f11", "%f12", "%f13", "%f14", "%f15",
> > +	"%f16", "%f17", "%f18", "%f19", "%f20", "%f21", "%f22", "%f23",
> > +	"%f24", "%f25", "%f26", "%f27", "%f28", "%f29", "%f30", "%f31",
> > +	"%f32", "%f33", "%f34", "%f35", "%f36", "%f37", "%f38", "%f39",
> > +	"%f40", "%f41", "%f42", "%f43", "%f44", "%f45", "%f46", "%f47",
> > +	"%f48", "%f49", "%f50", "%f51", "%f52", "%f53", "%f54", "%f55",
> > +	"%f56", "%f57", "%f58", "%f59", "%f60", "%f61", "%f62", "%f63",
> > +};
> > +#endif
> > diff --git a/tools/perf/arch/x86/include/dwarf-regs-table.h b/tools/perf/arch/x86/include/dwarf-regs-table.h
> > new file mode 100644
> > index 0000000..39ac7cb
> > --- /dev/null
> > +++ b/tools/perf/arch/x86/include/dwarf-regs-table.h
> > @@ -0,0 +1,14 @@
> > +#ifdef DEFINE_DWARF_REGSTR_TABLE
> > +/* This is included in perf/util/dwarf-regs.c */
> > +
> > +static const char * const x86_32_regstr_tbl[] = {
> > +	"%ax", "%cx", "%dx", "%bx", "$stack",/* Stack address instead of %sp */
> > +	"%bp", "%si", "%di",
> > +};
> > +
> > +static const char * const x86_64_regstr_tbl[] = {
> > +	"%ax", "dx", "%cx", "%bx", "%si", "%di",
> > +	"%bp", "%sp", "%r8", "%r9", "%r10", "%r11",
> > +	"%r12", "%r13", "%r14", "%r15",
> > +};
> > +#endif
> > diff --git a/tools/perf/arch/xtensa/include/dwarf-regs-table.h b/tools/perf/arch/xtensa/include/dwarf-regs-table.h
> > new file mode 100644
> > index 0000000..aa0444a
> > --- /dev/null
> > +++ b/tools/perf/arch/xtensa/include/dwarf-regs-table.h
> > @@ -0,0 +1,8 @@
> > +#ifdef DEFINE_DWARF_REGSTR_TABLE
> > +/* This is included in perf/util/dwarf-regs.c */
> > +
> > +static const char * const xtensa_regstr_tbl[] = {
> > +	"a0", "a1", "a2", "a3", "a4", "a5", "a6", "a7",
> > +	"a8", "a9", "a10", "a11", "a12", "a13", "a14", "a15",
> > +};
> > +#endif
> > diff --git a/tools/perf/util/Build b/tools/perf/util/Build
> > index 91c5f6e..f1a6d17 100644
> > --- a/tools/perf/util/Build
> > +++ b/tools/perf/util/Build
> > @@ -98,6 +98,7 @@ endif
> >  
> >  libperf-$(CONFIG_DWARF) += probe-finder.o
> >  libperf-$(CONFIG_DWARF) += dwarf-aux.o
> > +libperf-$(CONFIG_DWARF) += dwarf-regs.o
> >  
> >  libperf-$(CONFIG_LIBDW_DWARF_UNWIND) += unwind-libdw.o
> >  libperf-$(CONFIG_LOCAL_LIBUNWIND)    += unwind-libunwind-local.o
> > diff --git a/tools/perf/util/dwarf-regs.c b/tools/perf/util/dwarf-regs.c
> > new file mode 100644
> > index 0000000..ce15816
> > --- /dev/null
> > +++ b/tools/perf/util/dwarf-regs.c
> > @@ -0,0 +1,55 @@
> > +/*
> > + * dwarf-regs.c : Mapping of DWARF debug register numbers into register names.
> > + *
> > + * Written by: Masami Hiramatsu <mhiramat@kernel.org>
> > + */
> > +
> > +#include <util.h>
> > +#include <debug.h>
> > +#include <dwarf-regs.h>
> > +#include <elf.h>
> > +
> > +/* Define const char * {arch}_register_tbl[] */
> > +#define DEFINE_DWARF_REGSTR_TABLE
> > +#include "../arch/x86/include/dwarf-regs-table.h"
> > +#include "../arch/arm/include/dwarf-regs-table.h"
> > +#include "../arch/arm64/include/dwarf-regs-table.h"
> > +#include "../arch/sh/include/dwarf-regs-table.h"
> > +#include "../arch/powerpc/include/dwarf-regs-table.h"
> > +#include "../arch/s390/include/dwarf-regs-table.h"
> > +#include "../arch/sparc/include/dwarf-regs-table.h"
> > +#include "../arch/xtensa/include/dwarf-regs-table.h"
> > +
> > +#define __get_dwarf_regstr(tbl, n) (((n) < ARRAY_SIZE(tbl)) ? (tbl)[(n)] : NULL)
> > +
> > +/* Return architecture dependent register string (for kprobe-tracer) */
> > +const char *get_dwarf_regstr(unsigned int n, unsigned int machine)
> > +{
> > +	switch (machine) {
> > +	case EM_NONE:	/* Generic arch - use host arch */
> > +		return get_arch_regstr(n);
> > +	case EM_386:
> > +		return __get_dwarf_regstr(x86_32_regstr_tbl, n);
> > +	case EM_X86_64:
> > +		return __get_dwarf_regstr(x86_64_regstr_tbl, n);
> > +	case EM_ARM:
> > +		return __get_dwarf_regstr(arm_regstr_tbl, n);
> > +	case EM_AARCH64:
> > +		return __get_dwarf_regstr(aarch64_regstr_tbl, n);
> > +	case EM_SH:
> > +		return __get_dwarf_regstr(sh_regstr_tbl, n);
> > +	case EM_S390:
> > +		return __get_dwarf_regstr(s390_regstr_tbl, n);
> > +	case EM_PPC:
> > +	case EM_PPC64:
> > +		return __get_dwarf_regstr(powerpc_regstr_tbl, n);
> > +	case EM_SPARC:
> > +	case EM_SPARCV9:
> > +		return __get_dwarf_regstr(sparc_regstr_tbl, n);
> > +	case EM_XTENSA:
> > +		return __get_dwarf_regstr(xtensa_regstr_tbl, n);
> > +	default:
> > +		pr_err("ELF MACHINE %x is not supported.\n", machine);
> > +	}
> > +	return NULL;
> > +}
> > diff --git a/tools/perf/util/include/dwarf-regs.h b/tools/perf/util/include/dwarf-regs.h
> > index 07c644e..43bfd8d 100644
> > --- a/tools/perf/util/include/dwarf-regs.h
> > +++ b/tools/perf/util/include/dwarf-regs.h
> > @@ -3,6 +3,12 @@
> >  
> >  #ifdef HAVE_DWARF_SUPPORT
> >  const char *get_arch_regstr(unsigned int n);
> > +/*
> > + * get_dwarf_regstr - Returns ftrace register string from DWARF regnum
> > + * n: DWARF register number
> > + * machine: ELF machine signature (EM_*)
> > + */
> > +const char *get_dwarf_regstr(unsigned int n, unsigned int machine);
> >  #endif
> >  
> >  #ifdef HAVE_ARCH_REGS_QUERY_REGISTER_OFFSET
> > diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> > index ac4740f..508b61c 100644
> > --- a/tools/perf/util/probe-finder.c
> > +++ b/tools/perf/util/probe-finder.c
> > @@ -171,6 +171,7 @@ static struct probe_trace_arg_ref *alloc_trace_arg_ref(long offs)
> >   */
> >  static int convert_variable_location(Dwarf_Die *vr_die, Dwarf_Addr addr,
> >  				     Dwarf_Op *fb_ops, Dwarf_Die *sp_die,
> > +				     unsigned int machine,
> >  				     struct probe_trace_arg *tvar)
> >  {
> >  	Dwarf_Attribute attr;
> > @@ -266,7 +267,7 @@ static_var:
> >  	if (!tvar)
> >  		return ret2;
> >  
> > -	regs = get_arch_regstr(regn);
> > +	regs = get_dwarf_regstr(regn, machine);
> >  	if (!regs) {
> >  		/* This should be a bug in DWARF or this tool */
> >  		pr_warning("Mapping for the register number %u "
> > @@ -543,7 +544,7 @@ static int convert_variable(Dwarf_Die *vr_die, struct probe_finder *pf)
> >  		 dwarf_diename(vr_die));
> >  
> >  	ret = convert_variable_location(vr_die, pf->addr, pf->fb_ops,
> > -					&pf->sp_die, pf->tvar);
> > +					&pf->sp_die, pf->machine, pf->tvar);
> >  	if (ret == -ENOENT || ret == -EINVAL) {
> >  		pr_err("Failed to find the location of the '%s' variable at this address.\n"
> >  		       " Perhaps it has been optimized out.\n"
> > @@ -1106,11 +1107,8 @@ static int debuginfo__find_probes(struct debuginfo *dbg,
> >  				  struct probe_finder *pf)
> >  {
> >  	int ret = 0;
> > -
> > -#if _ELFUTILS_PREREQ(0, 142)
> >  	Elf *elf;
> >  	GElf_Ehdr ehdr;
> > -	GElf_Shdr shdr;
> >  
> >  	if (pf->cfi_eh || pf->cfi_dbg)
> >  		return debuginfo__find_probe_location(dbg, pf);
> > @@ -1123,11 +1121,18 @@ static int debuginfo__find_probes(struct debuginfo *dbg,
> >  	if (gelf_getehdr(elf, &ehdr) == NULL)
> >  		return -EINVAL;
> >  
> > -	if (elf_section_by_name(elf, &ehdr, &shdr, ".eh_frame", NULL) &&
> > -	    shdr.sh_type == SHT_PROGBITS)
> > -		pf->cfi_eh = dwarf_getcfi_elf(elf);
> > +	pf->machine = ehdr.e_machine;
> > +
> > +#if _ELFUTILS_PREREQ(0, 142)
> > +	do {
> > +		GElf_Shdr shdr;
> > +
> > +		if (elf_section_by_name(elf, &ehdr, &shdr, ".eh_frame", NULL) &&
> > +		    shdr.sh_type == SHT_PROGBITS)
> > +			pf->cfi_eh = dwarf_getcfi_elf(elf);
> >  
> > -	pf->cfi_dbg = dwarf_getcfi(dbg->dbg);
> > +		pf->cfi_dbg = dwarf_getcfi(dbg->dbg);
> > +	} while (0);
> >  #endif
> >  
> >  	ret = debuginfo__find_probe_location(dbg, pf);
> > @@ -1155,7 +1160,7 @@ static int copy_variables_cb(Dwarf_Die *die_mem, void *data)
> >  	    (tag == DW_TAG_variable && vf->vars)) {
> >  		if (convert_variable_location(die_mem, vf->pf->addr,
> >  					      vf->pf->fb_ops, &pf->sp_die,
> > -					      NULL) == 0) {
> > +					      pf->machine, NULL) == 0) {
> >  			vf->args[vf->nargs].var = (char *)dwarf_diename(die_mem);
> >  			if (vf->args[vf->nargs].var == NULL) {
> >  				vf->ret = -ENOMEM;
> > @@ -1318,7 +1323,7 @@ static int collect_variables_cb(Dwarf_Die *die_mem, void *data)
> >  	    tag == DW_TAG_variable) {
> >  		ret = convert_variable_location(die_mem, af->pf.addr,
> >  						af->pf.fb_ops, &af->pf.sp_die,
> > -						NULL);
> > +						af->pf.machine, NULL);
> >  		if (ret == 0 || ret == -ERANGE) {
> >  			int ret2;
> >  			bool externs = !af->child;
> > diff --git a/tools/perf/util/probe-finder.h b/tools/perf/util/probe-finder.h
> > index 51137fc..f1d8558 100644
> > --- a/tools/perf/util/probe-finder.h
> > +++ b/tools/perf/util/probe-finder.h
> > @@ -80,6 +80,7 @@ struct probe_finder {
> >  	Dwarf_CFI		*cfi_dbg;
> >  #endif
> >  	Dwarf_Op		*fb_ops;	/* Frame base attribute */
> > +	unsigned int		machine;	/* Target machine arch */
> >  	struct perf_probe_arg	*pvar;		/* Current target variable */
> >  	struct probe_trace_arg	*tvar;		/* Current result variable */
> >  };


-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH 2/4] perf-probe: Add offline output directory option
  2016-08-24 12:58   ` Arnaldo Carvalho de Melo
  2016-08-25  6:37     ` Masami Hiramatsu
@ 2016-08-25 10:48     ` Masami Hiramatsu
  2016-08-25 14:40       ` Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 12+ messages in thread
From: Masami Hiramatsu @ 2016-08-25 10:48 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Peter Zijlstra, Ingo Molnar, Jiri Olsa

On Wed, 24 Aug 2016 09:58:45 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> 
> Also please fix the OPT_STRING string, it should start with a capital
> letter:
> 
>         --max-probes <n>  Set how many probe points can be found for a probe.
>         --no-inlines      Don't search inlined functions
>         --outdir <directory>
>                           path to offline output directory
>         --range           Show variables location range in scope (with --vars only)
> 
> 
> See how it stands out? All the others start with a capital letter.

BTW, actually in most case, perf shows option explanations start without a captal letter,

./perf lock

 Usage: perf lock [<options>] {record|report|script|info}

    -D, --dump-raw-trace  dump raw trace in ASCII
    -i, --input <file>    input file name
    -v, --verbose         be more verbose (show symbol address, etc)

etc. Some commands (including probe) mixing it...

Thanks,

-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH 2/4] perf-probe: Add offline output directory option
  2016-08-25 10:48     ` Masami Hiramatsu
@ 2016-08-25 14:40       ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-08-25 14:40 UTC (permalink / raw)
  To: Masami Hiramatsu; +Cc: linux-kernel, Peter Zijlstra, Ingo Molnar, Jiri Olsa

Em Thu, Aug 25, 2016 at 07:48:16PM +0900, Masami Hiramatsu escreveu:
> On Wed, 24 Aug 2016 09:58:45 -0300
> Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> 
> > 
> > Also please fix the OPT_STRING string, it should start with a capital
> > letter:
> > 
> >         --max-probes <n>  Set how many probe points can be found for a probe.
> >         --no-inlines      Don't search inlined functions
> >         --outdir <directory>
> >                           path to offline output directory
> >         --range           Show variables location range in scope (with --vars only)
> > 
> > 
> > See how it stands out? All the others start with a capital letter.
> 
> BTW, actually in most case, perf shows option explanations start without a captal letter,
> 
> ./perf lock
> 
>  Usage: perf lock [<options>] {record|report|script|info}
> 
>     -D, --dump-raw-trace  dump raw trace in ASCII
>     -i, --input <file>    input file name
>     -v, --verbose         be more verbose (show symbol address, etc)
> 
> etc. Some commands (including probe) mixing it...

Right, inconsistent, so I went to see how other utilities do this, and,
say, something really old:

[acme@jouet linux]$ cp --help | grep -- "[      ]\+-[a-z]"
  or:  cp [OPTION]... -t DIRECTORY SOURCE...
  -a, --archive                same as -dR --preserve=all
  -b                           like --backup but does not accept an argument
  -d                           same as --no-dereference --preserve=links
  -f, --force                  if an existing destination file cannot be
                                 is ignored when the -n option is also used)
  -i, --interactive            prompt before overwrite (overrides a previous -n
  -l, --link                   hard link files instead of copying
  -n, --no-clobber             do not overwrite an existing file (overrides
                                 a previous -i option)
  -p                           same as --preserve=mode,ownership,timestamps
  -c                           deprecated, same as --preserve=context
  -R, -r, --recursive          copy directories recursively
  -s, --symbolic-link          make symbolic links instead of copying
  -t, --target-directory=DIRECTORY  copy all SOURCE arguments into DIRECTORY
  -u, --update                 copy only when the SOURCE file is newer
  -v, --verbose                explain what is being done
  -x, --one-file-system        stay on this file system
[acme@jouet linux]$ 

And then something more recent, and one perf initially was modelled after:

[acme@jouet linux]$ git commit -h |& grep -- "[         ]\+-[a-z]" | tail -15
    -q, --quiet           suppress summary after successful commit
    -v, --verbose         show diff in commit message template
    -m, --message <message>
    -c, --reedit-message <commit>
    -s, --signoff         add Signed-off-by:
    -t, --template <file>
    -e, --edit            force edit of commit
    -a, --all             commit all changed files
    -i, --include         add specified files to index for commit
    -p, --patch           interactively add changes
    -o, --only            commit only specified files
    -n, --no-verify       bypass pre-commit hook
    -z, --null            terminate entries with NUL
    -u, --untracked-files[=<mode>]
[acme@jouet linux]$ 

So, yeah, use lowercase and over time we can go on making it consistent...

- Arnaldo

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [tip:perf/core] perf probe: Remove unused tracing_dir variable
  2016-08-24  5:57 ` [RFC PATCH 1/4] perf-probe: Remove unused tracing_dir variable Masami Hiramatsu
@ 2016-09-05 13:19   ` tip-bot for Masami Hiramatsu
  0 siblings, 0 replies; 12+ messages in thread
From: tip-bot for Masami Hiramatsu @ 2016-09-05 13:19 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: jolsa, tglx, peterz, acme, hpa, mhiramat, linux-kernel, mingo

Commit-ID:  04e11960aa9a5edbe612dd8623190e341aedab35
Gitweb:     http://git.kernel.org/tip/04e11960aa9a5edbe612dd8623190e341aedab35
Author:     Masami Hiramatsu <mhiramat@kernel.org>
AuthorDate: Wed, 24 Aug 2016 14:57:58 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 24 Aug 2016 09:41:56 -0300

perf probe: Remove unused tracing_dir variable

Remove unused tracing_dir variable from open_probe_events().

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/147201827792.5713.4165387506020511920.stgit@devbox
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-file.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index 697ef66..6f931e4 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -73,11 +73,10 @@ static void print_both_open_warning(int kerr, int uerr)
 static int open_probe_events(const char *trace_file, bool readwrite)
 {
 	char buf[PATH_MAX];
-	const char *tracing_dir = "";
 	int ret;
 
-	ret = e_snprintf(buf, PATH_MAX, "%s/%s%s",
-			 tracing_path, tracing_dir, trace_file);
+	ret = e_snprintf(buf, PATH_MAX, "%s/%s",
+			 tracing_path, trace_file);
 	if (ret >= 0) {
 		pr_debug("Opening %s write=%d\n", buf, readwrite);
 		if (readwrite && !probe_event_dry_run)

^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2016-09-05 13:20 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-24  5:57 [RFC PATCH 0/4] perf probe: Introduce remote cross-arch probes Masami Hiramatsu
2016-08-24  5:57 ` [RFC PATCH 1/4] perf-probe: Remove unused tracing_dir variable Masami Hiramatsu
2016-09-05 13:19   ` [tip:perf/core] perf probe: " tip-bot for Masami Hiramatsu
2016-08-24  5:58 ` [RFC PATCH 2/4] perf-probe: Add offline output directory option Masami Hiramatsu
2016-08-24 12:58   ` Arnaldo Carvalho de Melo
2016-08-25  6:37     ` Masami Hiramatsu
2016-08-25 10:48     ` Masami Hiramatsu
2016-08-25 14:40       ` Arnaldo Carvalho de Melo
2016-08-24  5:58 ` [RFC PATCH 3/4] perf-probe: Ignore vmlinux buildid if offline kernel is given Masami Hiramatsu
2016-08-24  5:58 ` [RFC PATCH 4/4] perf-probe: Support probing on offline cross-arch binary Masami Hiramatsu
2016-08-24 13:03   ` Arnaldo Carvalho de Melo
2016-08-25  6:57     ` Masami Hiramatsu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).