linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/7] perf/probe: Support multiprobe and immediates with fixes
@ 2019-11-18  8:11 Masami Hiramatsu
  2019-11-18  8:11 ` [PATCH v3 1/7] perf probe: Show correct statement line number by perf probe -l Masami Hiramatsu
                   ` (7 more replies)
  0 siblings, 8 replies; 22+ messages in thread
From: Masami Hiramatsu @ 2019-11-18  8:11 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Masami Hiramatsu, Ingo Molnar, Steven Rostedt, linux-kernel,
	Tom Zanussi, Ravi Bangoria, Namhyung Kim

Hi Arnaldo,

This is the 3rd version of the multiprobe support on perf probe
including some fixes about "representive lines"

The previous thread is here:

https://lkml.kernel.org/r/157314406866.4063.16995747442215702109.stgit@devnote2

On the previous thread, we discussed some issues about incorrect line
information shown by perf probe. I finally fixed those by introducing
an idea of "representive line" -- a line which has a unique address
(no other lines shares the same address) or a line which has the least
line number among lines sharing same address. Now perf probe only shows
such representive lines can be probed([1/7][3/7]), and if user tries to
probe other non representive lines, it shows which line user should
probe ([2/7]). The rest of patches in the series are same as v2 (except
for [4/7], example output is updated)

This can be applied on top of perf/core.

Thank you,

---

Masami Hiramatsu (7):
      perf probe: Show correct statement line number by perf probe -l
      perf probe: Verify given line is a representive line
      perf probe: Do not show non representive lines by perf-probe -L
      perf probe: Generate event name with line number
      perf probe: Support multiprobe event
      perf probe: Support DW_AT_const_value constant value
      perf probe: Trace a magic number if variable is not found


 tools/perf/util/dwarf-aux.c    |   62 ++++++++++++++++++++-
 tools/perf/util/probe-event.c  |   19 ++++++-
 tools/perf/util/probe-event.h  |    3 +
 tools/perf/util/probe-file.c   |   14 +++++
 tools/perf/util/probe-file.h   |    2 +
 tools/perf/util/probe-finder.c |  116 +++++++++++++++++++++++++++++++++++++++-
 tools/perf/util/probe-finder.h |    1 
 7 files changed, 206 insertions(+), 11 deletions(-)

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

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

* [PATCH v3 1/7] perf probe: Show correct statement line number by perf probe -l
  2019-11-18  8:11 [PATCH v3 0/7] perf/probe: Support multiprobe and immediates with fixes Masami Hiramatsu
@ 2019-11-18  8:11 ` Masami Hiramatsu
  2019-11-18 21:57   ` Arnaldo Carvalho de Melo
  2019-11-19 16:56   ` [tip: perf/core] " tip-bot2 for Masami Hiramatsu
  2019-11-18  8:12 ` [PATCH v3 2/7] perf probe: Verify given line is a representive line Masami Hiramatsu
                   ` (6 subsequent siblings)
  7 siblings, 2 replies; 22+ messages in thread
From: Masami Hiramatsu @ 2019-11-18  8:11 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Masami Hiramatsu, Ingo Molnar, Steven Rostedt, linux-kernel,
	Tom Zanussi, Ravi Bangoria, Namhyung Kim

The dwarf_getsrc_die() can return the line which is not a statement
nor the least line number among the lines which shares same address.
This can lead perf probe --list shows incorrect line number for
probed address.
To fix this, this introduces cu_getsrc_die() which returns only a
statement line and which is the least line number (we call it
the representive line for an address), and use it in cu_find_lineinfo().

Also, if the given address is the entry address of a real function,
cu_find_lineinfo() returns the function declared line number instead
of the start line number of the function body.

For example, without this change perf probe -l shows incorrect line
as below.

  # perf probe -a kernel_read:2
  Added new event:
    probe:kernel_read    (on kernel_read:2)

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

  	perf record -e probe:kernel_read -aR sleep 1

  # perf probe -l
    probe:kernel_read    (on kernel_read:1@linux-5.0.0/fs/read_write.c)

With this fix, it shows correct line number as below;

  # perf probe -l
    probe:kernel_read    (on kernel_read:2@linux-5.0.0/fs/read_write.c)


Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 tools/perf/util/dwarf-aux.c |   62 ++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 58 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index 5544bfbd0f6c..aa898014ad12 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -59,6 +59,51 @@ const char *cu_get_comp_dir(Dwarf_Die *cu_die)
 	return dwarf_formstring(&attr);
 }
 
+/* Unlike dwarf_getsrc_die(), cu_getsrc_die() only returns statement line */
+static Dwarf_Line *cu_getsrc_die(Dwarf_Die *cu_die, Dwarf_Addr addr)
+{
+	Dwarf_Addr laddr;
+	Dwarf_Lines *lines;
+	Dwarf_Line *line;
+	size_t nlines, l, u, n;
+	bool flag;
+
+	if (dwarf_getsrclines(cu_die, &lines, &nlines) != 0 ||
+	    nlines == 0)
+		return NULL;
+
+	/* Lines are sorted by address, use binary search */
+	l = 0; u = nlines - 1;
+	while (l < u) {
+		n = u - (u - l) / 2;
+		line = dwarf_onesrcline(lines, n);
+		if (!line || dwarf_lineaddr(line, &laddr) != 0)
+			return NULL;
+		if (addr < laddr)
+			u = n - 1;
+		else
+			l = n;
+	}
+	/* Going backward to find the lowest line */
+	do {
+		line = dwarf_onesrcline(lines, --l);
+		if (!line || dwarf_lineaddr(line, &laddr) != 0)
+			return NULL;
+	} while (laddr == addr);
+	l++;
+	/* Going foward to find the statement line */
+	do {
+		line = dwarf_onesrcline(lines, l++);
+		if (!line || dwarf_lineaddr(line, &laddr) != 0 ||
+		    dwarf_linebeginstatement(line, &flag) != 0)
+			return NULL;
+		if (laddr > addr)
+			return NULL;
+	} while (!flag);
+
+	return line;
+}
+
 /**
  * cu_find_lineinfo - Get a line number and file name for given address
  * @cu_die: a CU DIE
@@ -72,17 +117,26 @@ int cu_find_lineinfo(Dwarf_Die *cu_die, unsigned long addr,
 		    const char **fname, int *lineno)
 {
 	Dwarf_Line *line;
-	Dwarf_Addr laddr;
+	Dwarf_Die die_mem;
+	Dwarf_Addr faddr;
 
-	line = dwarf_getsrc_die(cu_die, (Dwarf_Addr)addr);
-	if (line && dwarf_lineaddr(line, &laddr) == 0 &&
-	    addr == (unsigned long)laddr && dwarf_lineno(line, lineno) == 0) {
+	if (die_find_realfunc(cu_die, (Dwarf_Addr)addr, &die_mem)
+	    && die_entrypc(&die_mem, &faddr) == 0 &&
+	    faddr == addr) {
+		*fname = dwarf_decl_file(&die_mem);
+		dwarf_decl_line(&die_mem, lineno);
+		goto out;
+	}
+
+	line = cu_getsrc_die(cu_die, (Dwarf_Addr)addr);
+	if (line && dwarf_lineno(line, lineno) == 0) {
 		*fname = dwarf_linesrc(line, NULL, NULL);
 		if (!*fname)
 			/* line number is useless without filename */
 			*lineno = 0;
 	}
 
+out:
 	return *lineno ?: -ENOENT;
 }
 


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

* [PATCH v3 2/7] perf probe: Verify given line is a representive line
  2019-11-18  8:11 [PATCH v3 0/7] perf/probe: Support multiprobe and immediates with fixes Masami Hiramatsu
  2019-11-18  8:11 ` [PATCH v3 1/7] perf probe: Show correct statement line number by perf probe -l Masami Hiramatsu
@ 2019-11-18  8:12 ` Masami Hiramatsu
  2019-11-18 21:59   ` Arnaldo Carvalho de Melo
  2019-11-19 16:56   ` [tip: perf/core] " tip-bot2 for Masami Hiramatsu
  2019-11-18  8:12 ` [PATCH v3 3/7] perf probe: Do not show non representive lines by perf-probe -L Masami Hiramatsu
                   ` (5 subsequent siblings)
  7 siblings, 2 replies; 22+ messages in thread
From: Masami Hiramatsu @ 2019-11-18  8:12 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Masami Hiramatsu, Ingo Molnar, Steven Rostedt, linux-kernel,
	Tom Zanussi, Ravi Bangoria, Namhyung Kim

Verify user given probe line is a representive line (which doesn't
share the address with other lines or the line is the least line
among the lines which shares same address), and if not, it shows
what is the representive line.

Without this fix, user can put a probe on the lines which is not a
a representive line. But since this is not a representive line,
perf probe -l shows a representive line number instead of user given
line number. e.g. (put kernel_read:3, but listed as kernel_read:2)

  # perf probe -a kernel_read:3
  Added new event:
    probe:kernel_read    (on kernel_read:3)

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

  	perf record -e probe:kernel_read -aR sleep 1

  # perf probe -l
    probe:kernel_read    (on kernel_read:2@linux-5.0.0/fs/read_write.c)

With this fix, perf probe doesn't allow user to put a probe on
a representive line, and tell what is the representive line.

  # perf probe -a kernel_read:3
  This line is sharing the addrees with other lines.
  Please try to probe at kernel_read:2 instead.
    Error: Failed to add events.

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

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 9ecea45da4ca..ef1b320cedf8 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -776,6 +776,39 @@ static Dwarf_Die *find_best_scope(struct probe_finder *pf, Dwarf_Die *die_mem)
 	return fsp.found ? die_mem : NULL;
 }
 
+static int verify_representive_line(struct probe_finder *pf, const char *fname,
+				int lineno, Dwarf_Addr addr)
+{
+	const char *__fname, *__func = NULL;
+	Dwarf_Die die_mem;
+	int __lineno;
+
+	/* Verify line number and address by reverse search */
+	if (cu_find_lineinfo(&pf->cu_die, addr, &__fname, &__lineno) < 0)
+		return 0;
+
+	pr_debug2("Reversed line: %s:%d\n", __fname, __lineno);
+	if (strcmp(fname, __fname) || lineno == __lineno)
+		return 0;
+
+	pr_warning("This line is sharing the addrees with other lines.\n");
+
+	if (pf->pev->point.function) {
+		/* Find best match function name and lines */
+		pf->addr = addr;
+		if (find_best_scope(pf, &die_mem)
+		    && die_match_name(&die_mem, pf->pev->point.function)
+		    && dwarf_decl_line(&die_mem, &lineno) == 0) {
+			__func = dwarf_diename(&die_mem);
+			__lineno -= lineno;
+		}
+	}
+	pr_warning("Please try to probe at %s:%d instead.\n",
+		   __func ? : __fname, __lineno);
+
+	return -ENOENT;
+}
+
 static int probe_point_line_walker(const char *fname, int lineno,
 				   Dwarf_Addr addr, void *data)
 {
@@ -786,6 +819,9 @@ static int probe_point_line_walker(const char *fname, int lineno,
 	if (lineno != pf->lno || strtailcmp(fname, pf->fname) != 0)
 		return 0;
 
+	if (verify_representive_line(pf, fname, lineno, addr))
+		return -ENOENT;
+
 	pf->addr = addr;
 	sc_die = find_best_scope(pf, &die_mem);
 	if (!sc_die) {


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

* [PATCH v3 3/7] perf probe: Do not show non representive lines by perf-probe -L
  2019-11-18  8:11 [PATCH v3 0/7] perf/probe: Support multiprobe and immediates with fixes Masami Hiramatsu
  2019-11-18  8:11 ` [PATCH v3 1/7] perf probe: Show correct statement line number by perf probe -l Masami Hiramatsu
  2019-11-18  8:12 ` [PATCH v3 2/7] perf probe: Verify given line is a representive line Masami Hiramatsu
@ 2019-11-18  8:12 ` Masami Hiramatsu
  2019-11-18 22:01   ` Arnaldo Carvalho de Melo
  2019-11-19 16:56   ` [tip: perf/core] " tip-bot2 for Masami Hiramatsu
  2019-11-18  8:12 ` [PATCH v3 4/7] perf probe: Generate event name with line number Masami Hiramatsu
                   ` (4 subsequent siblings)
  7 siblings, 2 replies; 22+ messages in thread
From: Masami Hiramatsu @ 2019-11-18  8:12 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Masami Hiramatsu, Ingo Molnar, Steven Rostedt, linux-kernel,
	Tom Zanussi, Ravi Bangoria, Namhyung Kim

Since perf probe -L shows non representive lines, it can be
mislead users where user can put probes.
This prevents to show such non representive lines so that user
can understand which lines user can probe.

  # perf probe -L kernel_read
  <kernel_read@/build/linux-pvZVvI/linux-5.0.0/fs/read_write.c:0>
        0  ssize_t kernel_read(struct file *file, void *buf, size_t count, loff_t *pos)
           {
        2         mm_segment_t old_fs;
                  ssize_t result;

                  old_fs = get_fs();
        6         set_fs(get_ds());
                  /* The cast to a user pointer is valid due to the set_fs() */
        8         result = vfs_read(file, (void __user *)buf, count, pos);
        9         set_fs(old_fs);
       10         return result;
           }
           EXPORT_SYMBOL(kernel_read);

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

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index ef1b320cedf8..f12ad507a822 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -1734,12 +1734,19 @@ static int line_range_walk_cb(const char *fname, int lineno,
 			      void *data)
 {
 	struct line_finder *lf = data;
+	const char *__fname;
+	int __lineno;
 	int err;
 
 	if ((strtailcmp(fname, lf->fname) != 0) ||
 	    (lf->lno_s > lineno || lf->lno_e < lineno))
 		return 0;
 
+	/* Make sure this line can be reversable */
+	if (cu_find_lineinfo(&lf->cu_die, addr, &__fname, &__lineno) > 0
+	    && (lineno != __lineno || strcmp(fname, __fname)))
+		return 0;
+
 	err = line_range_add_line(fname, lineno, lf->lr);
 	if (err < 0 && err != -EEXIST)
 		return err;


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

* [PATCH v3 4/7] perf probe: Generate event name with line number
  2019-11-18  8:11 [PATCH v3 0/7] perf/probe: Support multiprobe and immediates with fixes Masami Hiramatsu
                   ` (2 preceding siblings ...)
  2019-11-18  8:12 ` [PATCH v3 3/7] perf probe: Do not show non representive lines by perf-probe -L Masami Hiramatsu
@ 2019-11-18  8:12 ` Masami Hiramatsu
  2019-11-18 22:03   ` Arnaldo Carvalho de Melo
  2019-11-19 16:56   ` [tip: perf/core] " tip-bot2 for Masami Hiramatsu
  2019-11-18  8:12 ` [PATCH v3 5/7] perf probe: Support multiprobe event Masami Hiramatsu
                   ` (3 subsequent siblings)
  7 siblings, 2 replies; 22+ messages in thread
From: Masami Hiramatsu @ 2019-11-18  8:12 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Masami Hiramatsu, Ingo Molnar, Steven Rostedt, linux-kernel,
	Tom Zanussi, Ravi Bangoria, Namhyung Kim

Generate event name from function name with line number
as <function>_L<line_number>. Note that this is only for
the new event which is defined by the line number of
function (except for line 0).

If there is another event on same line, you have to use
"-f" option. In that case, the new event has "_1" suffix.

 e.g.
  # perf probe -a kernel_read:2
  Added new event:
    probe:kernel_read_L2 (on kernel_read:2)

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

  	perf record -e probe:kernel_read_L2 -aR sleep 1


But if we omit the line number or 0th line, it will
have no suffix.

  # perf probe -a kernel_read:0
  Added new event:
    probe:kernel_read (on kernel_read)

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

  	perf record -e probe:kernel_read -aR sleep 1

# perf probe -l
  probe:kernel_read    (on kernel_read@linux-5.0.0/fs/read_write.c)
  probe:kernel_read_L2 (on kernel_read:2@linux-5.0.0/fs/read_write.c)

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 Changes in v3:
  - Update samples according to previous fixes.
 Changes in v2:
  - Do not add _L* suffix for the event which has no line
    number or line #0.
---
 tools/perf/util/probe-event.c |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index e29948b8fcab..5c86d2cf6338 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -1679,6 +1679,14 @@ int parse_perf_probe_command(const char *cmd, struct perf_probe_event *pev)
 	if (ret < 0)
 		goto out;
 
+	/* Generate event name if needed */
+	if (!pev->event && pev->point.function && pev->point.line
+			&& !pev->point.lazy_line && !pev->point.offset) {
+		if (asprintf(&pev->event, "%s_L%d", pev->point.function,
+			pev->point.line) < 0)
+			return -ENOMEM;
+	}
+
 	/* Copy arguments and ensure return probe has no C argument */
 	pev->nargs = argc - 1;
 	pev->args = zalloc(sizeof(struct perf_probe_arg) * pev->nargs);


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

* [PATCH v3 5/7] perf probe: Support multiprobe event
  2019-11-18  8:11 [PATCH v3 0/7] perf/probe: Support multiprobe and immediates with fixes Masami Hiramatsu
                   ` (3 preceding siblings ...)
  2019-11-18  8:12 ` [PATCH v3 4/7] perf probe: Generate event name with line number Masami Hiramatsu
@ 2019-11-18  8:12 ` Masami Hiramatsu
  2019-11-19 16:56   ` [tip: perf/core] " tip-bot2 for Masami Hiramatsu
  2019-11-18  8:12 ` [PATCH v3 6/7] perf probe: Support DW_AT_const_value constant value Masami Hiramatsu
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 22+ messages in thread
From: Masami Hiramatsu @ 2019-11-18  8:12 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Masami Hiramatsu, Ingo Molnar, Steven Rostedt, linux-kernel,
	Tom Zanussi, Ravi Bangoria, Namhyung Kim

Support multiprobe event if the event is based on function
and lines and kernel supports it. In this case, perf probe
creates the first probe with an event, and tries to append
following probes on that event, since those probes must be
on the same source code line.

Before this patch;
  # perf probe -a vfs_read:18
  Added new events:
    probe:vfs_read_L18   (on vfs_read:18)
    probe:vfs_read_L18_1 (on vfs_read:18)

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

  	perf record -e probe:vfs_read_L18_1 -aR sleep 1

  #

After this patch (on multiprobe supported kernel)
  # perf probe -a vfs_read:18
  Added new events:
    probe:vfs_read_L18   (on vfs_read:18)
    probe:vfs_read_L18   (on vfs_read:18)

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

  	perf record -e probe:vfs_read_L18 -aR sleep 1

  #

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

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 5c86d2cf6338..8f963a193a5d 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2738,8 +2738,13 @@ static int probe_trace_event__set_name(struct probe_trace_event *tev,
 	if (tev->event == NULL || tev->group == NULL)
 		return -ENOMEM;
 
-	/* Add added event name to namelist */
-	strlist__add(namelist, event);
+	/*
+	 * Add new event name to namelist if multiprobe event is NOT
+	 * supported, since we have to use new event name for following
+	 * probes in that case.
+	 */
+	if (!multiprobe_event_is_supported())
+		strlist__add(namelist, event);
 	return 0;
 }
 
diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index b659466ea498..a63f1a19b0e8 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -1007,6 +1007,7 @@ enum ftrace_readme {
 	FTRACE_README_KRETPROBE_OFFSET,
 	FTRACE_README_UPROBE_REF_CTR,
 	FTRACE_README_USER_ACCESS,
+	FTRACE_README_MULTIPROBE_EVENT,
 	FTRACE_README_END,
 };
 
@@ -1020,6 +1021,7 @@ static struct {
 	DEFINE_TYPE(FTRACE_README_KRETPROBE_OFFSET, "*place (kretprobe): *"),
 	DEFINE_TYPE(FTRACE_README_UPROBE_REF_CTR, "*ref_ctr_offset*"),
 	DEFINE_TYPE(FTRACE_README_USER_ACCESS, "*[u]<offset>*"),
+	DEFINE_TYPE(FTRACE_README_MULTIPROBE_EVENT, "*Create/append/*"),
 };
 
 static bool scan_ftrace_readme(enum ftrace_readme type)
@@ -1085,3 +1087,8 @@ bool user_access_is_supported(void)
 {
 	return scan_ftrace_readme(FTRACE_README_USER_ACCESS);
 }
+
+bool multiprobe_event_is_supported(void)
+{
+	return scan_ftrace_readme(FTRACE_README_MULTIPROBE_EVENT);
+}
diff --git a/tools/perf/util/probe-file.h b/tools/perf/util/probe-file.h
index 986c1c94f64f..850d1b52d60a 100644
--- a/tools/perf/util/probe-file.h
+++ b/tools/perf/util/probe-file.h
@@ -71,6 +71,7 @@ bool probe_type_is_available(enum probe_type type);
 bool kretprobe_offset_is_supported(void);
 bool uprobe_ref_ctr_is_supported(void);
 bool user_access_is_supported(void);
+bool multiprobe_event_is_supported(void);
 #else	/* ! HAVE_LIBELF_SUPPORT */
 static inline struct probe_cache *probe_cache__new(const char *tgt __maybe_unused, struct nsinfo *nsi __maybe_unused)
 {


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

* [PATCH v3 6/7] perf probe: Support DW_AT_const_value constant value
  2019-11-18  8:11 [PATCH v3 0/7] perf/probe: Support multiprobe and immediates with fixes Masami Hiramatsu
                   ` (4 preceding siblings ...)
  2019-11-18  8:12 ` [PATCH v3 5/7] perf probe: Support multiprobe event Masami Hiramatsu
@ 2019-11-18  8:12 ` Masami Hiramatsu
  2019-11-19 16:56   ` [tip: perf/core] " tip-bot2 for Masami Hiramatsu
  2019-11-18  8:12 ` [PATCH v3 7/7] perf probe: Trace a magic number if variable is not found Masami Hiramatsu
  2019-11-18 22:11 ` [PATCH v3 0/7] perf/probe: Support multiprobe and immediates with fixes Arnaldo Carvalho de Melo
  7 siblings, 1 reply; 22+ messages in thread
From: Masami Hiramatsu @ 2019-11-18  8:12 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Masami Hiramatsu, Ingo Molnar, Steven Rostedt, linux-kernel,
	Tom Zanussi, Ravi Bangoria, Namhyung Kim

Support DW_AT_const_value for variable assignment instead of location.
Note that this requires ftrace supporting immediate value.

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

diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index a63f1a19b0e8..5003ba403345 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -1008,6 +1008,7 @@ enum ftrace_readme {
 	FTRACE_README_UPROBE_REF_CTR,
 	FTRACE_README_USER_ACCESS,
 	FTRACE_README_MULTIPROBE_EVENT,
+	FTRACE_README_IMMEDIATE_VALUE,
 	FTRACE_README_END,
 };
 
@@ -1022,6 +1023,7 @@ static struct {
 	DEFINE_TYPE(FTRACE_README_UPROBE_REF_CTR, "*ref_ctr_offset*"),
 	DEFINE_TYPE(FTRACE_README_USER_ACCESS, "*[u]<offset>*"),
 	DEFINE_TYPE(FTRACE_README_MULTIPROBE_EVENT, "*Create/append/*"),
+	DEFINE_TYPE(FTRACE_README_IMMEDIATE_VALUE, "*\\imm-value,*"),
 };
 
 static bool scan_ftrace_readme(enum ftrace_readme type)
@@ -1092,3 +1094,8 @@ bool multiprobe_event_is_supported(void)
 {
 	return scan_ftrace_readme(FTRACE_README_MULTIPROBE_EVENT);
 }
+
+bool immediate_value_is_supported(void)
+{
+	return scan_ftrace_readme(FTRACE_README_IMMEDIATE_VALUE);
+}
diff --git a/tools/perf/util/probe-file.h b/tools/perf/util/probe-file.h
index 850d1b52d60a..0dba88c0f5f0 100644
--- a/tools/perf/util/probe-file.h
+++ b/tools/perf/util/probe-file.h
@@ -72,6 +72,7 @@ bool kretprobe_offset_is_supported(void);
 bool uprobe_ref_ctr_is_supported(void);
 bool user_access_is_supported(void);
 bool multiprobe_event_is_supported(void);
+bool immediate_value_is_supported(void);
 #else	/* ! HAVE_LIBELF_SUPPORT */
 static inline struct probe_cache *probe_cache__new(const char *tgt __maybe_unused, struct nsinfo *nsi __maybe_unused)
 {
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index f12ad507a822..33e90054ad84 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -177,6 +177,17 @@ static int convert_variable_location(Dwarf_Die *vr_die, Dwarf_Addr addr,
 	if (dwarf_attr(vr_die, DW_AT_external, &attr) != NULL)
 		goto static_var;
 
+	/* Constant value */
+	if (dwarf_attr(vr_die, DW_AT_const_value, &attr) &&
+	    immediate_value_is_supported()) {
+		Dwarf_Sword snum;
+
+		dwarf_formsdata(&attr, &snum);
+		ret = asprintf(&tvar->value, "\\%ld", (long)snum);
+
+		return ret < 0 ? -ENOMEM : 0;
+	}
+
 	/* TODO: handle more than 1 exprs */
 	if (dwarf_attr(vr_die, DW_AT_location, &attr) == NULL)
 		return -EINVAL;	/* Broken DIE ? */


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

* [PATCH v3 7/7] perf probe: Trace a magic number if variable is not found
  2019-11-18  8:11 [PATCH v3 0/7] perf/probe: Support multiprobe and immediates with fixes Masami Hiramatsu
                   ` (5 preceding siblings ...)
  2019-11-18  8:12 ` [PATCH v3 6/7] perf probe: Support DW_AT_const_value constant value Masami Hiramatsu
@ 2019-11-18  8:12 ` Masami Hiramatsu
  2019-11-19 16:56   ` [tip: perf/core] " tip-bot2 for Masami Hiramatsu
  2019-11-18 22:11 ` [PATCH v3 0/7] perf/probe: Support multiprobe and immediates with fixes Arnaldo Carvalho de Melo
  7 siblings, 1 reply; 22+ messages in thread
From: Masami Hiramatsu @ 2019-11-18  8:12 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Masami Hiramatsu, Ingo Molnar, Steven Rostedt, linux-kernel,
	Tom Zanussi, Ravi Bangoria, Namhyung Kim

Trace a magic number as immediate value if the target variable
is not found at some probe points which is based on one probe
event.

This feature is good for the case if you trace a source code
line with some local variables, which is compiled into several
instructions and some of the variables are optimized out on
some instructions. Even if so, with this feature, perf probe
trace a magic number instead of such disappeared variables and
fold those probes on one event.

E.g. without this patch,
# perf probe -D "pud_page_vaddr pud"
Failed to find 'pud' in this function.
Failed to find 'pud' in this function.
Failed to find 'pud' in this function.
Failed to find 'pud' in this function.
Failed to find 'pud' in this function.
Failed to find 'pud' in this function.
Failed to find 'pud' in this function.
Failed to find 'pud' in this function.
Failed to find 'pud' in this function.
Failed to find 'pud' in this function.
Failed to find 'pud' in this function.
Failed to find 'pud' in this function.
Failed to find 'pud' in this function.
Failed to find 'pud' in this function.
Failed to find 'pud' in this function.
Failed to find 'pud' in this function.
p:probe/pud_page_vaddr _text+23480787 pud=%ax:x64
p:probe/pud_page_vaddr _text+23808453 pud=%bp:x64
p:probe/pud_page_vaddr _text+23558082 pud=%ax:x64
p:probe/pud_page_vaddr _text+328373 pud=%r8:x64
p:probe/pud_page_vaddr _text+348448 pud=%bx:x64
p:probe/pud_page_vaddr _text+23816818 pud=%bx:x64

With this patch,
# perf probe -D "pud_page_vaddr pud" | head
spurious_kernel_fault is blacklisted function, skip it.
vmalloc_fault is blacklisted function, skip it.
p:probe/pud_page_vaddr _text+23480787 pud=%ax:x64
p:probe/pud_page_vaddr _text+149051 pud=\deade12d:x64
p:probe/pud_page_vaddr _text+23808453 pud=%bp:x64
p:probe/pud_page_vaddr _text+315926 pud=\deade12d:x64
p:probe/pud_page_vaddr _text+23807209 pud=\deade12d:x64
p:probe/pud_page_vaddr _text+23557365 pud=%ax:x64
p:probe/pud_page_vaddr _text+314097 pud=%di:x64
p:probe/pud_page_vaddr _text+314015 pud=\deade12d:x64
p:probe/pud_page_vaddr _text+313893 pud=\deade12d:x64
p:probe/pud_page_vaddr _text+324083 pud=\deade12d:x64

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 Changes in v2:
  - Fix not to check event name.
  - Update before/after example.
---
 tools/perf/util/probe-event.c  |    2 +
 tools/perf/util/probe-event.h  |    3 ++
 tools/perf/util/probe-finder.c |   62 +++++++++++++++++++++++++++++++++++++---
 tools/perf/util/probe-finder.h |    1 +
 4 files changed, 63 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 8f963a193a5d..52b2d165453a 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -46,7 +46,7 @@
 #define PERFPROBE_GROUP "probe"
 
 bool probe_event_dry_run;	/* Dry run flag */
-struct probe_conf probe_conf;
+struct probe_conf probe_conf = { .magic_num = DEFAULT_PROBE_MAGIC_NUM };
 
 #define semantic_error(msg ...) pr_err("Semantic error :" msg)
 
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index 96a319cd2378..4f0eb3a20c36 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -16,10 +16,13 @@ struct probe_conf {
 	bool	no_inlines;
 	bool	cache;
 	int	max_probes;
+	unsigned long	magic_num;
 };
 extern struct probe_conf probe_conf;
 extern bool probe_event_dry_run;
 
+#define DEFAULT_PROBE_MAGIC_NUM	0xdeade12d	/* u32: 3735937325 */
+
 struct symbol;
 
 /* kprobe-tracer and uprobe-tracer tracing point */
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 33e90054ad84..38d6cd22779f 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -536,6 +536,14 @@ static int convert_variable_fields(Dwarf_Die *vr_die, const char *varname,
 		return 0;
 }
 
+static void print_var_not_found(const char *varname)
+{
+	pr_err("Failed to find the location of the '%s' variable at this address.\n"
+	       " Perhaps it has been optimized out.\n"
+	       " Use -V with the --range option to show '%s' location range.\n",
+		varname, varname);
+}
+
 /* Show a variables in kprobe event format */
 static int convert_variable(Dwarf_Die *vr_die, struct probe_finder *pf)
 {
@@ -547,11 +555,11 @@ static int convert_variable(Dwarf_Die *vr_die, struct probe_finder *pf)
 
 	ret = convert_variable_location(vr_die, pf->addr, pf->fb_ops,
 					&pf->sp_die, pf->machine, pf->tvar);
+	if (ret == -ENOENT && pf->skip_empty_arg)
+		/* This can be found in other place. skip it */
+		return 0;
 	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"
-		       " Use -V with the --range option to show '%s' location range.\n",
-		       pf->pvar->var, pf->pvar->var);
+		print_var_not_found(pf->pvar->var);
 	} else if (ret == -ENOTSUP)
 		pr_err("Sorry, we don't support this variable location yet.\n");
 	else if (ret == 0 && pf->pvar->field) {
@@ -598,6 +606,8 @@ static int find_variable(Dwarf_Die *sc_die, struct probe_finder *pf)
 		/* Search again in global variables */
 		if (!die_find_variable_at(&pf->cu_die, pf->pvar->var,
 						0, &vr_die)) {
+			if (pf->skip_empty_arg)
+				return 0;
 			pr_warning("Failed to find '%s' in this function.\n",
 				   pf->pvar->var);
 			ret = -ENOENT;
@@ -1384,6 +1394,44 @@ static int add_probe_trace_event(Dwarf_Die *sc_die, struct probe_finder *pf)
 	return ret;
 }
 
+static int fill_empty_trace_arg(struct perf_probe_event *pev,
+				struct probe_trace_event *tevs, int ntevs)
+{
+	char **valp;
+	char *type;
+	int i, j, ret;
+
+	for (i = 0; i < pev->nargs; i++) {
+		type = NULL;
+		for (j = 0; j < ntevs; j++) {
+			if (tevs[j].args[i].value) {
+				type = tevs[j].args[i].type;
+				break;
+			}
+		}
+		if (j == ntevs) {
+			print_var_not_found(pev->args[i].var);
+			return -ENOENT;
+		}
+		for (j = 0; j < ntevs; j++) {
+			valp = &tevs[j].args[i].value;
+			if (*valp)
+				continue;
+
+			ret = asprintf(valp, "\\%lx", probe_conf.magic_num);
+			if (ret < 0)
+				return -ENOMEM;
+			/* Note that type can be NULL */
+			if (type) {
+				tevs[j].args[i].type = strdup(type);
+				if (!tevs[j].args[i].type)
+					return -ENOMEM;
+			}
+		}
+	}
+	return 0;
+}
+
 /* Find probe_trace_events specified by perf_probe_event from debuginfo */
 int debuginfo__find_trace_events(struct debuginfo *dbg,
 				 struct perf_probe_event *pev,
@@ -1402,7 +1450,13 @@ int debuginfo__find_trace_events(struct debuginfo *dbg,
 	tf.tevs = *tevs;
 	tf.ntevs = 0;
 
+	if (pev->nargs != 0 && immediate_value_is_supported())
+		tf.pf.skip_empty_arg = true;
+
 	ret = debuginfo__find_probes(dbg, &tf.pf);
+	if (ret >= 0 && tf.pf.skip_empty_arg)
+		ret = fill_empty_trace_arg(pev, tf.tevs, tf.ntevs);
+
 	if (ret < 0) {
 		for (i = 0; i < tf.ntevs; i++)
 			clear_probe_trace_event(&tf.tevs[i]);
diff --git a/tools/perf/util/probe-finder.h b/tools/perf/util/probe-finder.h
index 670c477bf8cf..11be10080613 100644
--- a/tools/perf/util/probe-finder.h
+++ b/tools/perf/util/probe-finder.h
@@ -87,6 +87,7 @@ struct probe_finder {
 	unsigned int		machine;	/* Target machine arch */
 	struct perf_probe_arg	*pvar;		/* Current target variable */
 	struct probe_trace_arg	*tvar;		/* Current result variable */
+	bool			skip_empty_arg;	/* Skip non-exist args */
 };
 
 struct trace_event_finder {


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

* Re: [PATCH v3 1/7] perf probe: Show correct statement line number by perf probe -l
  2019-11-18  8:11 ` [PATCH v3 1/7] perf probe: Show correct statement line number by perf probe -l Masami Hiramatsu
@ 2019-11-18 21:57   ` Arnaldo Carvalho de Melo
  2019-11-19 16:56   ` [tip: perf/core] " tip-bot2 for Masami Hiramatsu
  1 sibling, 0 replies; 22+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-11-18 21:57 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Ingo Molnar, Steven Rostedt, linux-kernel, Tom Zanussi,
	Ravi Bangoria, Namhyung Kim

Em Mon, Nov 18, 2019 at 05:11:50PM +0900, Masami Hiramatsu escreveu:
> The dwarf_getsrc_die() can return the line which is not a statement
> nor the least line number among the lines which shares same address.
> This can lead perf probe --list shows incorrect line number for
> probed address.
> To fix this, this introduces cu_getsrc_die() which returns only a
> statement line and which is the least line number (we call it
> the representive line for an address), and use it in cu_find_lineinfo().
> 
> Also, if the given address is the entry address of a real function,
> cu_find_lineinfo() returns the function declared line number instead
> of the start line number of the function body.
> 
> For example, without this change perf probe -l shows incorrect line
> as below.
> 
>   # perf probe -a kernel_read:2
>   Added new event:
>     probe:kernel_read    (on kernel_read:2)
> 
>   You can now use it in all perf tools, such as:
> 
>   	perf record -e probe:kernel_read -aR sleep 1
> 
>   # perf probe -l
>     probe:kernel_read    (on kernel_read:1@linux-5.0.0/fs/read_write.c)
> 
> With this fix, it shows correct line number as below;
> 
>   # perf probe -l
>     probe:kernel_read    (on kernel_read:2@linux-5.0.0/fs/read_write.c)

Thanks, this fixes the problem I reported, applied.

- Arnaldo
 
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
>  tools/perf/util/dwarf-aux.c |   62 ++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 58 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
> index 5544bfbd0f6c..aa898014ad12 100644
> --- a/tools/perf/util/dwarf-aux.c
> +++ b/tools/perf/util/dwarf-aux.c
> @@ -59,6 +59,51 @@ const char *cu_get_comp_dir(Dwarf_Die *cu_die)
>  	return dwarf_formstring(&attr);
>  }
>  
> +/* Unlike dwarf_getsrc_die(), cu_getsrc_die() only returns statement line */
> +static Dwarf_Line *cu_getsrc_die(Dwarf_Die *cu_die, Dwarf_Addr addr)
> +{
> +	Dwarf_Addr laddr;
> +	Dwarf_Lines *lines;
> +	Dwarf_Line *line;
> +	size_t nlines, l, u, n;
> +	bool flag;
> +
> +	if (dwarf_getsrclines(cu_die, &lines, &nlines) != 0 ||
> +	    nlines == 0)
> +		return NULL;
> +
> +	/* Lines are sorted by address, use binary search */
> +	l = 0; u = nlines - 1;
> +	while (l < u) {
> +		n = u - (u - l) / 2;
> +		line = dwarf_onesrcline(lines, n);
> +		if (!line || dwarf_lineaddr(line, &laddr) != 0)
> +			return NULL;
> +		if (addr < laddr)
> +			u = n - 1;
> +		else
> +			l = n;
> +	}
> +	/* Going backward to find the lowest line */
> +	do {
> +		line = dwarf_onesrcline(lines, --l);
> +		if (!line || dwarf_lineaddr(line, &laddr) != 0)
> +			return NULL;
> +	} while (laddr == addr);
> +	l++;
> +	/* Going foward to find the statement line */
> +	do {
> +		line = dwarf_onesrcline(lines, l++);
> +		if (!line || dwarf_lineaddr(line, &laddr) != 0 ||
> +		    dwarf_linebeginstatement(line, &flag) != 0)
> +			return NULL;
> +		if (laddr > addr)
> +			return NULL;
> +	} while (!flag);
> +
> +	return line;
> +}
> +
>  /**
>   * cu_find_lineinfo - Get a line number and file name for given address
>   * @cu_die: a CU DIE
> @@ -72,17 +117,26 @@ int cu_find_lineinfo(Dwarf_Die *cu_die, unsigned long addr,
>  		    const char **fname, int *lineno)
>  {
>  	Dwarf_Line *line;
> -	Dwarf_Addr laddr;
> +	Dwarf_Die die_mem;
> +	Dwarf_Addr faddr;
>  
> -	line = dwarf_getsrc_die(cu_die, (Dwarf_Addr)addr);
> -	if (line && dwarf_lineaddr(line, &laddr) == 0 &&
> -	    addr == (unsigned long)laddr && dwarf_lineno(line, lineno) == 0) {
> +	if (die_find_realfunc(cu_die, (Dwarf_Addr)addr, &die_mem)
> +	    && die_entrypc(&die_mem, &faddr) == 0 &&
> +	    faddr == addr) {
> +		*fname = dwarf_decl_file(&die_mem);
> +		dwarf_decl_line(&die_mem, lineno);
> +		goto out;
> +	}
> +
> +	line = cu_getsrc_die(cu_die, (Dwarf_Addr)addr);
> +	if (line && dwarf_lineno(line, lineno) == 0) {
>  		*fname = dwarf_linesrc(line, NULL, NULL);
>  		if (!*fname)
>  			/* line number is useless without filename */
>  			*lineno = 0;
>  	}
>  
> +out:
>  	return *lineno ?: -ENOENT;
>  }
>  

-- 

- Arnaldo

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

* Re: [PATCH v3 2/7] perf probe: Verify given line is a representive line
  2019-11-18  8:12 ` [PATCH v3 2/7] perf probe: Verify given line is a representive line Masami Hiramatsu
@ 2019-11-18 21:59   ` Arnaldo Carvalho de Melo
  2019-11-19 16:56   ` [tip: perf/core] " tip-bot2 for Masami Hiramatsu
  1 sibling, 0 replies; 22+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-11-18 21:59 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Ingo Molnar, Steven Rostedt, linux-kernel, Tom Zanussi,
	Ravi Bangoria, Namhyung Kim

Em Mon, Nov 18, 2019 at 05:12:00PM +0900, Masami Hiramatsu escreveu:
> Verify user given probe line is a representive line (which doesn't
> share the address with other lines or the line is the least line
> among the lines which shares same address), and if not, it shows
> what is the representive line.
> 
> Without this fix, user can put a probe on the lines which is not a
> a representive line. But since this is not a representive line,
> perf probe -l shows a representive line number instead of user given
> line number. e.g. (put kernel_read:3, but listed as kernel_read:2)
> 
>   # perf probe -a kernel_read:3
>   Added new event:
>     probe:kernel_read    (on kernel_read:3)
> 
>   You can now use it in all perf tools, such as:
> 
>   	perf record -e probe:kernel_read -aR sleep 1
> 
>   # perf probe -l
>     probe:kernel_read    (on kernel_read:2@linux-5.0.0/fs/read_write.c)
> 
> With this fix, perf probe doesn't allow user to put a probe on
> a representive line, and tell what is the representive line.
> 
>   # perf probe -a kernel_read:3
>   This line is sharing the addrees with other lines.
>   Please try to probe at kernel_read:2 instead.
>     Error: Failed to add events.

Thanks, this fixes the problem I reported, applied.

- Arnaldo
 
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
>  tools/perf/util/probe-finder.c |   36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index 9ecea45da4ca..ef1b320cedf8 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -776,6 +776,39 @@ static Dwarf_Die *find_best_scope(struct probe_finder *pf, Dwarf_Die *die_mem)
>  	return fsp.found ? die_mem : NULL;
>  }
>  
> +static int verify_representive_line(struct probe_finder *pf, const char *fname,
> +				int lineno, Dwarf_Addr addr)
> +{
> +	const char *__fname, *__func = NULL;
> +	Dwarf_Die die_mem;
> +	int __lineno;
> +
> +	/* Verify line number and address by reverse search */
> +	if (cu_find_lineinfo(&pf->cu_die, addr, &__fname, &__lineno) < 0)
> +		return 0;
> +
> +	pr_debug2("Reversed line: %s:%d\n", __fname, __lineno);
> +	if (strcmp(fname, __fname) || lineno == __lineno)
> +		return 0;
> +
> +	pr_warning("This line is sharing the addrees with other lines.\n");
> +
> +	if (pf->pev->point.function) {
> +		/* Find best match function name and lines */
> +		pf->addr = addr;
> +		if (find_best_scope(pf, &die_mem)
> +		    && die_match_name(&die_mem, pf->pev->point.function)
> +		    && dwarf_decl_line(&die_mem, &lineno) == 0) {
> +			__func = dwarf_diename(&die_mem);
> +			__lineno -= lineno;
> +		}
> +	}
> +	pr_warning("Please try to probe at %s:%d instead.\n",
> +		   __func ? : __fname, __lineno);
> +
> +	return -ENOENT;
> +}
> +
>  static int probe_point_line_walker(const char *fname, int lineno,
>  				   Dwarf_Addr addr, void *data)
>  {
> @@ -786,6 +819,9 @@ static int probe_point_line_walker(const char *fname, int lineno,
>  	if (lineno != pf->lno || strtailcmp(fname, pf->fname) != 0)
>  		return 0;
>  
> +	if (verify_representive_line(pf, fname, lineno, addr))
> +		return -ENOENT;
> +
>  	pf->addr = addr;
>  	sc_die = find_best_scope(pf, &die_mem);
>  	if (!sc_die) {

-- 

- Arnaldo

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

* Re: [PATCH v3 3/7] perf probe: Do not show non representive lines by perf-probe -L
  2019-11-18  8:12 ` [PATCH v3 3/7] perf probe: Do not show non representive lines by perf-probe -L Masami Hiramatsu
@ 2019-11-18 22:01   ` Arnaldo Carvalho de Melo
  2019-11-19 16:56   ` [tip: perf/core] " tip-bot2 for Masami Hiramatsu
  1 sibling, 0 replies; 22+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-11-18 22:01 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Ingo Molnar, Steven Rostedt, linux-kernel, Tom Zanussi,
	Ravi Bangoria, Namhyung Kim

Em Mon, Nov 18, 2019 at 05:12:10PM +0900, Masami Hiramatsu escreveu:
> Since perf probe -L shows non representive lines, it can be
> mislead users where user can put probes.
> This prevents to show such non representive lines so that user
> can understand which lines user can probe.
> 
>   # perf probe -L kernel_read
>   <kernel_read@/build/linux-pvZVvI/linux-5.0.0/fs/read_write.c:0>
>         0  ssize_t kernel_read(struct file *file, void *buf, size_t count, loff_t *pos)
>            {
>         2         mm_segment_t old_fs;
>                   ssize_t result;
> 
>                   old_fs = get_fs();
>         6         set_fs(get_ds());
>                   /* The cast to a user pointer is valid due to the set_fs() */
>         8         result = vfs_read(file, (void __user *)buf, count, pos);
>         9         set_fs(old_fs);
>        10         return result;
>            }
>            EXPORT_SYMBOL(kernel_read);

Thanks, this fixes the problem I reported, applied.

- Arnaldo
 
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
>  tools/perf/util/probe-finder.c |    7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index ef1b320cedf8..f12ad507a822 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -1734,12 +1734,19 @@ static int line_range_walk_cb(const char *fname, int lineno,
>  			      void *data)
>  {
>  	struct line_finder *lf = data;
> +	const char *__fname;
> +	int __lineno;
>  	int err;
>  
>  	if ((strtailcmp(fname, lf->fname) != 0) ||
>  	    (lf->lno_s > lineno || lf->lno_e < lineno))
>  		return 0;
>  
> +	/* Make sure this line can be reversable */
> +	if (cu_find_lineinfo(&lf->cu_die, addr, &__fname, &__lineno) > 0
> +	    && (lineno != __lineno || strcmp(fname, __fname)))
> +		return 0;
> +
>  	err = line_range_add_line(fname, lineno, lf->lr);
>  	if (err < 0 && err != -EEXIST)
>  		return err;

-- 

- Arnaldo

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

* Re: [PATCH v3 4/7] perf probe: Generate event name with line number
  2019-11-18  8:12 ` [PATCH v3 4/7] perf probe: Generate event name with line number Masami Hiramatsu
@ 2019-11-18 22:03   ` Arnaldo Carvalho de Melo
  2019-11-19 16:56   ` [tip: perf/core] " tip-bot2 for Masami Hiramatsu
  1 sibling, 0 replies; 22+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-11-18 22:03 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Ingo Molnar, Steven Rostedt, linux-kernel, Tom Zanussi,
	Ravi Bangoria, Namhyung Kim

Em Mon, Nov 18, 2019 at 05:12:20PM +0900, Masami Hiramatsu escreveu:
> Generate event name from function name with line number
> as <function>_L<line_number>. Note that this is only for
> the new event which is defined by the line number of
> function (except for line 0).
> 
> If there is another event on same line, you have to use
> "-f" option. In that case, the new event has "_1" suffix.
> 
>  e.g.
>   # perf probe -a kernel_read:2
>   Added new event:
>     probe:kernel_read_L2 (on kernel_read:2)
> 
>   You can now use it in all perf tools, such as:
> 
>   	perf record -e probe:kernel_read_L2 -aR sleep 1
> 
> 
> But if we omit the line number or 0th line, it will
> have no suffix.
> 
>   # perf probe -a kernel_read:0
>   Added new event:
>     probe:kernel_read (on kernel_read)
> 
>   You can now use it in all perf tools, such as:
> 
>   	perf record -e probe:kernel_read -aR sleep 1
> 
> # perf probe -l
>   probe:kernel_read    (on kernel_read@linux-5.0.0/fs/read_write.c)
>   probe:kernel_read_L2 (on kernel_read:2@linux-5.0.0/fs/read_write.c)

Thanks, tested and applied.

- Arnaldo

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

* Re: [PATCH v3 0/7] perf/probe: Support multiprobe and immediates with fixes
  2019-11-18  8:11 [PATCH v3 0/7] perf/probe: Support multiprobe and immediates with fixes Masami Hiramatsu
                   ` (6 preceding siblings ...)
  2019-11-18  8:12 ` [PATCH v3 7/7] perf probe: Trace a magic number if variable is not found Masami Hiramatsu
@ 2019-11-18 22:11 ` Arnaldo Carvalho de Melo
  2019-11-19 13:46   ` Masami Hiramatsu
  7 siblings, 1 reply; 22+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-11-18 22:11 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Ingo Molnar, Steven Rostedt, linux-kernel, Tom Zanussi,
	Ravi Bangoria, Namhyung Kim

Em Mon, Nov 18, 2019 at 05:11:40PM +0900, Masami Hiramatsu escreveu:
> Hi Arnaldo,
> 
> This is the 3rd version of the multiprobe support on perf probe
> including some fixes about "representive lines"
> 
> The previous thread is here:
> 
> https://lkml.kernel.org/r/157314406866.4063.16995747442215702109.stgit@devnote2
> 
> On the previous thread, we discussed some issues about incorrect line
> information shown by perf probe. I finally fixed those by introducing
> an idea of "representive line" -- a line which has a unique address
> (no other lines shares the same address) or a line which has the least
> line number among lines sharing same address. Now perf probe only shows
> such representive lines can be probed([1/7][3/7]), and if user tries to
> probe other non representive lines, it shows which line user should
> probe ([2/7]). The rest of patches in the series are same as v2 (except
> for [4/7], example output is updated)
> 
> This can be applied on top of perf/core.

Thanks, applied everything, only the two last patches I didn't test, the
kernel in this machine doesn't have the features it needs, we can fix
things if some problem lurks there.

- Arnaldo
 
> Thank you,
> 
> ---
> 
> Masami Hiramatsu (7):
>       perf probe: Show correct statement line number by perf probe -l
>       perf probe: Verify given line is a representive line
>       perf probe: Do not show non representive lines by perf-probe -L
>       perf probe: Generate event name with line number
>       perf probe: Support multiprobe event
>       perf probe: Support DW_AT_const_value constant value
>       perf probe: Trace a magic number if variable is not found
> 
> 
>  tools/perf/util/dwarf-aux.c    |   62 ++++++++++++++++++++-
>  tools/perf/util/probe-event.c  |   19 ++++++-
>  tools/perf/util/probe-event.h  |    3 +
>  tools/perf/util/probe-file.c   |   14 +++++
>  tools/perf/util/probe-file.h   |    2 +
>  tools/perf/util/probe-finder.c |  116 +++++++++++++++++++++++++++++++++++++++-
>  tools/perf/util/probe-finder.h |    1 
>  7 files changed, 206 insertions(+), 11 deletions(-)
> 
> --
> Masami Hiramatsu (Linaro) <mhiramat@kernel.org>

-- 

- Arnaldo

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

* Re: [PATCH v3 0/7] perf/probe: Support multiprobe and immediates with fixes
  2019-11-18 22:11 ` [PATCH v3 0/7] perf/probe: Support multiprobe and immediates with fixes Arnaldo Carvalho de Melo
@ 2019-11-19 13:46   ` Masami Hiramatsu
  2019-11-19 14:33     ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 22+ messages in thread
From: Masami Hiramatsu @ 2019-11-19 13:46 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ingo Molnar, Steven Rostedt, linux-kernel, Tom Zanussi,
	Ravi Bangoria, Namhyung Kim

On Mon, 18 Nov 2019 19:11:04 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Em Mon, Nov 18, 2019 at 05:11:40PM +0900, Masami Hiramatsu escreveu:
> > Hi Arnaldo,
> > 
> > This is the 3rd version of the multiprobe support on perf probe
> > including some fixes about "representive lines"
> > 
> > The previous thread is here:
> > 
> > https://lkml.kernel.org/r/157314406866.4063.16995747442215702109.stgit@devnote2
> > 
> > On the previous thread, we discussed some issues about incorrect line
> > information shown by perf probe. I finally fixed those by introducing
> > an idea of "representive line" -- a line which has a unique address
> > (no other lines shares the same address) or a line which has the least
> > line number among lines sharing same address. Now perf probe only shows
> > such representive lines can be probed([1/7][3/7]), and if user tries to
> > probe other non representive lines, it shows which line user should
> > probe ([2/7]). The rest of patches in the series are same as v2 (except
> > for [4/7], example output is updated)
> > 
> > This can be applied on top of perf/core.
> 
> Thanks, applied everything, only the two last patches I didn't test, the
> kernel in this machine doesn't have the features it needs, we can fix
> things if some problem lurks there.

Thank you Arnaldo! OK, if anything happens, I'll fix it soon.

Best Regards,
-- 
Masami Hiramatsu <mhiramat@kernel.org>

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

* Re: [PATCH v3 0/7] perf/probe: Support multiprobe and immediates with fixes
  2019-11-19 13:46   ` Masami Hiramatsu
@ 2019-11-19 14:33     ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 22+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-11-19 14:33 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Ingo Molnar, Steven Rostedt, linux-kernel, Tom Zanussi,
	Ravi Bangoria, Namhyung Kim

Em Tue, Nov 19, 2019 at 10:46:04PM +0900, Masami Hiramatsu escreveu:
> On Mon, 18 Nov 2019 19:11:04 -0300
> Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> 
> > Em Mon, Nov 18, 2019 at 05:11:40PM +0900, Masami Hiramatsu escreveu:
> > > Hi Arnaldo,
> > > 
> > > This is the 3rd version of the multiprobe support on perf probe
> > > including some fixes about "representive lines"
> > > 
> > > The previous thread is here:
> > > 
> > > https://lkml.kernel.org/r/157314406866.4063.16995747442215702109.stgit@devnote2
> > > 
> > > On the previous thread, we discussed some issues about incorrect line
> > > information shown by perf probe. I finally fixed those by introducing
> > > an idea of "representive line" -- a line which has a unique address
> > > (no other lines shares the same address) or a line which has the least
> > > line number among lines sharing same address. Now perf probe only shows
> > > such representive lines can be probed([1/7][3/7]), and if user tries to
> > > probe other non representive lines, it shows which line user should
> > > probe ([2/7]). The rest of patches in the series are same as v2 (except
> > > for [4/7], example output is updated)
> > > 
> > > This can be applied on top of perf/core.
> > 
> > Thanks, applied everything, only the two last patches I didn't test, the
> > kernel in this machine doesn't have the features it needs, we can fix
> > things if some problem lurks there.
> 
> Thank you Arnaldo! OK, if anything happens, I'll fix it soon.

np, Ingo already merged it, fast one, soon he'll push his tip/perf/core
branch out, thanks!

- Arnaldo

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

* [tip: perf/core] perf probe: Trace a magic number if variable is not found
  2019-11-18  8:12 ` [PATCH v3 7/7] perf probe: Trace a magic number if variable is not found Masami Hiramatsu
@ 2019-11-19 16:56   ` tip-bot2 for Masami Hiramatsu
  0 siblings, 0 replies; 22+ messages in thread
From: tip-bot2 for Masami Hiramatsu @ 2019-11-19 16:56 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Masami Hiramatsu, Namhyung Kim, Ravi Bangoria,
	Steven Rostedt (VMware),
	Tom Zanussi, Arnaldo Carvalho de Melo, Ingo Molnar,
	Borislav Petkov, linux-kernel

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     cb4027308570841869ec0c6bdafc9658c10f28a3
Gitweb:        https://git.kernel.org/tip/cb4027308570841869ec0c6bdafc9658c10f28a3
Author:        Masami Hiramatsu <mhiramat@kernel.org>
AuthorDate:    Mon, 18 Nov 2019 17:12:49 +09:00
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Mon, 18 Nov 2019 19:09:23 -03:00

perf probe: Trace a magic number if variable is not found

Trace a magic number as immediate value if the target variable is not
found at some probe points which is based on one probe event.

This feature is good for the case if you trace a source code line with
some local variables, which is compiled into several instructions and
some of the variables are optimized out on some instructions.

Even if so, with this feature, perf probe trace a magic number instead
of such disappeared variables and fold those probes on one event.

E.g. without this patch:

  # perf probe -D "pud_page_vaddr pud"
  Failed to find 'pud' in this function.
  Failed to find 'pud' in this function.
  Failed to find 'pud' in this function.
  Failed to find 'pud' in this function.
  Failed to find 'pud' in this function.
  Failed to find 'pud' in this function.
  Failed to find 'pud' in this function.
  Failed to find 'pud' in this function.
  Failed to find 'pud' in this function.
  Failed to find 'pud' in this function.
  Failed to find 'pud' in this function.
  Failed to find 'pud' in this function.
  Failed to find 'pud' in this function.
  Failed to find 'pud' in this function.
  Failed to find 'pud' in this function.
  Failed to find 'pud' in this function.
  p:probe/pud_page_vaddr _text+23480787 pud=%ax:x64
  p:probe/pud_page_vaddr _text+23808453 pud=%bp:x64
  p:probe/pud_page_vaddr _text+23558082 pud=%ax:x64
  p:probe/pud_page_vaddr _text+328373 pud=%r8:x64
  p:probe/pud_page_vaddr _text+348448 pud=%bx:x64
  p:probe/pud_page_vaddr _text+23816818 pud=%bx:x64

With this patch:

  # perf probe -D "pud_page_vaddr pud" | head
  spurious_kernel_fault is blacklisted function, skip it.
  vmalloc_fault is blacklisted function, skip it.
  p:probe/pud_page_vaddr _text+23480787 pud=%ax:x64
  p:probe/pud_page_vaddr _text+149051 pud=\deade12d:x64
  p:probe/pud_page_vaddr _text+23808453 pud=%bp:x64
  p:probe/pud_page_vaddr _text+315926 pud=\deade12d:x64
  p:probe/pud_page_vaddr _text+23807209 pud=\deade12d:x64
  p:probe/pud_page_vaddr _text+23557365 pud=%ax:x64
  p:probe/pud_page_vaddr _text+314097 pud=%di:x64
  p:probe/pud_page_vaddr _text+314015 pud=\deade12d:x64
  p:probe/pud_page_vaddr _text+313893 pud=\deade12d:x64
  p:probe/pud_page_vaddr _text+324083 pud=\deade12d:x64

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Cc: Steven Rostedt (VMware) <rostedt@goodmis.org>
Cc: Tom Zanussi <tom.zanussi@linux.intel.com>
Link: http://lore.kernel.org/lkml/157406476931.24476.6261475888681844285.stgit@devnote2
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-event.c  |  2 +-
 tools/perf/util/probe-event.h  |  3 ++-
 tools/perf/util/probe-finder.c | 62 ++++++++++++++++++++++++++++++---
 tools/perf/util/probe-finder.h |  1 +-
 4 files changed, 63 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 8f963a1..52b2d16 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -46,7 +46,7 @@
 #define PERFPROBE_GROUP "probe"
 
 bool probe_event_dry_run;	/* Dry run flag */
-struct probe_conf probe_conf;
+struct probe_conf probe_conf = { .magic_num = DEFAULT_PROBE_MAGIC_NUM };
 
 #define semantic_error(msg ...) pr_err("Semantic error :" msg)
 
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index 96a319c..4f0eb3a 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -16,10 +16,13 @@ struct probe_conf {
 	bool	no_inlines;
 	bool	cache;
 	int	max_probes;
+	unsigned long	magic_num;
 };
 extern struct probe_conf probe_conf;
 extern bool probe_event_dry_run;
 
+#define DEFAULT_PROBE_MAGIC_NUM	0xdeade12d	/* u32: 3735937325 */
+
 struct symbol;
 
 /* kprobe-tracer and uprobe-tracer tracing point */
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 33e9005..38d6cd2 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -536,6 +536,14 @@ next:
 		return 0;
 }
 
+static void print_var_not_found(const char *varname)
+{
+	pr_err("Failed to find the location of the '%s' variable at this address.\n"
+	       " Perhaps it has been optimized out.\n"
+	       " Use -V with the --range option to show '%s' location range.\n",
+		varname, varname);
+}
+
 /* Show a variables in kprobe event format */
 static int convert_variable(Dwarf_Die *vr_die, struct probe_finder *pf)
 {
@@ -547,11 +555,11 @@ static int convert_variable(Dwarf_Die *vr_die, struct probe_finder *pf)
 
 	ret = convert_variable_location(vr_die, pf->addr, pf->fb_ops,
 					&pf->sp_die, pf->machine, pf->tvar);
+	if (ret == -ENOENT && pf->skip_empty_arg)
+		/* This can be found in other place. skip it */
+		return 0;
 	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"
-		       " Use -V with the --range option to show '%s' location range.\n",
-		       pf->pvar->var, pf->pvar->var);
+		print_var_not_found(pf->pvar->var);
 	} else if (ret == -ENOTSUP)
 		pr_err("Sorry, we don't support this variable location yet.\n");
 	else if (ret == 0 && pf->pvar->field) {
@@ -598,6 +606,8 @@ static int find_variable(Dwarf_Die *sc_die, struct probe_finder *pf)
 		/* Search again in global variables */
 		if (!die_find_variable_at(&pf->cu_die, pf->pvar->var,
 						0, &vr_die)) {
+			if (pf->skip_empty_arg)
+				return 0;
 			pr_warning("Failed to find '%s' in this function.\n",
 				   pf->pvar->var);
 			ret = -ENOENT;
@@ -1384,6 +1394,44 @@ end:
 	return ret;
 }
 
+static int fill_empty_trace_arg(struct perf_probe_event *pev,
+				struct probe_trace_event *tevs, int ntevs)
+{
+	char **valp;
+	char *type;
+	int i, j, ret;
+
+	for (i = 0; i < pev->nargs; i++) {
+		type = NULL;
+		for (j = 0; j < ntevs; j++) {
+			if (tevs[j].args[i].value) {
+				type = tevs[j].args[i].type;
+				break;
+			}
+		}
+		if (j == ntevs) {
+			print_var_not_found(pev->args[i].var);
+			return -ENOENT;
+		}
+		for (j = 0; j < ntevs; j++) {
+			valp = &tevs[j].args[i].value;
+			if (*valp)
+				continue;
+
+			ret = asprintf(valp, "\\%lx", probe_conf.magic_num);
+			if (ret < 0)
+				return -ENOMEM;
+			/* Note that type can be NULL */
+			if (type) {
+				tevs[j].args[i].type = strdup(type);
+				if (!tevs[j].args[i].type)
+					return -ENOMEM;
+			}
+		}
+	}
+	return 0;
+}
+
 /* Find probe_trace_events specified by perf_probe_event from debuginfo */
 int debuginfo__find_trace_events(struct debuginfo *dbg,
 				 struct perf_probe_event *pev,
@@ -1402,7 +1450,13 @@ int debuginfo__find_trace_events(struct debuginfo *dbg,
 	tf.tevs = *tevs;
 	tf.ntevs = 0;
 
+	if (pev->nargs != 0 && immediate_value_is_supported())
+		tf.pf.skip_empty_arg = true;
+
 	ret = debuginfo__find_probes(dbg, &tf.pf);
+	if (ret >= 0 && tf.pf.skip_empty_arg)
+		ret = fill_empty_trace_arg(pev, tf.tevs, tf.ntevs);
+
 	if (ret < 0) {
 		for (i = 0; i < tf.ntevs; i++)
 			clear_probe_trace_event(&tf.tevs[i]);
diff --git a/tools/perf/util/probe-finder.h b/tools/perf/util/probe-finder.h
index 670c477..11be100 100644
--- a/tools/perf/util/probe-finder.h
+++ b/tools/perf/util/probe-finder.h
@@ -87,6 +87,7 @@ struct probe_finder {
 	unsigned int		machine;	/* Target machine arch */
 	struct perf_probe_arg	*pvar;		/* Current target variable */
 	struct probe_trace_arg	*tvar;		/* Current result variable */
+	bool			skip_empty_arg;	/* Skip non-exist args */
 };
 
 struct trace_event_finder {

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

* [tip: perf/core] perf probe: Do not show non representive lines by perf-probe -L
  2019-11-18  8:12 ` [PATCH v3 3/7] perf probe: Do not show non representive lines by perf-probe -L Masami Hiramatsu
  2019-11-18 22:01   ` Arnaldo Carvalho de Melo
@ 2019-11-19 16:56   ` tip-bot2 for Masami Hiramatsu
  1 sibling, 0 replies; 22+ messages in thread
From: tip-bot2 for Masami Hiramatsu @ 2019-11-19 16:56 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Masami Hiramatsu, Arnaldo Carvalho de Melo, Namhyung Kim,
	Ravi Bangoria, Steven Rostedt (VMware),
	Tom Zanussi, Ingo Molnar, Borislav Petkov, linux-kernel

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     499144c83d3b7e4f9e83916acfc97bbc3af891dc
Gitweb:        https://git.kernel.org/tip/499144c83d3b7e4f9e83916acfc97bbc3af891dc
Author:        Masami Hiramatsu <mhiramat@kernel.org>
AuthorDate:    Mon, 18 Nov 2019 17:12:10 +09:00
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Mon, 18 Nov 2019 18:59:36 -03:00

perf probe: Do not show non representive lines by perf-probe -L

Since perf probe -L shows non representive lines, it can be mislead
users where user can put probes.  This prevents to show such non
representive lines so that user can understand which lines user can
probe.

  # perf probe -L kernel_read
  <kernel_read@/build/linux-pvZVvI/linux-5.0.0/fs/read_write.c:0>
        0  ssize_t kernel_read(struct file *file, void *buf, size_t count, loff_t *pos)
           {
        2         mm_segment_t old_fs;
                  ssize_t result;

                  old_fs = get_fs();
        6         set_fs(get_ds());
                  /* The cast to a user pointer is valid due to the set_fs() */
        8         result = vfs_read(file, (void __user *)buf, count, pos);
        9         set_fs(old_fs);
       10         return result;
           }
           EXPORT_SYMBOL(kernel_read);

Committer testing:

Before:

  # perf probe -L kernel_read
  <kernel_read@/usr/src/debug/kernel-5.3.fc30/linux-5.3.8-200.fc30.x86_64/fs/read_write.c:0>
        0  ssize_t kernel_read(struct file *file, void *buf, size_t count, loff_t *pos)
        1  {
        2         mm_segment_t old_fs;
        3         ssize_t result;

        5         old_fs = get_fs();
        6         set_fs(KERNEL_DS);
                  /* The cast to a user pointer is valid due to the set_fs() */
        8         result = vfs_read(file, (void __user *)buf, count, pos);
        9         set_fs(old_fs);
       10         return result;
           }
           EXPORT_SYMBOL(kernel_read);
  #

See the 1, 3, 5 lines? They shouldn't be there, after this patch:

  # perf probe -L kernel_read
  <kernel_read@/usr/src/debug/kernel-5.3.fc30/linux-5.3.8-200.fc30.x86_64/fs/read_write.c:0>
        0  ssize_t kernel_read(struct file *file, void *buf, size_t count, loff_t *pos)
           {
        2         mm_segment_t old_fs;
                  ssize_t result;

                  old_fs = get_fs();
        6         set_fs(KERNEL_DS);
                  /* The cast to a user pointer is valid due to the set_fs() */
        8         result = vfs_read(file, (void __user *)buf, count, pos);
        9         set_fs(old_fs);
       10         return result;
           }
           EXPORT_SYMBOL(kernel_read);
  #

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Reported-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Cc: Steven Rostedt (VMware) <rostedt@goodmis.org>
Cc: Tom Zanussi <tom.zanussi@linux.intel.com>
Link: http://lore.kernel.org/lkml/157406473064.24476.2913278267727587314.stgit@devnote2
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-finder.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index ef1b320..f12ad50 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -1734,12 +1734,19 @@ static int line_range_walk_cb(const char *fname, int lineno,
 			      void *data)
 {
 	struct line_finder *lf = data;
+	const char *__fname;
+	int __lineno;
 	int err;
 
 	if ((strtailcmp(fname, lf->fname) != 0) ||
 	    (lf->lno_s > lineno || lf->lno_e < lineno))
 		return 0;
 
+	/* Make sure this line can be reversable */
+	if (cu_find_lineinfo(&lf->cu_die, addr, &__fname, &__lineno) > 0
+	    && (lineno != __lineno || strcmp(fname, __fname)))
+		return 0;
+
 	err = line_range_add_line(fname, lineno, lf->lr);
 	if (err < 0 && err != -EEXIST)
 		return err;

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

* [tip: perf/core] perf probe: Support multiprobe event
  2019-11-18  8:12 ` [PATCH v3 5/7] perf probe: Support multiprobe event Masami Hiramatsu
@ 2019-11-19 16:56   ` tip-bot2 for Masami Hiramatsu
  0 siblings, 0 replies; 22+ messages in thread
From: tip-bot2 for Masami Hiramatsu @ 2019-11-19 16:56 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Masami Hiramatsu, Arnaldo Carvalho de Melo, Namhyung Kim,
	Ravi Bangoria, Steven Rostedt (VMware),
	Tom Zanussi, Ingo Molnar, Borislav Petkov, linux-kernel

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     72363540c009db5014252a1a15e149d30f88bcc3
Gitweb:        https://git.kernel.org/tip/72363540c009db5014252a1a15e149d30f88bcc3
Author:        Masami Hiramatsu <mhiramat@kernel.org>
AuthorDate:    Mon, 18 Nov 2019 17:12:30 +09:00
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Mon, 18 Nov 2019 19:03:38 -03:00

perf probe: Support multiprobe event

Support multiprobe event if the event is based on function and lines and
kernel supports it. In this case, perf probe creates the first probe
with an event, and tries to append following probes on that event, since
those probes must be on the same source code line.

Before this patch;

  # perf probe -a vfs_read:18
  Added new events:
    probe:vfs_read_L18   (on vfs_read:18)
    probe:vfs_read_L18_1 (on vfs_read:18)

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

  	perf record -e probe:vfs_read_L18_1 -aR sleep 1

  #

After this patch (on multiprobe supported kernel)
  # perf probe -a vfs_read:18
  Added new events:
    probe:vfs_read_L18   (on vfs_read:18)
    probe:vfs_read_L18   (on vfs_read:18)

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

  	perf record -e probe:vfs_read_L18 -aR sleep 1

  #

Committer testing:

On a kernel that doesn't support multiprobe events, after this patch:

  # uname -a
  Linux quaco 5.3.8-200.fc30.x86_64 #1 SMP Tue Oct 29 14:46:22 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
  # grep append /sys/kernel/debug/tracing/README
  	    be modified by appending '.descending' or '.ascending' to a
  	    can be modified by appending any of the following modifiers
  #
  # perf probe -a vfs_read:18
  Added new events:
    probe:vfs_read_L18   (on vfs_read:18)
    probe:vfs_read_L18_1 (on vfs_read:18)

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

  	perf record -e probe:vfs_read_L18_1 -aR sleep 1

  # perf probe -l
    probe:vfs_read_L18   (on vfs_read:18@fs/read_write.c)
    probe:vfs_read_L18_1 (on vfs_read:18@fs/read_write.c)
  #

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Cc: Steven Rostedt (VMware) <rostedt@goodmis.org>
Cc: Tom Zanussi <tom.zanussi@linux.intel.com>
Link: http://lore.kernel.org/lkml/157406475010.24476.586290752591512351.stgit@devnote2
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-event.c |  9 +++++++--
 tools/perf/util/probe-file.c  |  7 +++++++
 tools/perf/util/probe-file.h  |  1 +
 3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 5c86d2c..8f963a1 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2738,8 +2738,13 @@ static int probe_trace_event__set_name(struct probe_trace_event *tev,
 	if (tev->event == NULL || tev->group == NULL)
 		return -ENOMEM;
 
-	/* Add added event name to namelist */
-	strlist__add(namelist, event);
+	/*
+	 * Add new event name to namelist if multiprobe event is NOT
+	 * supported, since we have to use new event name for following
+	 * probes in that case.
+	 */
+	if (!multiprobe_event_is_supported())
+		strlist__add(namelist, event);
 	return 0;
 }
 
diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index b659466..a63f1a1 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -1007,6 +1007,7 @@ enum ftrace_readme {
 	FTRACE_README_KRETPROBE_OFFSET,
 	FTRACE_README_UPROBE_REF_CTR,
 	FTRACE_README_USER_ACCESS,
+	FTRACE_README_MULTIPROBE_EVENT,
 	FTRACE_README_END,
 };
 
@@ -1020,6 +1021,7 @@ static struct {
 	DEFINE_TYPE(FTRACE_README_KRETPROBE_OFFSET, "*place (kretprobe): *"),
 	DEFINE_TYPE(FTRACE_README_UPROBE_REF_CTR, "*ref_ctr_offset*"),
 	DEFINE_TYPE(FTRACE_README_USER_ACCESS, "*[u]<offset>*"),
+	DEFINE_TYPE(FTRACE_README_MULTIPROBE_EVENT, "*Create/append/*"),
 };
 
 static bool scan_ftrace_readme(enum ftrace_readme type)
@@ -1085,3 +1087,8 @@ bool user_access_is_supported(void)
 {
 	return scan_ftrace_readme(FTRACE_README_USER_ACCESS);
 }
+
+bool multiprobe_event_is_supported(void)
+{
+	return scan_ftrace_readme(FTRACE_README_MULTIPROBE_EVENT);
+}
diff --git a/tools/perf/util/probe-file.h b/tools/perf/util/probe-file.h
index 986c1c9..850d1b5 100644
--- a/tools/perf/util/probe-file.h
+++ b/tools/perf/util/probe-file.h
@@ -71,6 +71,7 @@ bool probe_type_is_available(enum probe_type type);
 bool kretprobe_offset_is_supported(void);
 bool uprobe_ref_ctr_is_supported(void);
 bool user_access_is_supported(void);
+bool multiprobe_event_is_supported(void);
 #else	/* ! HAVE_LIBELF_SUPPORT */
 static inline struct probe_cache *probe_cache__new(const char *tgt __maybe_unused, struct nsinfo *nsi __maybe_unused)
 {

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

* [tip: perf/core] perf probe: Generate event name with line number
  2019-11-18  8:12 ` [PATCH v3 4/7] perf probe: Generate event name with line number Masami Hiramatsu
  2019-11-18 22:03   ` Arnaldo Carvalho de Melo
@ 2019-11-19 16:56   ` tip-bot2 for Masami Hiramatsu
  1 sibling, 0 replies; 22+ messages in thread
From: tip-bot2 for Masami Hiramatsu @ 2019-11-19 16:56 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Masami Hiramatsu, Arnaldo Carvalho de Melo, Namhyung Kim,
	Ravi Bangoria, Steven Rostedt (VMware),
	Tom Zanussi, Ingo Molnar, Borislav Petkov, linux-kernel

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     15354d54698648e20454fc8f298a5b18b6debea7
Gitweb:        https://git.kernel.org/tip/15354d54698648e20454fc8f298a5b18b6debea7
Author:        Masami Hiramatsu <mhiramat@kernel.org>
AuthorDate:    Mon, 18 Nov 2019 17:12:20 +09:00
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Mon, 18 Nov 2019 19:02:00 -03:00

perf probe: Generate event name with line number

Generate event name from function name with line number as
<function>_L<line_number>. Note that this is only for the new event
which is defined by the line number of function (except for line 0).

If there is another event on same line, you have to use
"-f" option. In that case, the new event has "_1" suffix.

 e.g.
  # perf probe -a kernel_read:2
  Added new event:
    probe:kernel_read_L2 (on kernel_read:2)

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

  	perf record -e probe:kernel_read_L2 -aR sleep 1

But if we omit the line number or 0th line, it will
have no suffix.

  # perf probe -a kernel_read:0
  Added new event:
    probe:kernel_read (on kernel_read)

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

  	perf record -e probe:kernel_read -aR sleep 1

  probe:kernel_read    (on kernel_read@linux-5.0.0/fs/read_write.c)
  probe:kernel_read_L2 (on kernel_read:2@linux-5.0.0/fs/read_write.c)

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Cc: Steven Rostedt (VMware) <rostedt@goodmis.org>
Cc: Tom Zanussi <tom.zanussi@linux.intel.com>
Link: http://lore.kernel.org/lkml/157406474026.24476.2828897745502059569.stgit@devnote2
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-event.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index e29948b..5c86d2c 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -1679,6 +1679,14 @@ int parse_perf_probe_command(const char *cmd, struct perf_probe_event *pev)
 	if (ret < 0)
 		goto out;
 
+	/* Generate event name if needed */
+	if (!pev->event && pev->point.function && pev->point.line
+			&& !pev->point.lazy_line && !pev->point.offset) {
+		if (asprintf(&pev->event, "%s_L%d", pev->point.function,
+			pev->point.line) < 0)
+			return -ENOMEM;
+	}
+
 	/* Copy arguments and ensure return probe has no C argument */
 	pev->nargs = argc - 1;
 	pev->args = zalloc(sizeof(struct perf_probe_arg) * pev->nargs);

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

* [tip: perf/core] perf probe: Support DW_AT_const_value constant value
  2019-11-18  8:12 ` [PATCH v3 6/7] perf probe: Support DW_AT_const_value constant value Masami Hiramatsu
@ 2019-11-19 16:56   ` tip-bot2 for Masami Hiramatsu
  0 siblings, 0 replies; 22+ messages in thread
From: tip-bot2 for Masami Hiramatsu @ 2019-11-19 16:56 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Masami Hiramatsu, Namhyung Kim, Ravi Bangoria,
	Steven Rostedt (VMware),
	Tom Zanussi, Arnaldo Carvalho de Melo, Ingo Molnar,
	Borislav Petkov, linux-kernel

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     66f69b2197167cb99330c77a550da50f1f597abc
Gitweb:        https://git.kernel.org/tip/66f69b2197167cb99330c77a550da50f1f597abc
Author:        Masami Hiramatsu <mhiramat@kernel.org>
AuthorDate:    Mon, 18 Nov 2019 17:12:40 +09:00
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Mon, 18 Nov 2019 19:08:02 -03:00

perf probe: Support DW_AT_const_value constant value

Support DW_AT_const_value for variable assignment instead of location.
Note that this requires ftrace supporting immediate value.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Cc: Steven Rostedt (VMware) <rostedt@goodmis.org>
Cc: Tom Zanussi <tom.zanussi@linux.intel.com>
Link: http://lore.kernel.org/lkml/157406476012.24476.16096289871757175775.stgit@devnote2
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-file.c   |  7 +++++++
 tools/perf/util/probe-file.h   |  1 +
 tools/perf/util/probe-finder.c | 11 +++++++++++
 3 files changed, 19 insertions(+)

diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index a63f1a1..5003ba4 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -1008,6 +1008,7 @@ enum ftrace_readme {
 	FTRACE_README_UPROBE_REF_CTR,
 	FTRACE_README_USER_ACCESS,
 	FTRACE_README_MULTIPROBE_EVENT,
+	FTRACE_README_IMMEDIATE_VALUE,
 	FTRACE_README_END,
 };
 
@@ -1022,6 +1023,7 @@ static struct {
 	DEFINE_TYPE(FTRACE_README_UPROBE_REF_CTR, "*ref_ctr_offset*"),
 	DEFINE_TYPE(FTRACE_README_USER_ACCESS, "*[u]<offset>*"),
 	DEFINE_TYPE(FTRACE_README_MULTIPROBE_EVENT, "*Create/append/*"),
+	DEFINE_TYPE(FTRACE_README_IMMEDIATE_VALUE, "*\\imm-value,*"),
 };
 
 static bool scan_ftrace_readme(enum ftrace_readme type)
@@ -1092,3 +1094,8 @@ bool multiprobe_event_is_supported(void)
 {
 	return scan_ftrace_readme(FTRACE_README_MULTIPROBE_EVENT);
 }
+
+bool immediate_value_is_supported(void)
+{
+	return scan_ftrace_readme(FTRACE_README_IMMEDIATE_VALUE);
+}
diff --git a/tools/perf/util/probe-file.h b/tools/perf/util/probe-file.h
index 850d1b5..0dba88c 100644
--- a/tools/perf/util/probe-file.h
+++ b/tools/perf/util/probe-file.h
@@ -72,6 +72,7 @@ bool kretprobe_offset_is_supported(void);
 bool uprobe_ref_ctr_is_supported(void);
 bool user_access_is_supported(void);
 bool multiprobe_event_is_supported(void);
+bool immediate_value_is_supported(void);
 #else	/* ! HAVE_LIBELF_SUPPORT */
 static inline struct probe_cache *probe_cache__new(const char *tgt __maybe_unused, struct nsinfo *nsi __maybe_unused)
 {
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index f12ad50..33e9005 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -177,6 +177,17 @@ static int convert_variable_location(Dwarf_Die *vr_die, Dwarf_Addr addr,
 	if (dwarf_attr(vr_die, DW_AT_external, &attr) != NULL)
 		goto static_var;
 
+	/* Constant value */
+	if (dwarf_attr(vr_die, DW_AT_const_value, &attr) &&
+	    immediate_value_is_supported()) {
+		Dwarf_Sword snum;
+
+		dwarf_formsdata(&attr, &snum);
+		ret = asprintf(&tvar->value, "\\%ld", (long)snum);
+
+		return ret < 0 ? -ENOMEM : 0;
+	}
+
 	/* TODO: handle more than 1 exprs */
 	if (dwarf_attr(vr_die, DW_AT_location, &attr) == NULL)
 		return -EINVAL;	/* Broken DIE ? */

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

* [tip: perf/core] perf probe: Show correct statement line number by perf probe -l
  2019-11-18  8:11 ` [PATCH v3 1/7] perf probe: Show correct statement line number by perf probe -l Masami Hiramatsu
  2019-11-18 21:57   ` Arnaldo Carvalho de Melo
@ 2019-11-19 16:56   ` tip-bot2 for Masami Hiramatsu
  1 sibling, 0 replies; 22+ messages in thread
From: tip-bot2 for Masami Hiramatsu @ 2019-11-19 16:56 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Arnaldo Carvalho de Melo, Masami Hiramatsu, Namhyung Kim,
	Ravi Bangoria, Steven Rostedt (VMware),
	Tom Zanussi, Ingo Molnar, Borislav Petkov, linux-kernel

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     57f95bf5f88295612871c36cb0de5069e50570f8
Gitweb:        https://git.kernel.org/tip/57f95bf5f88295612871c36cb0de5069e50570f8
Author:        Masami Hiramatsu <mhiramat@kernel.org>
AuthorDate:    Mon, 18 Nov 2019 17:11:50 +09:00
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Mon, 18 Nov 2019 18:56:27 -03:00

perf probe: Show correct statement line number by perf probe -l

The dwarf_getsrc_die() can return the line which is not a statement nor
the least line number among the lines which shares same address.

This can lead perf probe --list shows incorrect line number for probed
address.

To fix this, this introduces cu_getsrc_die() which returns only a
statement line and which is the least line number (we call it the
representive line for an address), and use it in cu_find_lineinfo().

Also, if the given address is the entry address of a real function,
cu_find_lineinfo() returns the function declared line number instead of
the start line number of the function body.

For example, without this change perf probe -l shows incorrect line as
below.

  # perf probe -a kernel_read:2
  Added new event:
    probe:kernel_read    (on kernel_read:2)

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

  	perf record -e probe:kernel_read -aR sleep 1

  # perf probe -l
    probe:kernel_read    (on kernel_read:1@linux-5.0.0/fs/read_write.c)

With this fix, it shows correct line number as below;

  # perf probe -l
    probe:kernel_read    (on kernel_read:2@linux-5.0.0/fs/read_write.c)

Reported-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Cc: Steven Rostedt (VMware) <rostedt@goodmis.org>
Cc: Tom Zanussi <tom.zanussi@linux.intel.com>
Link: http://lore.kernel.org/lkml/157406471067.24476.17463149618465494448.stgit@devnote2
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/dwarf-aux.c | 62 +++++++++++++++++++++++++++++++++---
 1 file changed, 58 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index 5544bfb..aa89801 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -59,6 +59,51 @@ const char *cu_get_comp_dir(Dwarf_Die *cu_die)
 	return dwarf_formstring(&attr);
 }
 
+/* Unlike dwarf_getsrc_die(), cu_getsrc_die() only returns statement line */
+static Dwarf_Line *cu_getsrc_die(Dwarf_Die *cu_die, Dwarf_Addr addr)
+{
+	Dwarf_Addr laddr;
+	Dwarf_Lines *lines;
+	Dwarf_Line *line;
+	size_t nlines, l, u, n;
+	bool flag;
+
+	if (dwarf_getsrclines(cu_die, &lines, &nlines) != 0 ||
+	    nlines == 0)
+		return NULL;
+
+	/* Lines are sorted by address, use binary search */
+	l = 0; u = nlines - 1;
+	while (l < u) {
+		n = u - (u - l) / 2;
+		line = dwarf_onesrcline(lines, n);
+		if (!line || dwarf_lineaddr(line, &laddr) != 0)
+			return NULL;
+		if (addr < laddr)
+			u = n - 1;
+		else
+			l = n;
+	}
+	/* Going backward to find the lowest line */
+	do {
+		line = dwarf_onesrcline(lines, --l);
+		if (!line || dwarf_lineaddr(line, &laddr) != 0)
+			return NULL;
+	} while (laddr == addr);
+	l++;
+	/* Going foward to find the statement line */
+	do {
+		line = dwarf_onesrcline(lines, l++);
+		if (!line || dwarf_lineaddr(line, &laddr) != 0 ||
+		    dwarf_linebeginstatement(line, &flag) != 0)
+			return NULL;
+		if (laddr > addr)
+			return NULL;
+	} while (!flag);
+
+	return line;
+}
+
 /**
  * cu_find_lineinfo - Get a line number and file name for given address
  * @cu_die: a CU DIE
@@ -72,17 +117,26 @@ int cu_find_lineinfo(Dwarf_Die *cu_die, unsigned long addr,
 		    const char **fname, int *lineno)
 {
 	Dwarf_Line *line;
-	Dwarf_Addr laddr;
+	Dwarf_Die die_mem;
+	Dwarf_Addr faddr;
 
-	line = dwarf_getsrc_die(cu_die, (Dwarf_Addr)addr);
-	if (line && dwarf_lineaddr(line, &laddr) == 0 &&
-	    addr == (unsigned long)laddr && dwarf_lineno(line, lineno) == 0) {
+	if (die_find_realfunc(cu_die, (Dwarf_Addr)addr, &die_mem)
+	    && die_entrypc(&die_mem, &faddr) == 0 &&
+	    faddr == addr) {
+		*fname = dwarf_decl_file(&die_mem);
+		dwarf_decl_line(&die_mem, lineno);
+		goto out;
+	}
+
+	line = cu_getsrc_die(cu_die, (Dwarf_Addr)addr);
+	if (line && dwarf_lineno(line, lineno) == 0) {
 		*fname = dwarf_linesrc(line, NULL, NULL);
 		if (!*fname)
 			/* line number is useless without filename */
 			*lineno = 0;
 	}
 
+out:
 	return *lineno ?: -ENOENT;
 }
 

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

* [tip: perf/core] perf probe: Verify given line is a representive line
  2019-11-18  8:12 ` [PATCH v3 2/7] perf probe: Verify given line is a representive line Masami Hiramatsu
  2019-11-18 21:59   ` Arnaldo Carvalho de Melo
@ 2019-11-19 16:56   ` tip-bot2 for Masami Hiramatsu
  1 sibling, 0 replies; 22+ messages in thread
From: tip-bot2 for Masami Hiramatsu @ 2019-11-19 16:56 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Masami Hiramatsu, Arnaldo Carvalho de Melo, Namhyung Kim,
	Ravi Bangoria, Steven Rostedt (VMware),
	Tom Zanussi, Ingo Molnar, Borislav Petkov, linux-kernel

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     1ae5d88a4eefacd4a3643170c20cf6403a24d254
Gitweb:        https://git.kernel.org/tip/1ae5d88a4eefacd4a3643170c20cf6403a24d254
Author:        Masami Hiramatsu <mhiramat@kernel.org>
AuthorDate:    Mon, 18 Nov 2019 17:12:00 +09:00
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Mon, 18 Nov 2019 18:58:25 -03:00

perf probe: Verify given line is a representive line

Verify user given probe line is a representive line (which doesn't share
the address with other lines or the line is the least line among the
lines which shares same address), and if not, it shows what is the
representive line.

Without this fix, user can put a probe on the lines which is not a a
representive line. But since this is not a representive line, perf probe
-l shows a representive line number instead of user given line number.
e.g. (put kernel_read:3, but listed as kernel_read:2)

  # perf probe -a kernel_read:3
  Added new event:
    probe:kernel_read    (on kernel_read:3)

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

  	perf record -e probe:kernel_read -aR sleep 1

  # perf probe -l
    probe:kernel_read    (on kernel_read:2@linux-5.0.0/fs/read_write.c)

With this fix, perf probe doesn't allow user to put a probe on a
representive line, and tell what is the representive line.

  # perf probe -a kernel_read:3
  This line is sharing the addrees with other lines.
  Please try to probe at kernel_read:2 instead.
    Error: Failed to add events.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Reported-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Cc: Steven Rostedt (VMware) <rostedt@goodmis.org>
Cc: Tom Zanussi <tom.zanussi@linux.intel.com>
Link: http://lore.kernel.org/lkml/157406472071.24476.14915451439785001021.stgit@devnote2
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-finder.c | 36 +++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+)

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 9ecea45..ef1b320 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -776,6 +776,39 @@ static Dwarf_Die *find_best_scope(struct probe_finder *pf, Dwarf_Die *die_mem)
 	return fsp.found ? die_mem : NULL;
 }
 
+static int verify_representive_line(struct probe_finder *pf, const char *fname,
+				int lineno, Dwarf_Addr addr)
+{
+	const char *__fname, *__func = NULL;
+	Dwarf_Die die_mem;
+	int __lineno;
+
+	/* Verify line number and address by reverse search */
+	if (cu_find_lineinfo(&pf->cu_die, addr, &__fname, &__lineno) < 0)
+		return 0;
+
+	pr_debug2("Reversed line: %s:%d\n", __fname, __lineno);
+	if (strcmp(fname, __fname) || lineno == __lineno)
+		return 0;
+
+	pr_warning("This line is sharing the addrees with other lines.\n");
+
+	if (pf->pev->point.function) {
+		/* Find best match function name and lines */
+		pf->addr = addr;
+		if (find_best_scope(pf, &die_mem)
+		    && die_match_name(&die_mem, pf->pev->point.function)
+		    && dwarf_decl_line(&die_mem, &lineno) == 0) {
+			__func = dwarf_diename(&die_mem);
+			__lineno -= lineno;
+		}
+	}
+	pr_warning("Please try to probe at %s:%d instead.\n",
+		   __func ? : __fname, __lineno);
+
+	return -ENOENT;
+}
+
 static int probe_point_line_walker(const char *fname, int lineno,
 				   Dwarf_Addr addr, void *data)
 {
@@ -786,6 +819,9 @@ static int probe_point_line_walker(const char *fname, int lineno,
 	if (lineno != pf->lno || strtailcmp(fname, pf->fname) != 0)
 		return 0;
 
+	if (verify_representive_line(pf, fname, lineno, addr))
+		return -ENOENT;
+
 	pf->addr = addr;
 	sc_die = find_best_scope(pf, &die_mem);
 	if (!sc_die) {

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

end of thread, other threads:[~2019-11-19 16:58 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-18  8:11 [PATCH v3 0/7] perf/probe: Support multiprobe and immediates with fixes Masami Hiramatsu
2019-11-18  8:11 ` [PATCH v3 1/7] perf probe: Show correct statement line number by perf probe -l Masami Hiramatsu
2019-11-18 21:57   ` Arnaldo Carvalho de Melo
2019-11-19 16:56   ` [tip: perf/core] " tip-bot2 for Masami Hiramatsu
2019-11-18  8:12 ` [PATCH v3 2/7] perf probe: Verify given line is a representive line Masami Hiramatsu
2019-11-18 21:59   ` Arnaldo Carvalho de Melo
2019-11-19 16:56   ` [tip: perf/core] " tip-bot2 for Masami Hiramatsu
2019-11-18  8:12 ` [PATCH v3 3/7] perf probe: Do not show non representive lines by perf-probe -L Masami Hiramatsu
2019-11-18 22:01   ` Arnaldo Carvalho de Melo
2019-11-19 16:56   ` [tip: perf/core] " tip-bot2 for Masami Hiramatsu
2019-11-18  8:12 ` [PATCH v3 4/7] perf probe: Generate event name with line number Masami Hiramatsu
2019-11-18 22:03   ` Arnaldo Carvalho de Melo
2019-11-19 16:56   ` [tip: perf/core] " tip-bot2 for Masami Hiramatsu
2019-11-18  8:12 ` [PATCH v3 5/7] perf probe: Support multiprobe event Masami Hiramatsu
2019-11-19 16:56   ` [tip: perf/core] " tip-bot2 for Masami Hiramatsu
2019-11-18  8:12 ` [PATCH v3 6/7] perf probe: Support DW_AT_const_value constant value Masami Hiramatsu
2019-11-19 16:56   ` [tip: perf/core] " tip-bot2 for Masami Hiramatsu
2019-11-18  8:12 ` [PATCH v3 7/7] perf probe: Trace a magic number if variable is not found Masami Hiramatsu
2019-11-19 16:56   ` [tip: perf/core] " tip-bot2 for Masami Hiramatsu
2019-11-18 22:11 ` [PATCH v3 0/7] perf/probe: Support multiprobe and immediates with fixes Arnaldo Carvalho de Melo
2019-11-19 13:46   ` Masami Hiramatsu
2019-11-19 14:33     ` Arnaldo Carvalho de Melo

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).