[PATCH 0/5] perf/probe: Support multiprobe and immediates

[PATCH 0/5] perf/probe: Support multiprobe and immediates

[Masami Hiramatsu]

This series adds multiprobe support on perf probe command.

This can be applied on top of my previous (sent last week)
bugfix series,

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

Inlined functions or the lines which have multiple statements can
be compiled in multiple addresses. Current perf probe generates
different events for each address, but this is not useful for
users.

Since ftrace multiprobe per event support is on ftrace/for-next,
it is a time to push this series. To support multi-probe, this
adds below 4 patches + 1 minor fix.

 - Fix a rare case bug for missing scope-DIE in some inlined
   functions.
 - Modify the naming scheme of probe event to Line number based
   instead of function name based.
 - Add multi-probe support on perf probe. This also converts
   different register assignment on local variables for each
   probe point.
 - Add const value attribute support. It is possible that the
   compiler assigns a constant value for optimized function
   parameter or local variables. This handles such case.
 - Introduce a magic number assignment to non-exist local variable.
   Inlined functions can be embedded (inlined) in caller function
   and the parameter or local vars in such inlined function can be
   optimized out some place, but in some place we still can see
   those vars. For supporting multiprobe correctly, we need to
   fill out some value even if we can not find the given local vars.
   So this fills it with "0xdeade12d(dead end)" magic number.


Thank you,

---

Masami Hiramatsu (5):
      perf probe: Return a better scope DIE if there is no best scope
      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/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 |   90 ++++++++++++++++++++++++++++++++++++++--
 tools/perf/util/probe-finder.h |    1 
 6 files changed, 121 insertions(+), 8 deletions(-)

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

[PATCH 1/5] perf probe: Return a better scope DIE if there is no best scope

[Masami Hiramatsu]

Make find_best_scope() returns innermost DIE at given address if
there is no best matched scope DIE. Since Gcc sometimes generates
intuitively strange line info which is out of inlined function
address range, we need this fixup.

Without this, sometimes perf probe failed to probe on a line
inside an inlined function.
  # perf probe -D ksys_open:3
  Failed to find scope of probe point.
    Error: Failed to add events.

With this fix, perf probe can probe it.
  # perf probe -D ksys_open:3
  p:probe/ksys_open _text+25707308
  p:probe/ksys_open_1 _text+25710596
  p:probe/ksys_open_2 _text+25711114
  p:probe/ksys_open_3 _text+25711343
  p:probe/ksys_open_4 _text+25714058
  p:probe/ksys_open_5 _text+2819653
  p:probe/ksys_open_6 _text+2819701

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

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index f441e0174334..9ecea45da4ca 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -744,6 +744,16 @@ static int find_best_scope_cb(Dwarf_Die *fn_die, void *data)
 	return 0;
 }
 
+/* Return innermost DIE */
+static int find_inner_scope_cb(Dwarf_Die *fn_die, void *data)
+{
+	struct find_scope_param *fsp = data;
+
+	memcpy(fsp->die_mem, fn_die, sizeof(Dwarf_Die));
+	fsp->found = true;
+	return 1;
+}
+
 /* Find an appropriate scope fits to given conditions */
 static Dwarf_Die *find_best_scope(struct probe_finder *pf, Dwarf_Die *die_mem)
 {
@@ -755,8 +765,13 @@ static Dwarf_Die *find_best_scope(struct probe_finder *pf, Dwarf_Die *die_mem)
 		.die_mem = die_mem,
 		.found = false,
 	};
+	int ret;
 
-	cu_walk_functions_at(&pf->cu_die, pf->addr, find_best_scope_cb, &fsp);
+	ret = cu_walk_functions_at(&pf->cu_die, pf->addr, find_best_scope_cb,
+				   &fsp);
+	if (!ret && !fsp.found)
+		cu_walk_functions_at(&pf->cu_die, pf->addr,
+				     find_inner_scope_cb, &fsp);
 
 	return fsp.found ? die_mem : NULL;
 }

[PATCH 2/5] perf probe: Generate event name with line number

[Masami Hiramatsu]

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 function and lines.

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
  Added new event:
    probe:kernel_read_L0 (on kernel_read)

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

  	perf record -e probe:kernel_read_L0 -aR sleep 1

  # perf probe -a kernel_read:1
  Added new events:
    probe:kernel_read_L1 (on kernel_read:1)

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

  	perf record -e probe:kernel_read_L1_1 -aR sleep 1

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

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 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 91cab5f669d2..d14b970a6461 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.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);

[PATCH 3/5] perf probe: Support multiprobe event

[Masami Hiramatsu]

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 d14b970a6461..23db6786c3ea 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)
 {

[PATCH 4/5] perf probe: Support DW_AT_const_value constant value

[Masami Hiramatsu]

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 9ecea45da4ca..2e3a468c8350 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 ? */

[PATCH 5/5] perf probe: Trace a magic number if variable is not found

[Masami Hiramatsu]

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_L0 _text+23480787 pud=%ax:x64
p:probe/pud_page_vaddr_L0 _text+23808289 pud=%bp:x64
p:probe/pud_page_vaddr_L0 _text+23558066 pud=%ax:x64
p:probe/pud_page_vaddr_L0 _text+327957 pud=%r8:x64
p:probe/pud_page_vaddr_L0 _text+348032 pud=%bx:x64
p:probe/pud_page_vaddr_L0 _text+23816654 pud=%bx:x64

With this patch,
# perf probe -D "pud_page_vaddr pud" | head -n 10
spurious_kernel_fault is blacklisted function, skip it.
vmalloc_fault is blacklisted function, skip it.
p:probe/pud_page_vaddr_L0 _text+23480787 pud=%ax:x64
p:probe/pud_page_vaddr_L0 _text+148635 pud=\deade12d:x64
p:probe/pud_page_vaddr_L0 _text+23808289 pud=%bp:x64
p:probe/pud_page_vaddr_L0 _text+315510 pud=\deade12d:x64
p:probe/pud_page_vaddr_L0 _text+23807045 pud=\deade12d:x64
p:probe/pud_page_vaddr_L0 _text+23557349 pud=%ax:x64
p:probe/pud_page_vaddr_L0 _text+313681 pud=%di:x64
p:probe/pud_page_vaddr_L0 _text+313599 pud=\deade12d:x64
p:probe/pud_page_vaddr_L0 _text+313477 pud=\deade12d:x64
p:probe/pud_page_vaddr_L0 _text+323667 pud=\deade12d:x64


Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 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 23db6786c3ea..ceeb75849311 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 2e3a468c8350..eff6063bebe6 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;
@@ -1348,6 +1358,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,
@@ -1366,7 +1414,13 @@ int debuginfo__find_trace_events(struct debuginfo *dbg,
 	tf.tevs = *tevs;
 	tf.ntevs = 0;
 
+	if (pev->event && 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 {

Re: [PATCH 1/5] perf probe: Return a better scope DIE if there is no best scope

[Arnaldo Carvalho de Melo]

Em Tue, Nov 05, 2019 at 09:16:49AM +0900, Masami Hiramatsu escreveu:
> Make find_best_scope() returns innermost DIE at given address if
> there is no best matched scope DIE. Since Gcc sometimes generates
> intuitively strange line info which is out of inlined function
> address range, we need this fixup.
> 
> Without this, sometimes perf probe failed to probe on a line
> inside an inlined function.
>   # perf probe -D ksys_open:3
>   Failed to find scope of probe point.
>     Error: Failed to add events.
> 
> With this fix, perf probe can probe it.
>   # perf probe -D ksys_open:3
>   p:probe/ksys_open _text+25707308
>   p:probe/ksys_open_1 _text+25710596
>   p:probe/ksys_open_2 _text+25711114
>   p:probe/ksys_open_3 _text+25711343
>   p:probe/ksys_open_4 _text+25714058
>   p:probe/ksys_open_5 _text+2819653
>   p:probe/ksys_open_6 _text+2819701

Thanks, tested before and after and applied,

- Arnaldo
 
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
>  tools/perf/util/probe-finder.c |   17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index f441e0174334..9ecea45da4ca 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -744,6 +744,16 @@ static int find_best_scope_cb(Dwarf_Die *fn_die, void *data)
>  	return 0;
>  }
>  
> +/* Return innermost DIE */
> +static int find_inner_scope_cb(Dwarf_Die *fn_die, void *data)
> +{
> +	struct find_scope_param *fsp = data;
> +
> +	memcpy(fsp->die_mem, fn_die, sizeof(Dwarf_Die));
> +	fsp->found = true;
> +	return 1;
> +}
> +
>  /* Find an appropriate scope fits to given conditions */
>  static Dwarf_Die *find_best_scope(struct probe_finder *pf, Dwarf_Die *die_mem)
>  {
> @@ -755,8 +765,13 @@ static Dwarf_Die *find_best_scope(struct probe_finder *pf, Dwarf_Die *die_mem)
>  		.die_mem = die_mem,
>  		.found = false,
>  	};
> +	int ret;
>  
> -	cu_walk_functions_at(&pf->cu_die, pf->addr, find_best_scope_cb, &fsp);
> +	ret = cu_walk_functions_at(&pf->cu_die, pf->addr, find_best_scope_cb,
> +				   &fsp);
> +	if (!ret && !fsp.found)
> +		cu_walk_functions_at(&pf->cu_die, pf->addr,
> +				     find_inner_scope_cb, &fsp);
>  
>  	return fsp.found ? die_mem : NULL;
>  }

-- 

- Arnaldo

Re: [PATCH 2/5] perf probe: Generate event name with line number

[Arnaldo Carvalho de Melo]

Em Tue, Nov 05, 2019 at 09:16:59AM +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 function and lines.
> 
> If there is another event on same line, you have to use
> "-f" option. In that case, the new event has "_1" suffix.

So I don't like this, the existing practice of, if given a function
name, just create the probe:name looks more natural, if one states
kernel:1, then, sure, appending L1 to them is natural, better than the
previous naming convention,

Thanks,

- Arnaldo
 
>  e.g.
>   # perf probe -a kernel_read
>   Added new event:
>     probe:kernel_read_L0 (on kernel_read)
> 
>   You can now use it in all perf tools, such as:
> 
>   	perf record -e probe:kernel_read_L0 -aR sleep 1
> 
>   # perf probe -a kernel_read:1
>   Added new events:
>     probe:kernel_read_L1 (on kernel_read:1)
> 
>   You can now use it in all perf tools, such as:
> 
>   	perf record -e probe:kernel_read_L1_1 -aR sleep 1
> 
>   # perf probe -l
>     probe:kernel_read_L0 (on kernel_read@linux/linux/fs/read_write.c)
>     probe:kernel_read_L1 (on kernel_read@linux/linux/fs/read_write.c)
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
>  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 91cab5f669d2..d14b970a6461 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.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);

-- 

- Arnaldo

Re: [PATCH 3/5] perf probe: Support multiprobe event

[Arnaldo Carvalho de Melo]

Em Tue, Nov 05, 2019 at 09:17:09AM +0900, Masami Hiramatsu escreveu:
> 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)

So this seems to depend on the previous one, I'll leave it for later,
till we figure that one out,

- Arnaldo
 
>   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 d14b970a6461..23db6786c3ea 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)
>  {

-- 

- Arnaldo

Re: [PATCH 4/5] perf probe: Support DW_AT_const_value constant value

[Arnaldo Carvalho de Melo]

Em Tue, Nov 05, 2019 at 09:17:19AM +0900, Masami Hiramatsu escreveu:
> Support DW_AT_const_value for variable assignment instead of location.
> Note that this requires ftrace supporting immediate value.

This one could have come before the other two, depends on the previous
two being applied.

- Arnaldo
 
> 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 9ecea45da4ca..2e3a468c8350 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 ? */

-- 

- Arnaldo

Re: [PATCH 5/5] perf probe: Trace a magic number if variable is not found

[Arnaldo Carvalho de Melo]

Em Tue, Nov 05, 2019 at 09:17:28AM +0900, Masami Hiramatsu escreveu:
> 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.

This too depends on a function introduced in previous patches on hold.

- Arnaldo
 
> 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_L0 _text+23480787 pud=%ax:x64
> p:probe/pud_page_vaddr_L0 _text+23808289 pud=%bp:x64
> p:probe/pud_page_vaddr_L0 _text+23558066 pud=%ax:x64
> p:probe/pud_page_vaddr_L0 _text+327957 pud=%r8:x64
> p:probe/pud_page_vaddr_L0 _text+348032 pud=%bx:x64
> p:probe/pud_page_vaddr_L0 _text+23816654 pud=%bx:x64
> 
> With this patch,
> # perf probe -D "pud_page_vaddr pud" | head -n 10
> spurious_kernel_fault is blacklisted function, skip it.
> vmalloc_fault is blacklisted function, skip it.
> p:probe/pud_page_vaddr_L0 _text+23480787 pud=%ax:x64
> p:probe/pud_page_vaddr_L0 _text+148635 pud=\deade12d:x64
> p:probe/pud_page_vaddr_L0 _text+23808289 pud=%bp:x64
> p:probe/pud_page_vaddr_L0 _text+315510 pud=\deade12d:x64
> p:probe/pud_page_vaddr_L0 _text+23807045 pud=\deade12d:x64
> p:probe/pud_page_vaddr_L0 _text+23557349 pud=%ax:x64
> p:probe/pud_page_vaddr_L0 _text+313681 pud=%di:x64
> p:probe/pud_page_vaddr_L0 _text+313599 pud=\deade12d:x64
> p:probe/pud_page_vaddr_L0 _text+313477 pud=\deade12d:x64
> p:probe/pud_page_vaddr_L0 _text+323667 pud=\deade12d:x64
> 
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
>  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 23db6786c3ea..ceeb75849311 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 2e3a468c8350..eff6063bebe6 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;
> @@ -1348,6 +1358,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,
> @@ -1366,7 +1414,13 @@ int debuginfo__find_trace_events(struct debuginfo *dbg,
>  	tf.tevs = *tevs;
>  	tf.ntevs = 0;
>  
> +	if (pev->event && 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 {

-- 

- Arnaldo

Re: [PATCH 2/5] perf probe: Generate event name with line number

[Masami Hiramatsu]

Hi Arnaldo,

On Wed, 6 Nov 2019 16:54:32 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Em Tue, Nov 05, 2019 at 09:16:59AM +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 function and lines.
> > 
> > If there is another event on same line, you have to use
> > "-f" option. In that case, the new event has "_1" suffix.
> 
> So I don't like this, the existing practice of, if given a function
> name, just create the probe:name looks more natural, if one states
> kernel:1, then, sure, appending L1 to them is natural, better than the
> previous naming convention,

OK, then what about adding L* only when the user add it on a
specific line (like kernel_read:1) ?
IOW, _L0 will be skipped.

Thank you,

> 
> Thanks,
> 
> - Arnaldo
>  
> >  e.g.
> >   # perf probe -a kernel_read
> >   Added new event:
> >     probe:kernel_read_L0 (on kernel_read)
> > 
> >   You can now use it in all perf tools, such as:
> > 
> >   	perf record -e probe:kernel_read_L0 -aR sleep 1
> > 
> >   # perf probe -a kernel_read:1
> >   Added new events:
> >     probe:kernel_read_L1 (on kernel_read:1)
> > 
> >   You can now use it in all perf tools, such as:
> > 
> >   	perf record -e probe:kernel_read_L1_1 -aR sleep 1
> > 
> >   # perf probe -l
> >     probe:kernel_read_L0 (on kernel_read@linux/linux/fs/read_write.c)
> >     probe:kernel_read_L1 (on kernel_read@linux/linux/fs/read_write.c)
> > 
> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > ---
> >  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 91cab5f669d2..d14b970a6461 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.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);
> 
> -- 
> 
> - Arnaldo


-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: [PATCH 3/5] perf probe: Support multiprobe event

[Masami Hiramatsu]

On Wed, 6 Nov 2019 16:56:30 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Em Tue, Nov 05, 2019 at 09:17:09AM +0900, Masami Hiramatsu escreveu:
> > 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)
> 
> So this seems to depend on the previous one, I'll leave it for later,
> till we figure that one out,

Yes, multiprobe can append several different probe points
to one probe-event. User can operate those probes in
one action. (sorry, not "atomically" controlled. that requires
a stop_machine...)

Thank you,

> 
> - Arnaldo
>  
> >   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 d14b970a6461..23db6786c3ea 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)
> >  {
> 
> -- 
> 
> - Arnaldo


-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: [PATCH 2/5] perf probe: Generate event name with line number

[Arnaldo Carvalho de Melo]

Em Thu, Nov 07, 2019 at 10:43:41PM +0900, Masami Hiramatsu escreveu:
> Hi Arnaldo,
> 
> On Wed, 6 Nov 2019 16:54:32 -0300
> Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> 
> > Em Tue, Nov 05, 2019 at 09:16:59AM +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 function and lines.
> > > 
> > > If there is another event on same line, you have to use
> > > "-f" option. In that case, the new event has "_1" suffix.
> > 
> > So I don't like this, the existing practice of, if given a function
> > name, just create the probe:name looks more natural, if one states
> > kernel:1, then, sure, appending L1 to them is natural, better than the
> > previous naming convention,
> 
> OK, then what about adding L* only when the user add it on a
> specific line (like kernel_read:1) ?
> IOW, _L0 will be skipped.

Exactly, agreed.
 
> Thank you,
> 
> > 
> > Thanks,
> > 
> > - Arnaldo
> >  
> > >  e.g.
> > >   # perf probe -a kernel_read
> > >   Added new event:
> > >     probe:kernel_read_L0 (on kernel_read)
> > > 
> > >   You can now use it in all perf tools, such as:
> > > 
> > >   	perf record -e probe:kernel_read_L0 -aR sleep 1
> > > 
> > >   # perf probe -a kernel_read:1
> > >   Added new events:
> > >     probe:kernel_read_L1 (on kernel_read:1)
> > > 
> > >   You can now use it in all perf tools, such as:
> > > 
> > >   	perf record -e probe:kernel_read_L1_1 -aR sleep 1
> > > 
> > >   # perf probe -l
> > >     probe:kernel_read_L0 (on kernel_read@linux/linux/fs/read_write.c)
> > >     probe:kernel_read_L1 (on kernel_read@linux/linux/fs/read_write.c)
> > > 
> > > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > > ---
> > >  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 91cab5f669d2..d14b970a6461 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.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);
> > 
> > -- 
> > 
> > - Arnaldo
> 
> 
> -- 
> Masami Hiramatsu <mhiramat@kernel.org>

-- 

- Arnaldo

[tip: perf/core] perf probe: Return a better scope DIE if there is no best scope

[tip-bot2 for Masami Hiramatsu]

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

Commit-ID:     c701636aeec4c173208697d68da6e4271125564b
Gitweb:        https://git.kernel.org/tip/c701636aeec4c173208697d68da6e4271125564b
Author:        Masami Hiramatsu <mhiramat@kernel.org>
AuthorDate:    Tue, 05 Nov 2019 09:16:49 +09:00
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Thu, 07 Nov 2019 08:30:18 -03:00

perf probe: Return a better scope DIE if there is no best scope

Make find_best_scope() returns innermost DIE at given address if there
is no best matched scope DIE. Since Gcc sometimes generates intuitively
strange line info which is out of inlined function address range, we
need this fixup.

Without this, sometimes perf probe failed to probe on a line inside an
inlined function:

  # perf probe -D ksys_open:3
  Failed to find scope of probe point.
    Error: Failed to add events.

With this fix, 'perf probe' can probe it:

  # perf probe -D ksys_open:3
  p:probe/ksys_open _text+25707308
  p:probe/ksys_open_1 _text+25710596
  p:probe/ksys_open_2 _text+25711114
  p:probe/ksys_open_3 _text+25711343
  p:probe/ksys_open_4 _text+25714058
  p:probe/ksys_open_5 _text+2819653
  p:probe/ksys_open_6 _text+2819701

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/157291300887.19771.14936015360963292236.stgit@devnote2
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-finder.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 88e17a4..582f8c3 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -744,6 +744,16 @@ static int find_best_scope_cb(Dwarf_Die *fn_die, void *data)
 	return 0;
 }
 
+/* Return innermost DIE */
+static int find_inner_scope_cb(Dwarf_Die *fn_die, void *data)
+{
+	struct find_scope_param *fsp = data;
+
+	memcpy(fsp->die_mem, fn_die, sizeof(Dwarf_Die));
+	fsp->found = true;
+	return 1;
+}
+
 /* Find an appropriate scope fits to given conditions */
 static Dwarf_Die *find_best_scope(struct probe_finder *pf, Dwarf_Die *die_mem)
 {
@@ -755,8 +765,13 @@ static Dwarf_Die *find_best_scope(struct probe_finder *pf, Dwarf_Die *die_mem)
 		.die_mem = die_mem,
 		.found = false,
 	};
+	int ret;
 
-	cu_walk_functions_at(&pf->cu_die, pf->addr, find_best_scope_cb, &fsp);
+	ret = cu_walk_functions_at(&pf->cu_die, pf->addr, find_best_scope_cb,
+				   &fsp);
+	if (!ret && !fsp.found)
+		cu_walk_functions_at(&pf->cu_die, pf->addr,
+				     find_inner_scope_cb, &fsp);
 
 	return fsp.found ? die_mem : NULL;
 }