perf probe: Show the error reason comes from invalid DSO

PATCH RESEND In review Masami Hiramatsu 3 participants 6 messages May 27, 2015 perf events

lore

Share & tools

lore

Masami Hiramatsu

author

2015-05-27 08:40 UTC #0 lore raw

PATCH RESEND 1/2 perf probe: Show the error reason comes from invalid DSO

Show the reason of error when dso__load* failed. This shows when user gives wrong kernel image or wrong path.

Without this, perf probe shows an obscure message.
----

  $ perf probe -k ~/kbin/linux-3.x86_64/vmlinux -L vfs_read
  Failed to find path of kernel module.
    Error: Failed to show lines.
  ----

With this, perf shows appropriate error message.
----

  $ perf probe -k ~/kbin/linux-3.x86_64/vmlinux -L vfs_read
  Failed to find the path for kernel: Mismatching build id
    Error: Failed to show lines.
  ----
And
  ----
  $ perf probe -k /non-exist/kernel/vmlinux -L vfs_read
  Failed to find the path for kernel: No such file or directory
    Error: Failed to show lines.
  ----

Signed-off-by Masami Hiramatsu <[email protected]>

---
 tools/perf/util/probe-event.c |   47 +++++++++++++++++++++--------------------
 tools/perf/util/probe-event.h |    3 ---
 2 files changed, 24 insertions(+), 26 deletions(-)

tools/perf/util/probe-event.c +24 ~~-23~~

index 0922565..f5be411 100644
@@ -200,11 +200,12 @@ static void put_target_map(struct map *map, bool user)
 }
 
 
-static struct dso *kernel_get_module_dso(const char *module)
+static int kernel_get_module_dso(const char *module, struct dso **pdso)
 {
 	struct dso *dso;
 	struct map *map;
 	const char *vmlinux_name;
+	int ret = 0;
 
 	if (module) {
 		list_for_each_entry(dso, &host_machine->kernel_dsos.head,

@@ -214,30 +215,21 @@ static struct dso *kernel_get_module_dso(const char *module)
 				goto found;
 		}
 		pr_debug("Failed to find module %s.\n", module);
-		return NULL;
+		return -ENOENT;
 	}
 
 	map = host_machine->vmlinux_maps[MAP__FUNCTION];
 	dso = map->dso;
 
 	vmlinux_name = symbol_conf.vmlinux_name;
-	if (vmlinux_name) {
-		if (dso__load_vmlinux(dso, map, vmlinux_name, false, NULL) <= 0)
-			return NULL;
-	} else {
-		if (dso__load_vmlinux_path(dso, map, NULL) <= 0) {
-			pr_debug("Failed to load kernel map.\n");
-			return NULL;
-		}
-	}
+	dso->load_errno = 0;
+	if (vmlinux_name)
+		ret = dso__load_vmlinux(dso, map, vmlinux_name, false, NULL);
+	else
+		ret = dso__load_vmlinux_path(dso, map, NULL);
 found:
-	return dso;
-}
-
-const char *kernel_get_module_path(const char *module)
-{
-	struct dso *dso = kernel_get_module_dso(module);
-	return (dso) ? dso->long_name : NULL;
+	*pdso = dso;
+	return ret;
 }
 
 static int convert_exec_to_group(const char *exec, char **result)

@@ -389,16 +381,25 @@ static int get_alternative_line_range(struct debuginfo *dinfo,
 static struct debuginfo *open_debuginfo(const char *module, bool silent)
 {
 	const char *path = module;
-	struct debuginfo *ret;
+	char reason[STRERR_BUFSIZE];
+	struct debuginfo *ret = NULL;
+	struct dso *dso = NULL;
+	int err;
 
 	if (!module || !strchr(module, '/')) {
-		path = kernel_get_module_path(module);
-		if (!path) {
+		err = kernel_get_module_dso(module, &dso);
+		if (err < 0) {
+			if (!dso || dso->load_errno == 0) {
+				if (!strerror_r(-err, reason, STRERR_BUFSIZE))
+					strcpy(reason, "(unknown)");
+			} else
+				dso__strerror_load(dso, reason, STRERR_BUFSIZE);
 			if (!silent)
-				pr_err("Failed to find path of %s module.\n",
-				       module ?: "kernel");
+				pr_err("Failed to find the path for %s: %s\n",
+					module ?: "kernel", reason);
 			return NULL;
 		}
+		path = dso->long_name;
 	}
 	ret = debuginfo__new(path);
 	if (!ret && !silent) {

tools/perf/util/probe-event.h +0 -3

index 537eb32..31db6ee 100644
@@ -131,9 +131,6 @@ extern void line_range__clear(struct line_range *lr);
 /* Initialize line range */
 extern int line_range__init(struct line_range *lr);
 
-/* Internal use: Return kernel/module path */
-extern const char *kernel_get_module_path(const char *module);
-
 extern int add_perf_probe_events(struct perf_probe_event *pevs, int npevs);
 extern int del_perf_probe_events(struct strfilter *filter);
 extern int show_perf_probe_events(struct strfilter *filter);

Masami Hiramatsu

author

2015-05-27 08:40 UTC #1 lore raw

PATCH 2/2 perf probe: Fix an error when deleting probes successfully

Fix a bug in del_perf_probe_events() which returns an error (-ENOENT) even if the probes are successfully deleted. This happens only if the probes are on user-apps and not on kernel, simply because it doesn't clear the previous error.

So, without this fix, we get an error even though events are successfully removed.
------

  # ./perf probe -x ./perf del_perf_probe_events
  Added new event:
    probe_perf:del_perf_probe_events (on del_perf_probe_events in ...

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

perf record -e probe_perf:del_perf_probe_events -aR sleep 1

  # ./perf probe -d \*:\*
  Removed event: probe_perf:del_perf_probe_events
    Error: Failed to delete events.
  ------

This fixes the above error.
------

  # ./perf probe -d \*:\*
  Removed event: probe_perf:del_perf_probe_events
  ------

Signed-off-by Masami Hiramatsu <[email protected]>

Reported-by Arnaldo Carvalho de Melo <[email protected]>

---
 tools/perf/util/probe-event.c |    9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

tools/perf/util/probe-event.c +5 -4

index f5be411..97da984 100644
@@ -2811,13 +2811,14 @@ int del_perf_probe_events(struct strfilter *filter)
 		goto error;
 
 	ret2 = del_trace_probe_events(ufd, filter, unamelist);
-	if (ret2 < 0 && ret2 != -ENOENT)
+	if (ret2 < 0 && ret2 != -ENOENT) {
 		ret = ret2;
-	else if (ret == -ENOENT && ret2 == -ENOENT) {
+		goto error;
+	}
+	if (ret == -ENOENT && ret2 == -ENOENT)
 		pr_debug("\"%s\" does not hit any event.\n", str);
 		/* Note that this is silently ignored */
-		ret = 0;
-	}
+	ret = 0;
 
 error:
 	if (kfd >= 0) {

tip-bot for Masami Hiramatsu

merge bot

2015-05-27 16:53 UTC #2 lore raw

[tip:perf/core] perf probe: Fix an error when deleting probes successfully

perf probe: Fix an error when deleting probes successfully

Commit-ID:  dddc7ee32fa13efc66afa71ebd83bce545c8392a
Gitweb:     http://git.kernel.org/tip/dddc7ee32fa13efc66afa71ebd83bce545c8392a
Author:     Masami Hiramatsu <[email protected]>
AuthorDate: Wed, 27 May 2015 17:37:25 +0900
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Wed, 27 May 2015 12:21:46 -0300

perf probe: Fix an error when deleting probes successfully

Fix a bug in del_perf_probe_events() which returns an error (-ENOENT) even if the probes are successfully deleted.

This happens only if the probes are on user-apps and not on kernel, simply because it doesn't clear the previous error.

So, without this fix, we get an error even though events are being successfully removed.

------

  # ./perf probe -x ./perf del_perf_probe_events
  Added new event:
    probe_perf:del_perf_probe_events (on del_perf_probe_events in ...

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

perf record -e probe_perf:del_perf_probe_events -aR sleep 1

  # ./perf probe -d \*:\*
  Removed event: probe_perf:del_perf_probe_events
    Error: Failed to delete events.
  ------

This fixes the above error.
------

  # ./perf probe -d \*:\*
  Removed event: probe_perf:del_perf_probe_events
  ------

Reported-by Arnaldo Carvalho de Melo <[email protected]>

Signed-off-by Masami Hiramatsu <[email protected]>

Tested-by Arnaldo Carvalho de Melo <[email protected]>

Cc David Ahern <[email protected]>

Cc Jiri Olsa <[email protected]>

Cc Namhyung Kim <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Richard Weinberger <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 tools/perf/util/probe-event.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

tools/perf/util/probe-event.c +5 -4

index f5be411..97da984 100644
@@ -2811,13 +2811,14 @@ int del_perf_probe_events(struct strfilter *filter)
 		goto error;
 
 	ret2 = del_trace_probe_events(ufd, filter, unamelist);
-	if (ret2 < 0 && ret2 != -ENOENT)
+	if (ret2 < 0 && ret2 != -ENOENT) {
 		ret = ret2;
-	else if (ret == -ENOENT && ret2 == -ENOENT) {
+		goto error;
+	}
+	if (ret == -ENOENT && ret2 == -ENOENT)
 		pr_debug("\"%s\" does not hit any event.\n", str);
 		/* Note that this is silently ignored */
-		ret = 0;
-	}
+	ret = 0;
 
 error:
 	if (kfd >= 0) {

Arnaldo Carvalho de Melo

maintainer

2015-05-27 12:50 UTC #3 lore raw

In reply to Masami Hiramatsu

Em Wed, May 27, 2015 at 05:37:18PM +0900, Masami Hiramatsu escreveu:

Quoted 12 lines

Show the reason of error when dso__load* failed. This shows when user gives wrong kernel image or wrong path.
Without this, perf probe shows an obscure message.
----
  $ perf probe -k ~/kbin/linux-3.x86_64/vmlinux -L vfs_read
  Failed to find path of kernel module.
    Error: Failed to show lines.
  ----
With this, perf shows appropriate error message.
----

Quoted 10 lines

  $ perf probe -k ~/kbin/linux-3.x86_64/vmlinux -L vfs_read
  Failed to find the path for kernel: Mismatching build id
    Error: Failed to show lines.
  ----
And
  ----
  $ perf probe -k /non-exist/kernel/vmlinux -L vfs_read
  Failed to find the path for kernel: No such file or directory
    Error: Failed to show lines.
  ----

Its better than before, and I am applying it.

But please consider to change the message further, because when one specifies the vmlinux file to use with -k/--vmlinux, the tool is not trying to "find the path for kernel", it is trying to use a specific vmlinux file.

I.e. the messages above are good if we do:

perf probe -L vfs_read

Here it _will_ try to find the path, as none was given.

So, when -k/--vmlinux is used, I suggest that the message be:

  $ perf probe -k ~/kbin/linux-3.x86_64/vmlinux -L vfs_read
  The kernel file "~/kbin/linux-3.x86_64/vmlinux" could not be used: Mismatching build id
    Error: Failed to show lines.

And:

  $ perf probe -k /non-exist/kernel/vmlinux -L vfs_read
  The kernel file "/non-exist/kernel/vmlinux" could not be used: No such file or directory
    Error: Failed to show lines.

Thanks!

- Arnaldo

Masami Hiramatsu

author

2015-05-27 23:42 UTC #4 lore raw

Quoted 43 lines · Arnaldo Carvalho de Melo

Em Wed, May 27, 2015 at 05:37:18PM +0900, Masami Hiramatsu escreveu:
Quoted 12 lines
Show the reason of error when dso__load* failed. This shows when user gives wrong kernel image or wrong path.
Without this, perf probe shows an obscure message.
----
  $ perf probe -k ~/kbin/linux-3.x86_64/vmlinux -L vfs_read
  Failed to find path of kernel module.
    Error: Failed to show lines.
  ----
With this, perf shows appropriate error message.
----
Quoted 10 lines
  $ perf probe -k ~/kbin/linux-3.x86_64/vmlinux -L vfs_read
  Failed to find the path for kernel: Mismatching build id
    Error: Failed to show lines.
  ----
And
  ----
  $ perf probe -k /non-exist/kernel/vmlinux -L vfs_read
  Failed to find the path for kernel: No such file or directory
    Error: Failed to show lines.
  ----
Its better than before, and I am applying it.
But please consider to change the message further, because when one specifies the vmlinux file to use with -k/--vmlinux, the tool is not trying to "find the path for kernel", it is trying to use a specific vmlinux file.
I.e. the messages above are good if we do:
perf probe -L vfs_read
Here it _will_ try to find the path, as none was given.
So, when -k/--vmlinux is used, I suggest that the message be:
  $ perf probe -k ~/kbin/linux-3.x86_64/vmlinux -L vfs_read
  The kernel file "~/kbin/linux-3.x86_64/vmlinux" could not be used: Mismatching build id
    Error: Failed to show lines.

Quoted 5 lines

And:

  $ perf probe -k /non-exist/kernel/vmlinux -L vfs_read
  The kernel file "/non-exist/kernel/vmlinux" could not be used: No such file or directory
    Error: Failed to show lines.

Indeed! This looks better for me.

Thank you!

Signature · 5 lines

Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: [email protected]

tip-bot for Masami Hiramatsu

merge bot

2015-05-27 16:53 UTC #5 lore raw

In reply to Masami Hiramatsu

perf probe: Show the error reason comes from invalid DSO

Commit-ID:  419e87382873b11b17cb31e2f21859570a32e0d1
Gitweb:     http://git.kernel.org/tip/419e87382873b11b17cb31e2f21859570a32e0d1
Author:     Masami Hiramatsu <[email protected]>
AuthorDate: Wed, 27 May 2015 17:37:18 +0900
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Wed, 27 May 2015 12:21:45 -0300

perf probe: Show the error reason comes from invalid DSO

Show the reason of error when dso__load* fails. This shows when user gives wrong kernel image or wrong path.

Without this, perf probe shows an obscure message:

----

  $ perf probe -k ~/kbin/linux-3.x86_64/vmlinux -L vfs_read
  Failed to find path of kernel module.
    Error: Failed to show lines.
  ----

With this, perf shows appropriate error message:

----

  $ perf probe -k ~/kbin/linux-3.x86_64/vmlinux -L vfs_read
  Failed to find the path for kernel: Mismatching build id
    Error: Failed to show lines.
  ----

And:

----

  $ perf probe -k /non-exist/kernel/vmlinux -L vfs_read
  Failed to find the path for kernel: No such file or directory
    Error: Failed to show lines.
  ----

Signed-off-by Masami Hiramatsu <[email protected]>

Tested-by Arnaldo Carvalho de Melo <[email protected]>

Cc David Ahern <[email protected]>

Cc Jiri Olsa <[email protected]>

Cc Namhyung Kim <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Richard Weinberger <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 tools/perf/util/probe-event.c | 47 ++++++++++++++++++++++---------------------
 tools/perf/util/probe-event.h |  3 ---
 2 files changed, 24 insertions(+), 26 deletions(-)

tools/perf/util/probe-event.c +24 ~~-23~~

index 0922565..f5be411 100644
@@ -200,11 +200,12 @@ static void put_target_map(struct map *map, bool user)
 }
 
 
-static struct dso *kernel_get_module_dso(const char *module)
+static int kernel_get_module_dso(const char *module, struct dso **pdso)
 {
 	struct dso *dso;
 	struct map *map;
 	const char *vmlinux_name;
+	int ret = 0;
 
 	if (module) {
 		list_for_each_entry(dso, &host_machine->kernel_dsos.head,

@@ -214,30 +215,21 @@ static struct dso *kernel_get_module_dso(const char *module)
 				goto found;
 		}
 		pr_debug("Failed to find module %s.\n", module);
-		return NULL;
+		return -ENOENT;
 	}
 
 	map = host_machine->vmlinux_maps[MAP__FUNCTION];
 	dso = map->dso;
 
 	vmlinux_name = symbol_conf.vmlinux_name;
-	if (vmlinux_name) {
-		if (dso__load_vmlinux(dso, map, vmlinux_name, false, NULL) <= 0)
-			return NULL;
-	} else {
-		if (dso__load_vmlinux_path(dso, map, NULL) <= 0) {
-			pr_debug("Failed to load kernel map.\n");
-			return NULL;
-		}
-	}
+	dso->load_errno = 0;
+	if (vmlinux_name)
+		ret = dso__load_vmlinux(dso, map, vmlinux_name, false, NULL);
+	else
+		ret = dso__load_vmlinux_path(dso, map, NULL);
 found:
-	return dso;
-}
-
-const char *kernel_get_module_path(const char *module)
-{
-	struct dso *dso = kernel_get_module_dso(module);
-	return (dso) ? dso->long_name : NULL;
+	*pdso = dso;
+	return ret;
 }
 
 static int convert_exec_to_group(const char *exec, char **result)

@@ -389,16 +381,25 @@ static int get_alternative_line_range(struct debuginfo *dinfo,
 static struct debuginfo *open_debuginfo(const char *module, bool silent)
 {
 	const char *path = module;
-	struct debuginfo *ret;
+	char reason[STRERR_BUFSIZE];
+	struct debuginfo *ret = NULL;
+	struct dso *dso = NULL;
+	int err;
 
 	if (!module || !strchr(module, '/')) {
-		path = kernel_get_module_path(module);
-		if (!path) {
+		err = kernel_get_module_dso(module, &dso);
+		if (err < 0) {
+			if (!dso || dso->load_errno == 0) {
+				if (!strerror_r(-err, reason, STRERR_BUFSIZE))
+					strcpy(reason, "(unknown)");
+			} else
+				dso__strerror_load(dso, reason, STRERR_BUFSIZE);
 			if (!silent)
-				pr_err("Failed to find path of %s module.\n",
-				       module ?: "kernel");
+				pr_err("Failed to find the path for %s: %s\n",
+					module ?: "kernel", reason);
 			return NULL;
 		}
+		path = dso->long_name;
 	}
 	ret = debuginfo__new(path);
 	if (!ret && !silent) {

tools/perf/util/probe-event.h +0 -3

index 537eb32..31db6ee 100644
@@ -131,9 +131,6 @@ extern void line_range__clear(struct line_range *lr);
 /* Initialize line range */
 extern int line_range__init(struct line_range *lr);
 
-/* Internal use: Return kernel/module path */
-extern const char *kernel_get_module_path(const char *module);
-
 extern int add_perf_probe_events(struct perf_probe_event *pevs, int npevs);
 extern int del_perf_probe_events(struct strfilter *filter);
 extern int show_perf_probe_events(struct strfilter *filter);