Commit 4c859351226c920b227fec040a3b447f0d482af3 ("perf probe: Support
glob wildcards for function name") introduces a problem:
# /root/perf probe kmem_cache_free
Failed to find symbol kmem_cache_free in kernel
Error: Failed to add events.
The reason is the replacement of map__for_each_symbol_by_name() (by
map__for_each_symbol()). Although their names are similay,
map__for_each_symbol doesn't call map__load() and dso__sort_by_name()
before searching. The missing of map__load() causes this problem
because it search symbol before load dso map.
This patch ensures map__load() is called before using
map__for_each_symbol().
After this patch:
# /root/perf probe kmem_cache_free
Added new event:
probe:kmem_cache_free (on kmem_cache_free%return)
You can now use it in all perf tools, such as:
perf record -e probe:kmem_cache_free -aR sleep 1
Signed-off-by: Wang Nan <[email protected]>
---
tools/perf/util/probe-event.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 2399dc4..1faa1e6 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2499,6 +2499,9 @@ static int find_probe_functions(struct map *map, char *name)
struct symbol *sym;
struct rb_node *tmp;
+ if (map__load(map, NULL) < 0)
+ return 0;
+
map__for_each_symbol(map, sym, tmp) {
if (strglobmatch(sym->name, name))
found++;
--
1.8.3.4
On 2015/05/15 21:14, Wang Nan wrote:
> Commit 4c859351226c920b227fec040a3b447f0d482af3 ("perf probe: Support
> glob wildcards for function name") introduces a problem:
>
> # /root/perf probe kmem_cache_free
> Failed to find symbol kmem_cache_free in kernel
> Error: Failed to add events.
>
> The reason is the replacement of map__for_each_symbol_by_name() (by
> map__for_each_symbol()). Although their names are similay,
> map__for_each_symbol doesn't call map__load() and dso__sort_by_name()
> before searching. The missing of map__load() causes this problem
> because it search symbol before load dso map.
Oops, yes. But I think we'd better fix map__for_each_symbol() to call
map__load() if the map is not loaded, doesn't it?
Thank you,
>
> This patch ensures map__load() is called before using
> map__for_each_symbol().
>
> After this patch:
>
> # /root/perf probe kmem_cache_free
> Added new event:
> probe:kmem_cache_free (on kmem_cache_free%return)
>
> You can now use it in all perf tools, such as:
>
> perf record -e probe:kmem_cache_free -aR sleep 1
>
> Signed-off-by: Wang Nan <[email protected]>
> ---
> tools/perf/util/probe-event.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 2399dc4..1faa1e6 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -2499,6 +2499,9 @@ static int find_probe_functions(struct map *map, char *name)
> struct symbol *sym;
> struct rb_node *tmp;
>
> + if (map__load(map, NULL) < 0)
> + return 0;
> +
> map__for_each_symbol(map, sym, tmp) {
> if (strglobmatch(sym->name, name))
> found++;
>
--
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]
Em Sat, May 16, 2015 at 12:15:45AM +0900, Masami Hiramatsu escreveu:
> On 2015/05/15 21:14, Wang Nan wrote:
> > Commit 4c859351226c920b227fec040a3b447f0d482af3 ("perf probe: Support
> > glob wildcards for function name") introduces a problem:
> >
> > # /root/perf probe kmem_cache_free
> > Failed to find symbol kmem_cache_free in kernel
> > Error: Failed to add events.
> >
> > The reason is the replacement of map__for_each_symbol_by_name() (by
> > map__for_each_symbol()). Although their names are similay,
> > map__for_each_symbol doesn't call map__load() and dso__sort_by_name()
> > before searching. The missing of map__load() causes this problem
> > because it search symbol before load dso map.
>
> Oops, yes. But I think we'd better fix map__for_each_symbol() to call
> map__load() if the map is not loaded, doesn't it?
Well, that is something for a follow up patch, I'm applying his patch as
it fixes a problem and is trivially correct.
- Arnaldo
> Thank you,
>
> >
> > This patch ensures map__load() is called before using
> > map__for_each_symbol().
> >
> > After this patch:
> >
> > # /root/perf probe kmem_cache_free
> > Added new event:
> > probe:kmem_cache_free (on kmem_cache_free%return)
> >
> > You can now use it in all perf tools, such as:
> >
> > perf record -e probe:kmem_cache_free -aR sleep 1
> >
> > Signed-off-by: Wang Nan <[email protected]>
> > ---
> > tools/perf/util/probe-event.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> > index 2399dc4..1faa1e6 100644
> > --- a/tools/perf/util/probe-event.c
> > +++ b/tools/perf/util/probe-event.c
> > @@ -2499,6 +2499,9 @@ static int find_probe_functions(struct map *map, char *name)
> > struct symbol *sym;
> > struct rb_node *tmp;
> >
> > + if (map__load(map, NULL) < 0)
> > + return 0;
> > +
> > map__for_each_symbol(map, sym, tmp) {
> > if (strglobmatch(sym->name, name))
> > found++;
> >
>
>
> --
> 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]
On 2015/05/16 5:07, Arnaldo Carvalho de Melo wrote:
> Em Sat, May 16, 2015 at 12:15:45AM +0900, Masami Hiramatsu escreveu:
>> On 2015/05/15 21:14, Wang Nan wrote:
>>> Commit 4c859351226c920b227fec040a3b447f0d482af3 ("perf probe: Support
>>> glob wildcards for function name") introduces a problem:
>>>
>>> # /root/perf probe kmem_cache_free
>>> Failed to find symbol kmem_cache_free in kernel
>>> Error: Failed to add events.
>>>
>>> The reason is the replacement of map__for_each_symbol_by_name() (by
>>> map__for_each_symbol()). Although their names are similay,
>>> map__for_each_symbol doesn't call map__load() and dso__sort_by_name()
>>> before searching. The missing of map__load() causes this problem
>>> because it search symbol before load dso map.
>>
>> Oops, yes. But I think we'd better fix map__for_each_symbol() to call
>> map__load() if the map is not loaded, doesn't it?
>
> Well, that is something for a follow up patch, I'm applying his patch as
> it fixes a problem and is trivially correct.
OK, then I'm OK for this patch.
Thank you,
>
> - Arnaldo
>
>> Thank you,
>>
>>>
>>> This patch ensures map__load() is called before using
>>> map__for_each_symbol().
>>>
>>> After this patch:
>>>
>>> # /root/perf probe kmem_cache_free
>>> Added new event:
>>> probe:kmem_cache_free (on kmem_cache_free%return)
>>>
>>> You can now use it in all perf tools, such as:
>>>
>>> perf record -e probe:kmem_cache_free -aR sleep 1
>>>
>>> Signed-off-by: Wang Nan <[email protected]>
>>> ---
>>> tools/perf/util/probe-event.c | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
>>> index 2399dc4..1faa1e6 100644
>>> --- a/tools/perf/util/probe-event.c
>>> +++ b/tools/perf/util/probe-event.c
>>> @@ -2499,6 +2499,9 @@ static int find_probe_functions(struct map *map, char *name)
>>> struct symbol *sym;
>>> struct rb_node *tmp;
>>>
>>> + if (map__load(map, NULL) < 0)
>>> + return 0;
>>> +
>>> map__for_each_symbol(map, sym, tmp) {
>>> if (strglobmatch(sym->name, name))
>>> found++;
>>>
>>
--
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]
Em Sat, May 16, 2015 at 09:10:59PM +0900, Masami Hiramatsu escreveu:
> On 2015/05/16 5:07, Arnaldo Carvalho de Melo wrote:
> > Em Sat, May 16, 2015 at 12:15:45AM +0900, Masami Hiramatsu escreveu:
> >> On 2015/05/15 21:14, Wang Nan wrote:
> >>> Commit 4c859351226c920b227fec040a3b447f0d482af3 ("perf probe: Support
> >>> glob wildcards for function name") introduces a problem:
> >>>
> >>> # /root/perf probe kmem_cache_free
> >>> Failed to find symbol kmem_cache_free in kernel
> >>> Error: Failed to add events.
> >>>
> >>> The reason is the replacement of map__for_each_symbol_by_name() (by
> >>> map__for_each_symbol()). Although their names are similay,
> >>> map__for_each_symbol doesn't call map__load() and dso__sort_by_name()
> >>> before searching. The missing of map__load() causes this problem
> >>> because it search symbol before load dso map.
> >>
> >> Oops, yes. But I think we'd better fix map__for_each_symbol() to call
> >> map__load() if the map is not loaded, doesn't it?
> >
> > Well, that is something for a follow up patch, I'm applying his patch as
> > it fixes a problem and is trivially correct.
>
> OK, then I'm OK for this patch.
Ok, tranforming the above statement into an Acked-by, ok?
> Thank you,
>
> >
> > - Arnaldo
> >
> >> Thank you,
> >>
> >>>
> >>> This patch ensures map__load() is called before using
> >>> map__for_each_symbol().
> >>>
> >>> After this patch:
> >>>
> >>> # /root/perf probe kmem_cache_free
> >>> Added new event:
> >>> probe:kmem_cache_free (on kmem_cache_free%return)
> >>>
> >>> You can now use it in all perf tools, such as:
> >>>
> >>> perf record -e probe:kmem_cache_free -aR sleep 1
> >>>
> >>> Signed-off-by: Wang Nan <[email protected]>
> >>> ---
> >>> tools/perf/util/probe-event.c | 3 +++
> >>> 1 file changed, 3 insertions(+)
> >>>
> >>> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> >>> index 2399dc4..1faa1e6 100644
> >>> --- a/tools/perf/util/probe-event.c
> >>> +++ b/tools/perf/util/probe-event.c
> >>> @@ -2499,6 +2499,9 @@ static int find_probe_functions(struct map *map, char *name)
> >>> struct symbol *sym;
> >>> struct rb_node *tmp;
> >>>
> >>> + if (map__load(map, NULL) < 0)
> >>> + return 0;
> >>> +
> >>> map__for_each_symbol(map, sym, tmp) {
> >>> if (strglobmatch(sym->name, name))
> >>> found++;
> >>>
> >>
> --
> 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]
On 2015/05/18 22:17, Arnaldo Carvalho de Melo wrote:
> Em Sat, May 16, 2015 at 09:10:59PM +0900, Masami Hiramatsu escreveu:
>> On 2015/05/16 5:07, Arnaldo Carvalho de Melo wrote:
>>> Em Sat, May 16, 2015 at 12:15:45AM +0900, Masami Hiramatsu escreveu:
>>>> On 2015/05/15 21:14, Wang Nan wrote:
>>>>> Commit 4c859351226c920b227fec040a3b447f0d482af3 ("perf probe: Support
>>>>> glob wildcards for function name") introduces a problem:
>>>>>
>>>>> # /root/perf probe kmem_cache_free
>>>>> Failed to find symbol kmem_cache_free in kernel
>>>>> Error: Failed to add events.
>>>>>
>>>>> The reason is the replacement of map__for_each_symbol_by_name() (by
>>>>> map__for_each_symbol()). Although their names are similay,
>>>>> map__for_each_symbol doesn't call map__load() and dso__sort_by_name()
>>>>> before searching. The missing of map__load() causes this problem
>>>>> because it search symbol before load dso map.
>>>>
>>>> Oops, yes. But I think we'd better fix map__for_each_symbol() to call
>>>> map__load() if the map is not loaded, doesn't it?
>>>
>>> Well, that is something for a follow up patch, I'm applying his patch as
>>> it fixes a problem and is trivially correct.
>>
>> OK, then I'm OK for this patch.
>
> Ok, tranforming the above statement into an Acked-by, ok?
Yes, here is my ack :)
Acked-by: Masami Hiramatsu <[email protected]>
And also, I've found that including map__load() in map__for_each_symbol()
is not good idea. Also I think map__for_each_symbol_by_name() should
not call map__load(), since the "filter" argument will be ignored if
the map is already loaded. This means that the iterated elements can
be changed.
So, obviously the map should be loaded before calling map__for_each_XXX.
Thank you,
>
>> Thank you,
>>
>>>
>>> - Arnaldo
>>>
>>>> Thank you,
>>>>
>>>>>
>>>>> This patch ensures map__load() is called before using
>>>>> map__for_each_symbol().
>>>>>
>>>>> After this patch:
>>>>>
>>>>> # /root/perf probe kmem_cache_free
>>>>> Added new event:
>>>>> probe:kmem_cache_free (on kmem_cache_free%return)
>>>>>
>>>>> You can now use it in all perf tools, such as:
>>>>>
>>>>> perf record -e probe:kmem_cache_free -aR sleep 1
>>>>>
>>>>> Signed-off-by: Wang Nan <[email protected]>
>>>>> ---
>>>>> tools/perf/util/probe-event.c | 3 +++
>>>>> 1 file changed, 3 insertions(+)
>>>>>
>>>>> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
>>>>> index 2399dc4..1faa1e6 100644
>>>>> --- a/tools/perf/util/probe-event.c
>>>>> +++ b/tools/perf/util/probe-event.c
>>>>> @@ -2499,6 +2499,9 @@ static int find_probe_functions(struct map *map, char *name)
>>>>> struct symbol *sym;
>>>>> struct rb_node *tmp;
>>>>>
>>>>> + if (map__load(map, NULL) < 0)
>>>>> + return 0;
>>>>> +
>>>>> map__for_each_symbol(map, sym, tmp) {
>>>>> if (strglobmatch(sym->name, name))
>>>>> found++;
>>>>>
>>>>
>> --
>> 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]
>
--
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]
Em Tue, May 19, 2015 at 08:41:30AM +0900, Masami Hiramatsu escreveu:
> On 2015/05/18 22:17, Arnaldo Carvalho de Melo wrote:
> > Em Sat, May 16, 2015 at 09:10:59PM +0900, Masami Hiramatsu escreveu:
> >> On 2015/05/16 5:07, Arnaldo Carvalho de Melo wrote:
> >> OK, then I'm OK for this patch.
> >
> > Ok, tranforming the above statement into an Acked-by, ok?
>
> Yes, here is my ack :)
>
> Acked-by: Masami Hiramatsu <[email protected]>
>
>
> And also, I've found that including map__load() in map__for_each_symbol()
> is not good idea. Also I think map__for_each_symbol_by_name() should
> not call map__load(), since the "filter" argument will be ignored if
> the map is already loaded. This means that the iterated elements can
> be changed.
> So, obviously the map should be loaded before calling map__for_each_XXX.
Right, agreed with your comments, and that filter thing needs moving to
some other suitable place, like setting a filter at symbol system start,
etc.
- Arnaldo
> Thank you,
>
> >
> >> Thank you,
> >>
> >>>
> >>> - Arnaldo
> >>>
> >>>> Thank you,
> >>>>
> >>>>>
> >>>>> This patch ensures map__load() is called before using
> >>>>> map__for_each_symbol().
> >>>>>
> >>>>> After this patch:
> >>>>>
> >>>>> # /root/perf probe kmem_cache_free
> >>>>> Added new event:
> >>>>> probe:kmem_cache_free (on kmem_cache_free%return)
> >>>>>
> >>>>> You can now use it in all perf tools, such as:
> >>>>>
> >>>>> perf record -e probe:kmem_cache_free -aR sleep 1
> >>>>>
> >>>>> Signed-off-by: Wang Nan <[email protected]>
> >>>>> ---
> >>>>> tools/perf/util/probe-event.c | 3 +++
> >>>>> 1 file changed, 3 insertions(+)
> >>>>>
> >>>>> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> >>>>> index 2399dc4..1faa1e6 100644
> >>>>> --- a/tools/perf/util/probe-event.c
> >>>>> +++ b/tools/perf/util/probe-event.c
> >>>>> @@ -2499,6 +2499,9 @@ static int find_probe_functions(struct map *map, char *name)
> >>>>> struct symbol *sym;
> >>>>> struct rb_node *tmp;
> >>>>>
> >>>>> + if (map__load(map, NULL) < 0)
> >>>>> + return 0;
> >>>>> +
> >>>>> map__for_each_symbol(map, sym, tmp) {
> >>>>> if (strglobmatch(sym->name, name))
> >>>>> found++;
> >>>>>
> >>>>
> >> --
> >> 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]
> >
>
>
> --
> 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]
Commit-ID: 75e4a2a6af15e956993913314ced2582b350a647
Gitweb: http://git.kernel.org/tip/75e4a2a6af15e956993913314ced2582b350a647
Author: Wang Nan <[email protected]>
AuthorDate: Fri, 15 May 2015 12:14:44 +0000
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Mon, 18 May 2015 10:16:48 -0300
perf probe: Load map before glob matching
Commit 4c859351226c920b227fec040a3b447f0d482af3 ("perf probe: Support
glob wildcards for function name") introduces a problem:
# /root/perf probe kmem_cache_free
Failed to find symbol kmem_cache_free in kernel
Error: Failed to add events.
The reason is the replacement of map__for_each_symbol_by_name() (by
map__for_each_symbol()). Although their names are similar,
map__for_each_symbol doesn't call map__load() and dso__sort_by_name()
before searching. The missing of map__load() causes this problem because
it search symbol before load dso map.
This patch ensures map__load() is called before using
map__for_each_symbol().
After this patch:
# /root/perf probe kmem_cache_free
Added new event:
probe:kmem_cache_free (on kmem_cache_free%return)
You can now use it in all perf tools, such as:
perf record -e probe:kmem_cache_free -aR sleep 1
Signed-off-by: Wang Nan <[email protected]>
Acked-by: Masami Hiramatsu <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Zefan Li <[email protected]>
Cc: [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 | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 2399dc4..1faa1e6 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2499,6 +2499,9 @@ static int find_probe_functions(struct map *map, char *name)
struct symbol *sym;
struct rb_node *tmp;
+ if (map__load(map, NULL) < 0)
+ return 0;
+
map__for_each_symbol(map, sym, tmp) {
if (strglobmatch(sym->name, name))
found++;