Subject: [PATCH -tip 0/4] perf/probe: Improve error messages

Hi,

Here is a series that improves error messages of perf probe.

- Improve the error message if we can not find given member in
the given structure. (perf probe --add)
- Show error code and description only in verbose mode if
perf probe command is failed.
- Improve error messages of perf probe --vars mode.
- Improve error messages of perf probe --line mode.

This series is mainly for fixing confusing messages and
for removing the error code which is just a mysterious
number for users.

Thank you,

---

Masami Hiramatsu (4):
perf/probe: Improve error message for unknown member of data structure
perf/probe: Show error code and description in verbose mode
perf/probe: Improve an error message of perf probe --vars mode
perf/probe: Improve error messages with --line option


tools/perf/builtin-probe.c | 23 ++++++++++++++---------
tools/perf/util/probe-event.c | 13 +++++++++----
tools/perf/util/probe-finder.c | 11 +++++++----
3 files changed, 30 insertions(+), 17 deletions(-)

--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]


Subject: [PATCH -tip 1/4] perf/probe: Improve error message for unknown member of data structure

Improve the error message if we can not find given member in
the given structure. Currently perf probe shows a wrong error
message as below.

-----
# perf probe getname_flags:65 "result->BOGUS"
result(type:filename) has no member BOGUS.
Failed to find 'result' in this function.
Error: Failed to add events. (-22)
-----

The first message is correct, but the second one is not, since
we didn't fail to find a variable but fails to find the member
of given variable.

-----
# perf probe getname_flags:65 "result->BOGUS"
result(type:filename) has no member BOGUS.
Error: Failed to add events. (-22)
-----

With this patch, the error message shows only the first one.
And if we really failed to find given variable, it tells us so.

-----
# perf probe getname_flags:65 "BOGUS"
Failed to find 'BOGUS' in this function.
Error: Failed to add events. (-2)
-----

Signed-off-by: Masami Hiramatsu <[email protected]>
Reported-by: Arnaldo Carvalho de Melo <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Jiri Olsa <[email protected]>
---
tools/perf/util/probe-finder.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 9d8eb26..ce8faf4 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -573,14 +573,13 @@ static int find_variable(Dwarf_Die *sc_die, struct probe_finder *pf)
if (!die_find_variable_at(sc_die, pf->pvar->var, pf->addr, &vr_die)) {
/* Search again in global variables */
if (!die_find_variable_at(&pf->cu_die, pf->pvar->var, 0, &vr_die))
+ pr_warning("Failed to find '%s' in this function.\n",
+ pf->pvar->var);
ret = -ENOENT;
}
if (ret >= 0)
ret = convert_variable(&vr_die, pf);

- if (ret < 0)
- pr_warning("Failed to find '%s' in this function.\n",
- pf->pvar->var);
return ret;
}


Subject: [PATCH -tip 2/4] perf/probe: Show error code and description in verbose mode

Show error code and description only in verbose mode if
perf probe command is failed. Current perf probe shows
error code with final error message, and that is meaningless
for many users. This changes error messages to show the error
code and its description only in verbose mode (-v option).

Without this patch:
-----
# perf probe -a do_execve@hoge
Probe point 'do_execve@hoge' not found.
Error: Failed to add events. (-2)
-----

With this patch, normally the message doesn't show the
misterious error number.
-----
# perf probe -a do_execve@hoge
Probe point 'do_execve@hoge' not found.
Error: Failed to add events.
-----

And in verbose mode, it also shows additional error
messages as below:
-----
# perf probe -va do_execve@hoge
probe-definition(0): do_execve@hoge
symbol:do_execve file:hoge line:0 offset:0 return:0 lazy:(null)
0 arguments
Looking at the vmlinux_path (6 entries long)
Using /lib/modules/3.15.0-rc8+/build/vmlinux for symbols
Open Debuginfo file: /lib/modules/3.15.0-rc8+/build/vmlinux
Try to find probe point from debuginfo.
Probe point 'do_execve@hoge' not found.
Error: Failed to add events. Reason: No such file or directory (Code: -2)
-----

Signed-off-by: Masami Hiramatsu <[email protected]>
Reported-by: Arnaldo Carvalho de Melo <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Ingo Molnar <[email protected]>
---
tools/perf/builtin-probe.c | 23 ++++++++++++++---------
1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index cdcd4eb..c63fa29 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -288,6 +288,13 @@ static void cleanup_params(void)
memset(&params, 0, sizeof(params));
}

+static void pr_err_with_code(const char *msg, int err)
+{
+ pr_err("%s", msg);
+ pr_debug(" Reason: %s (Code: %d)", strerror(-err), err);
+ pr_err("\n");
+}
+
static int
__cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
{
@@ -379,7 +386,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
}
ret = parse_probe_event_argv(argc, argv);
if (ret < 0) {
- pr_err(" Error: Parse Error. (%d)\n", ret);
+ pr_err_with_code(" Error: Command Parse Error.", ret);
return ret;
}
}
@@ -419,8 +426,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
}
ret = show_perf_probe_events();
if (ret < 0)
- pr_err(" Error: Failed to show event list. (%d)\n",
- ret);
+ pr_err_with_code(" Error: Failed to show event list.", ret);
return ret;
}
if (params.show_funcs) {
@@ -445,8 +451,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
strfilter__delete(params.filter);
params.filter = NULL;
if (ret < 0)
- pr_err(" Error: Failed to show functions."
- " (%d)\n", ret);
+ pr_err_with_code(" Error: Failed to show functions.", ret);
return ret;
}

@@ -464,7 +469,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)

ret = show_line_range(&params.line_range, params.target);
if (ret < 0)
- pr_err(" Error: Failed to show lines. (%d)\n", ret);
+ pr_err_with_code(" Error: Failed to show lines.", ret);
return ret;
}
if (params.show_vars) {
@@ -485,7 +490,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
strfilter__delete(params.filter);
params.filter = NULL;
if (ret < 0)
- pr_err(" Error: Failed to show vars. (%d)\n", ret);
+ pr_err_with_code(" Error: Failed to show vars.", ret);
return ret;
}
#endif
@@ -493,7 +498,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
if (params.dellist) {
ret = del_perf_probe_events(params.dellist);
if (ret < 0) {
- pr_err(" Error: Failed to delete events. (%d)\n", ret);
+ pr_err_with_code(" Error: Failed to delete events.", ret);
return ret;
}
}
@@ -504,7 +509,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
params.target,
params.force_add);
if (ret < 0) {
- pr_err(" Error: Failed to add events. (%d)\n", ret);
+ pr_err_with_code(" Error: Failed to add events.", ret);
return ret;
}
}

Subject: [PATCH -tip 3/4] perf/probe: Improve an error message of perf probe --vars mode

Fix an error message when failed to find given address in --vars
mode.

Without this fix, perf probe -V doesn't show the final "Error"
message if it fails to find given source line. Moreover, it
tells it fails to find "variables" instead of the source line.
-----
# perf probe -V foo@bar
Failed to find variables at foo@bar (0)
-----
The result also shows mysterious error code. Actually the error
returns 0 or -ENOENT means that it just fails to find the address
of given source line. (0 means there is no matching address,
and -ENOENT means there is an entry(DIE) but it has no instance,
e.g. an empty inlined function)

This fixes it to show what happened and the final error message
as below.
-----
# perf probe -V foo@bar
Failed to find the address of foo@bar
Error: Failed to show vars.
-----

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Jiri Olsa <[email protected]>
---
tools/perf/util/probe-event.c | 7 ++++++-
tools/perf/util/probe-finder.c | 6 +++++-
2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 0d1542f..44c7141 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -721,9 +721,14 @@ static int show_available_vars_at(struct debuginfo *dinfo,
ret = debuginfo__find_available_vars_at(dinfo, pev, &vls,
max_vls, externs);
if (ret <= 0) {
- pr_err("Failed to find variables at %s (%d)\n", buf, ret);
+ if (ret == 0 || ret == -ENOENT) {
+ pr_err("Failed to find the address of %s\n", buf);
+ ret = -ENOENT;
+ } else
+ pr_warning("Debuginfo analysis failed.\n");
goto end;
}
+
/* Some variables are found */
fprintf(stdout, "Available variables at %s\n", buf);
for (i = 0; i < ret; i++) {
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index ce8faf4..98e3047 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -1280,7 +1280,11 @@ out:
return ret;
}

-/* Find available variables at given probe point */
+/*
+ * Find available variables at given probe point
+ * Return the number of found probe points. Return 0 if there is no
+ * matched probe point. Return <0 if an error occurs.
+ */
int debuginfo__find_available_vars_at(struct debuginfo *dbg,
struct perf_probe_event *pev,
struct variable_list **vls,

Subject: [PATCH -tip 4/4] perf/probe: Improve error messages with --line option

Improve error messages of perf probe --line mode. Current
perf probe shows "Debuginfo analysis failed" error with an
error code when the given symbol is not found as below.

-----
# perf probe -L page_cgroup_init_flatmem
Debuginfo analysis failed. (-2)
Error: Failed to show lines.
-----

But -2 (-ENOMEM) means that the given source line or function
is not found. With this patch, perf probe shows correct error
message as below.

-----
# perf probe -L page_cgroup_init_flatmem
Specified source line is not found.
Error: Failed to show lines.
-----

There is also another debug error code is shown in the same
function after get_real_path(). This removes that too.

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/probe-event.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 44c7141..9a0a183 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -628,11 +628,11 @@ static int __show_line_range(struct line_range *lr, const char *module)

ret = debuginfo__find_line_range(dinfo, lr);
debuginfo__delete(dinfo);
- if (ret == 0) {
+ if (ret == 0 || ret == -ENOENT) {
pr_warning("Specified source line is not found.\n");
return -ENOENT;
} else if (ret < 0) {
- pr_warning("Debuginfo analysis failed. (%d)\n", ret);
+ pr_warning("Debuginfo analysis failed.\n");
return ret;
}

@@ -641,7 +641,7 @@ static int __show_line_range(struct line_range *lr, const char *module)
ret = get_real_path(tmp, lr->comp_dir, &lr->path);
free(tmp); /* Free old path */
if (ret < 0) {
- pr_warning("Failed to find source file. (%d)\n", ret);
+ pr_warning("Failed to find source file path.\n");
return ret;
}


2014-06-06 13:09:08

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH -tip 0/4] perf/probe: Improve error messages

Em Fri, Jun 06, 2014 at 07:13:38AM +0000, Masami Hiramatsu escreveu:
> Here is a series that improves error messages of perf probe.
>
> - Improve the error message if we can not find given member in
> the given structure. (perf probe --add)
> - Show error code and description only in verbose mode if
> perf probe command is failed.
> - Improve error messages of perf probe --vars mode.
> - Improve error messages of perf probe --line mode.
>
> This series is mainly for fixing confusing messages and
> for removing the error code which is just a mysterious
> number for users.

Thanks, its better now!

Jiri, I'll process these patches,

- Arnaldo

2014-06-10 08:11:50

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCH -tip 4/4] perf/probe: Improve error messages with --line option

Hi Masami,

On Fri, 06 Jun 2014 07:14:06 +0000, Masami Hiramatsu wrote:
> Improve error messages of perf probe --line mode. Current
> perf probe shows "Debuginfo analysis failed" error with an
> error code when the given symbol is not found as below.
>
> -----
> # perf probe -L page_cgroup_init_flatmem
> Debuginfo analysis failed. (-2)
> Error: Failed to show lines.
> -----
>
> But -2 (-ENOMEM) means that the given source line or function

s/ENOMEM/ENOENT/

Thanks,
Namhyung


> is not found. With this patch, perf probe shows correct error
> message as below.
>
> -----
> # perf probe -L page_cgroup_init_flatmem
> Specified source line is not found.
> Error: Failed to show lines.
> -----
>
> There is also another debug error code is shown in the same
> function after get_real_path(). This removes that too.
>
> Signed-off-by: Masami Hiramatsu <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Paul Mackerras <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Arnaldo Carvalho de Melo <[email protected]>
> ---
> tools/perf/util/probe-event.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 44c7141..9a0a183 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -628,11 +628,11 @@ static int __show_line_range(struct line_range *lr, const char *module)
>
> ret = debuginfo__find_line_range(dinfo, lr);
> debuginfo__delete(dinfo);
> - if (ret == 0) {
> + if (ret == 0 || ret == -ENOENT) {
> pr_warning("Specified source line is not found.\n");
> return -ENOENT;
> } else if (ret < 0) {
> - pr_warning("Debuginfo analysis failed. (%d)\n", ret);
> + pr_warning("Debuginfo analysis failed.\n");
> return ret;
> }
>
> @@ -641,7 +641,7 @@ static int __show_line_range(struct line_range *lr, const char *module)
> ret = get_real_path(tmp, lr->comp_dir, &lr->path);
> free(tmp); /* Free old path */
> if (ret < 0) {
> - pr_warning("Failed to find source file. (%d)\n", ret);
> + pr_warning("Failed to find source file path.\n");
> return ret;
> }
>

Subject: Re: Re: [PATCH -tip 4/4] perf/probe: Improve error messages with --line option

(2014/06/10 17:11), Namhyung Kim wrote:
> Hi Masami,
>
> On Fri, 06 Jun 2014 07:14:06 +0000, Masami Hiramatsu wrote:
>> Improve error messages of perf probe --line mode. Current
>> perf probe shows "Debuginfo analysis failed" error with an
>> error code when the given symbol is not found as below.
>>
>> -----
>> # perf probe -L page_cgroup_init_flatmem
>> Debuginfo analysis failed. (-2)
>> Error: Failed to show lines.
>> -----
>>
>> But -2 (-ENOMEM) means that the given source line or function
>
> s/ENOMEM/ENOENT/

Ah, right :) It's a typo...

Thanks!

>
> Thanks,
> Namhyung
>
>
>> is not found. With this patch, perf probe shows correct error
>> message as below.
>>
>> -----
>> # perf probe -L page_cgroup_init_flatmem
>> Specified source line is not found.
>> Error: Failed to show lines.
>> -----
>>
>> There is also another debug error code is shown in the same
>> function after get_real_path(). This removes that too.
>>
>> Signed-off-by: Masami Hiramatsu <[email protected]>
>> Cc: Peter Zijlstra <[email protected]>
>> Cc: Paul Mackerras <[email protected]>
>> Cc: Ingo Molnar <[email protected]>
>> Cc: Arnaldo Carvalho de Melo <[email protected]>
>> ---
>> tools/perf/util/probe-event.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
>> index 44c7141..9a0a183 100644
>> --- a/tools/perf/util/probe-event.c
>> +++ b/tools/perf/util/probe-event.c
>> @@ -628,11 +628,11 @@ static int __show_line_range(struct line_range *lr, const char *module)
>>
>> ret = debuginfo__find_line_range(dinfo, lr);
>> debuginfo__delete(dinfo);
>> - if (ret == 0) {
>> + if (ret == 0 || ret == -ENOENT) {
>> pr_warning("Specified source line is not found.\n");
>> return -ENOENT;
>> } else if (ret < 0) {
>> - pr_warning("Debuginfo analysis failed. (%d)\n", ret);
>> + pr_warning("Debuginfo analysis failed.\n");
>> return ret;
>> }
>>
>> @@ -641,7 +641,7 @@ static int __show_line_range(struct line_range *lr, const char *module)
>> ret = get_real_path(tmp, lr->comp_dir, &lr->path);
>> free(tmp); /* Free old path */
>> if (ret < 0) {
>> - pr_warning("Failed to find source file. (%d)\n", ret);
>> + pr_warning("Failed to find source file path.\n");
>> return ret;
>> }
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>


--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]

Subject: [tip:perf/core] perf probe: Improve error message for unknown member of data structure

Commit-ID: 36d789a4d75f3826faa6e75b018942b63ffed1a0
Gitweb: http://git.kernel.org/tip/36d789a4d75f3826faa6e75b018942b63ffed1a0
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Fri, 6 Jun 2014 07:13:45 +0000
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Mon, 9 Jun 2014 12:15:07 -0300

perf probe: Improve error message for unknown member of data structure

Improve the error message if we can not find given member in the given
structure. Currently perf probe shows a wrong error message as below.

-----
# perf probe getname_flags:65 "result->BOGUS"
result(type:filename) has no member BOGUS.
Failed to find 'result' in this function.
Error: Failed to add events. (-22)
-----

The first message is correct, but the second one is not, since we didn't
fail to find a variable but fails to find the member of given variable.

-----
# perf probe getname_flags:65 "result->BOGUS"
result(type:filename) has no member BOGUS.
Error: Failed to add events. (-22)
-----

With this patch, the error message shows only the first one. And if we
really failed to find given variable, it tells us so.

-----
# perf probe getname_flags:65 "BOGUS"
Failed to find 'BOGUS' in this function.
Error: Failed to add events. (-2)
-----

Reported-by: Arnaldo Carvalho de Melo <[email protected]>
Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/probe-finder.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 9d8eb26..ce8faf4 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -573,14 +573,13 @@ static int find_variable(Dwarf_Die *sc_die, struct probe_finder *pf)
if (!die_find_variable_at(sc_die, pf->pvar->var, pf->addr, &vr_die)) {
/* Search again in global variables */
if (!die_find_variable_at(&pf->cu_die, pf->pvar->var, 0, &vr_die))
+ pr_warning("Failed to find '%s' in this function.\n",
+ pf->pvar->var);
ret = -ENOENT;
}
if (ret >= 0)
ret = convert_variable(&vr_die, pf);

- if (ret < 0)
- pr_warning("Failed to find '%s' in this function.\n",
- pf->pvar->var);
return ret;
}

Subject: [tip:perf/core] perf probe: Show error code and description in verbose mode

Commit-ID: b4bf1130cdee7d5247bd3171530869809f5aca54
Gitweb: http://git.kernel.org/tip/b4bf1130cdee7d5247bd3171530869809f5aca54
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Fri, 6 Jun 2014 07:13:52 +0000
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Mon, 9 Jun 2014 14:34:09 -0300

perf probe: Show error code and description in verbose mode

Show error code and description only in verbose mode if 'perf probe'
command failed.

Current 'perf probe' shows error code with final error message, and that
is meaningless for many users.

This changes error messages to show the error code and its description
only in verbose mode (-v option).

Without this patch:
-----
# perf probe -a do_execve@hoge
Probe point 'do_execve@hoge' not found.
Error: Failed to add events. (-2)
-----

With this patch, normally the message doesn't show the misterious error
number:
-----
# perf probe -a do_execve@hoge
Probe point 'do_execve@hoge' not found.
Error: Failed to add events.
-----

And in verbose mode, it also shows additional error messages as below:
-----
# perf probe -va do_execve@hoge
probe-definition(0): do_execve@hoge
symbol:do_execve file:hoge line:0 offset:0 return:0 lazy:(null)
0 arguments
Looking at the vmlinux_path (6 entries long)
Using /lib/modules/3.15.0-rc8+/build/vmlinux for symbols
Open Debuginfo file: /lib/modules/3.15.0-rc8+/build/vmlinux
Try to find probe point from debuginfo.
Probe point 'do_execve@hoge' not found.
Error: Failed to add events. Reason: No such file or directory (Code: -2)
-----

Reported-by: Arnaldo Carvalho de Melo <[email protected]>
Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/builtin-probe.c | 23 ++++++++++++++---------
1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index cdcd4eb..c63fa29 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -288,6 +288,13 @@ static void cleanup_params(void)
memset(&params, 0, sizeof(params));
}

+static void pr_err_with_code(const char *msg, int err)
+{
+ pr_err("%s", msg);
+ pr_debug(" Reason: %s (Code: %d)", strerror(-err), err);
+ pr_err("\n");
+}
+
static int
__cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
{
@@ -379,7 +386,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
}
ret = parse_probe_event_argv(argc, argv);
if (ret < 0) {
- pr_err(" Error: Parse Error. (%d)\n", ret);
+ pr_err_with_code(" Error: Command Parse Error.", ret);
return ret;
}
}
@@ -419,8 +426,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
}
ret = show_perf_probe_events();
if (ret < 0)
- pr_err(" Error: Failed to show event list. (%d)\n",
- ret);
+ pr_err_with_code(" Error: Failed to show event list.", ret);
return ret;
}
if (params.show_funcs) {
@@ -445,8 +451,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
strfilter__delete(params.filter);
params.filter = NULL;
if (ret < 0)
- pr_err(" Error: Failed to show functions."
- " (%d)\n", ret);
+ pr_err_with_code(" Error: Failed to show functions.", ret);
return ret;
}

@@ -464,7 +469,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)

ret = show_line_range(&params.line_range, params.target);
if (ret < 0)
- pr_err(" Error: Failed to show lines. (%d)\n", ret);
+ pr_err_with_code(" Error: Failed to show lines.", ret);
return ret;
}
if (params.show_vars) {
@@ -485,7 +490,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
strfilter__delete(params.filter);
params.filter = NULL;
if (ret < 0)
- pr_err(" Error: Failed to show vars. (%d)\n", ret);
+ pr_err_with_code(" Error: Failed to show vars.", ret);
return ret;
}
#endif
@@ -493,7 +498,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
if (params.dellist) {
ret = del_perf_probe_events(params.dellist);
if (ret < 0) {
- pr_err(" Error: Failed to delete events. (%d)\n", ret);
+ pr_err_with_code(" Error: Failed to delete events.", ret);
return ret;
}
}
@@ -504,7 +509,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
params.target,
params.force_add);
if (ret < 0) {
- pr_err(" Error: Failed to add events. (%d)\n", ret);
+ pr_err_with_code(" Error: Failed to add events.", ret);
return ret;
}
}

Subject: [tip:perf/core] perf probe: Improve an error message of perf probe --vars mode

Commit-ID: 69e96eaa4fef04ad543eda3eab787dbae99d8912
Gitweb: http://git.kernel.org/tip/69e96eaa4fef04ad543eda3eab787dbae99d8912
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Fri, 6 Jun 2014 07:13:59 +0000
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Mon, 9 Jun 2014 14:35:58 -0300

perf probe: Improve an error message of perf probe --vars mode

Fix an error message when failed to find given address in --vars
mode.

Without this fix, perf probe -V doesn't show the final "Error"
message if it fails to find given source line. Moreover, it
tells it fails to find "variables" instead of the source line.
-----
# perf probe -V foo@bar
Failed to find variables at foo@bar (0)
-----
The result also shows mysterious error code. Actually the error
returns 0 or -ENOENT means that it just fails to find the address
of given source line. (0 means there is no matching address,
and -ENOENT means there is an entry(DIE) but it has no instance,
e.g. an empty inlined function)

This fixes it to show what happened and the final error message
as below.
-----
# perf probe -V foo@bar
Failed to find the address of foo@bar
Error: Failed to show vars.
-----

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/probe-event.c | 7 ++++++-
tools/perf/util/probe-finder.c | 6 +++++-
2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 0d1542f..44c7141 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -721,9 +721,14 @@ static int show_available_vars_at(struct debuginfo *dinfo,
ret = debuginfo__find_available_vars_at(dinfo, pev, &vls,
max_vls, externs);
if (ret <= 0) {
- pr_err("Failed to find variables at %s (%d)\n", buf, ret);
+ if (ret == 0 || ret == -ENOENT) {
+ pr_err("Failed to find the address of %s\n", buf);
+ ret = -ENOENT;
+ } else
+ pr_warning("Debuginfo analysis failed.\n");
goto end;
}
+
/* Some variables are found */
fprintf(stdout, "Available variables at %s\n", buf);
for (i = 0; i < ret; i++) {
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index ce8faf4..98e3047 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -1280,7 +1280,11 @@ out:
return ret;
}

-/* Find available variables at given probe point */
+/*
+ * Find available variables at given probe point
+ * Return the number of found probe points. Return 0 if there is no
+ * matched probe point. Return <0 if an error occurs.
+ */
int debuginfo__find_available_vars_at(struct debuginfo *dbg,
struct perf_probe_event *pev,
struct variable_list **vls,

Subject: [tip:perf/core] perf probe: Improve error messages in --line option

Commit-ID: 5ee05b8801892ecc5df44e03429008dfa89aa361
Gitweb: http://git.kernel.org/tip/5ee05b8801892ecc5df44e03429008dfa89aa361
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Fri, 6 Jun 2014 07:14:06 +0000
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 10 Jun 2014 10:02:06 -0300

perf probe: Improve error messages in --line option

Improve error messages of 'perf probe --line' mode.

Currently 'perf probe' shows the "Debuginfo analysis failed" message with
an error code when the given symbol is not found:

-----
# perf probe -L page_cgroup_init_flatmem
Debuginfo analysis failed. (-2)
Error: Failed to show lines.
-----

But -2 (-ENOENT) means that the given source line or function was not
found. With this patch, 'perf probe' shows the correct error message:

-----
# perf probe -L page_cgroup_init_flatmem
Specified source line is not found.
Error: Failed to show lines.
-----

There is also another debug error code is shown in the same function
after get_real_path(). This removes that too.

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/probe-event.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 44c7141..9a0a183 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -628,11 +628,11 @@ static int __show_line_range(struct line_range *lr, const char *module)

ret = debuginfo__find_line_range(dinfo, lr);
debuginfo__delete(dinfo);
- if (ret == 0) {
+ if (ret == 0 || ret == -ENOENT) {
pr_warning("Specified source line is not found.\n");
return -ENOENT;
} else if (ret < 0) {
- pr_warning("Debuginfo analysis failed. (%d)\n", ret);
+ pr_warning("Debuginfo analysis failed.\n");
return ret;
}

@@ -641,7 +641,7 @@ static int __show_line_range(struct line_range *lr, const char *module)
ret = get_real_path(tmp, lr->comp_dir, &lr->path);
free(tmp); /* Free old path */
if (ret < 0) {
- pr_warning("Failed to find source file. (%d)\n", ret);
+ pr_warning("Failed to find source file path.\n");
return ret;
}

2014-06-21 18:18:50

by Patrick Palka

[permalink] [raw]
Subject: Re: [PATCH -tip 1/4] perf/probe: Improve error message for unknown member of data structure

On Fri, Jun 6, 2014 at 3:13 AM, Masami Hiramatsu
<[email protected]> wrote:
> Improve the error message if we can not find given member in
> the given structure. Currently perf probe shows a wrong error
> message as below.
>
> -----
> # perf probe getname_flags:65 "result->BOGUS"
> result(type:filename) has no member BOGUS.
> Failed to find 'result' in this function.
> Error: Failed to add events. (-22)
> -----
>
> The first message is correct, but the second one is not, since
> we didn't fail to find a variable but fails to find the member
> of given variable.
>
> -----
> # perf probe getname_flags:65 "result->BOGUS"
> result(type:filename) has no member BOGUS.
> Error: Failed to add events. (-22)
> -----
>
> With this patch, the error message shows only the first one.
> And if we really failed to find given variable, it tells us so.
>
> -----
> # perf probe getname_flags:65 "BOGUS"
> Failed to find 'BOGUS' in this function.
> Error: Failed to add events. (-2)
> -----
>
> Signed-off-by: Masami Hiramatsu <[email protected]>
> Reported-by: Arnaldo Carvalho de Melo <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Paul Mackerras <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Namhyung Kim <[email protected]>
> Cc: Jiri Olsa <[email protected]>
> ---
> tools/perf/util/probe-finder.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index 9d8eb26..ce8faf4 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -573,14 +573,13 @@ static int find_variable(Dwarf_Die *sc_die, struct probe_finder *pf)
> if (!die_find_variable_at(sc_die, pf->pvar->var, pf->addr, &vr_die)) {
> /* Search again in global variables */
> if (!die_find_variable_at(&pf->cu_die, pf->pvar->var, 0, &vr_die))
> + pr_warning("Failed to find '%s' in this function.\n",
> + pf->pvar->var);
> ret = -ENOENT;

It looks like you're missing a pair of braces here.

Subject: Re: [PATCH -tip 1/4] perf/probe: Improve error message for unknown member of data structure

(2014/06/22 3:18), Patrick Palka wrote:
> On Fri, Jun 6, 2014 at 3:13 AM, Masami Hiramatsu
> <[email protected]> wrote:
>> Improve the error message if we can not find given member in
>> the given structure. Currently perf probe shows a wrong error
>> message as below.
>>
>> -----
>> # perf probe getname_flags:65 "result->BOGUS"
>> result(type:filename) has no member BOGUS.
>> Failed to find 'result' in this function.
>> Error: Failed to add events. (-22)
>> -----
>>
>> The first message is correct, but the second one is not, since
>> we didn't fail to find a variable but fails to find the member
>> of given variable.
>>
>> -----
>> # perf probe getname_flags:65 "result->BOGUS"
>> result(type:filename) has no member BOGUS.
>> Error: Failed to add events. (-22)
>> -----
>>
>> With this patch, the error message shows only the first one.
>> And if we really failed to find given variable, it tells us so.
>>
>> -----
>> # perf probe getname_flags:65 "BOGUS"
>> Failed to find 'BOGUS' in this function.
>> Error: Failed to add events. (-2)
>> -----
>>
>> Signed-off-by: Masami Hiramatsu <[email protected]>
>> Reported-by: Arnaldo Carvalho de Melo <[email protected]>
>> Cc: Peter Zijlstra <[email protected]>
>> Cc: Paul Mackerras <[email protected]>
>> Cc: Ingo Molnar <[email protected]>
>> Cc: Namhyung Kim <[email protected]>
>> Cc: Jiri Olsa <[email protected]>
>> ---
>> tools/perf/util/probe-finder.c | 5 ++---
>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
>> index 9d8eb26..ce8faf4 100644
>> --- a/tools/perf/util/probe-finder.c
>> +++ b/tools/perf/util/probe-finder.c
>> @@ -573,14 +573,13 @@ static int find_variable(Dwarf_Die *sc_die, struct probe_finder *pf)
>> if (!die_find_variable_at(sc_die, pf->pvar->var, pf->addr, &vr_die)) {
>> /* Search again in global variables */
>> if (!die_find_variable_at(&pf->cu_die, pf->pvar->var, 0, &vr_die))
>> + pr_warning("Failed to find '%s' in this function.\n",
>> + pf->pvar->var);
>> ret = -ENOENT;
>
> It looks like you're missing a pair of braces here.

Oops, right! Thank you very much!

--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]

Subject: [PATCH -tip ] [BUGFIX]: Fix to add a missing pair of braces for error path.

Fix to add a missing pair of braces for error path.
Commit 36d789a4d75f (perf probe: Improve error message for
unknown member of data structure) introduced this bug.

Without this fix, defining an event with global variables
is always failed, because it always returns -ENOENT if
the argument is not a local variable.

----
# perf probe -na "vfs_read smp_found_config"
Error: Failed to add events.
----

With this fix, you can set a global variable for the
argument of new event.

----
# perf probe -na "vfs_read smp_found_config"
Added new event:
probe:vfs_read (on vfs_read with smp_found_config)

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

perf record -e probe:vfs_read -aR sleep 1
----

Signed-off-by: Masami Hiramatsu <[email protected]>
Reported-by: Patrick Palka <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
---
tools/perf/util/probe-finder.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 98e3047..10560fc 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -572,10 +572,12 @@ static int find_variable(Dwarf_Die *sc_die, struct probe_finder *pf)
/* Search child die for local variables and parameters. */
if (!die_find_variable_at(sc_die, pf->pvar->var, pf->addr, &vr_die)) {
/* Search again in global variables */
- if (!die_find_variable_at(&pf->cu_die, pf->pvar->var, 0, &vr_die))
+ if (!die_find_variable_at(&pf->cu_die, pf->pvar->var, 0,
+ &vr_die)) {
pr_warning("Failed to find '%s' in this function.\n",
pf->pvar->var);
ret = -ENOENT;
+ }
}
if (ret >= 0)
ret = convert_variable(&vr_die, pf);

2014-06-24 07:40:57

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCH -tip ] [BUGFIX]: Fix to add a missing pair of braces for error path.

Hi Masami,

On Mon, 23 Jun 2014 03:17:12 +0000, Masami Hiramatsu wrote:
> Fix to add a missing pair of braces for error path.
> Commit 36d789a4d75f (perf probe: Improve error message for
> unknown member of data structure) introduced this bug.
>
> Without this fix, defining an event with global variables
> is always failed, because it always returns -ENOENT if
> the argument is not a local variable.
>
> ----
> # perf probe -na "vfs_read smp_found_config"
> Error: Failed to add events.
> ----
>
> With this fix, you can set a global variable for the
> argument of new event.
>
> ----
> # perf probe -na "vfs_read smp_found_config"
> Added new event:
> probe:vfs_read (on vfs_read with smp_found_config)
>
> You can now use it in all perf tools, such as:
>
> perf record -e probe:vfs_read -aR sleep 1
> ----
>
> Signed-off-by: Masami Hiramatsu <[email protected]>
> Reported-by: Patrick Palka <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Paul Mackerras <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Arnaldo Carvalho de Melo <[email protected]>
> Cc: Jiri Olsa <[email protected]>

Acked-by: Namhyung Kim <[email protected]>

Thanks,
Namhyung

2014-06-24 09:09:00

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH -tip ] [BUGFIX]: Fix to add a missing pair of braces for error path.

On Mon, Jun 23, 2014 at 03:17:12AM +0000, Masami Hiramatsu wrote:
> Fix to add a missing pair of braces for error path.
> Commit 36d789a4d75f (perf probe: Improve error message for
> unknown member of data structure) introduced this bug.
>
> Without this fix, defining an event with global variables
> is always failed, because it always returns -ENOENT if
> the argument is not a local variable.
>
> ----
> # perf probe -na "vfs_read smp_found_config"
> Error: Failed to add events.
> ----
>
> With this fix, you can set a global variable for the
> argument of new event.
>
> ----
> # perf probe -na "vfs_read smp_found_config"
> Added new event:
> probe:vfs_read (on vfs_read with smp_found_config)
>
> You can now use it in all perf tools, such as:
>
> perf record -e probe:vfs_read -aR sleep 1
> ----

queued for perf/urgent

thanks,
jirka