[PATCH 1/2] perf tools: fix handling of zero-length symbols.

[PATCH 1/2] perf tools: fix handling of zero-length symbols.

[Chris Phlipot]

This change introduces a fix to symbols__find, so that it is able to find
symbols of length zero (where start==end)

The current code has the following problem:
-The current implementation of symbols__find is unable to find any symbols
 of length zero.
-The db-export framework explicitly creates zero length symbols at
 locations where no symbol currently exists.

The combination of the two above behaviors results in behavior similar to
the example below.

1. addr_location is created for a sample, but symbol is unable to be
   resolved.
2. db export creates an "unknown" symbol of length zero at that address
   and inserts it into the dso.
3. A new sample comes in at the same address, but symbol__find is unable
   to find the zero length symbol, so it is still unresolved.
4. db export sees the symbol is unresolved, and allocated a duplicate
   symbol, even though it already did this in step 2.

This behavior continues every time an address without symbol information
is seen, which causes a very large number of these symbols to be
allocated.

The effect of this fix can be observed by looking at the contents of an
exported database before/after the fix
(generated with scripts/python/export-to-postgresql.py)

Ex.
BEFORE THE CHANGE:
  example_db=# select count(*) from symbols;
   count
  --------
   900213
  (1 row)

  example_db=# select count(*) from symbols where symbols.name='unknown';
   count
  --------
   897355
  (1 row)

  example_db=# select count(*) from symbols where symbols.name!='unknown';
   count
  -------
    2858
  (1 row)

AFTER THE CHANGE:

  example_db=# select count(*) from symbols;
   count
  -------
   25217
  (1 row)

  example_db=# select count(*) from symbols where name='unknown';
   count
  -------
   22359
  (1 row)

  example_db=# select count(*) from symbols where name!='unknown';
   count
  -------
    2858
  (1 row)

Signed-off-by: Chris Phlipot <cphlipot0@gmail.com>
---
 tools/perf/util/symbol.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 415c4f6..e42bf9a 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -299,7 +299,9 @@ static struct symbol *symbols__find(struct rb_root *symbols, u64 ip)
 	while (n) {
 		struct symbol *s = rb_entry(n, struct symbol, rb_node);
 
-		if (ip < s->start)
+		if (ip == s->start && s->start == s->end)
+			return s;
+		else if (ip < s->start)
 			n = n->rb_left;
 		else if (ip >= s->end)
 			n = n->rb_right;
-- 
2.7.4

[PATCH 2/2] perf script: fix incorrect python db-export error message

[Chris Phlipot]

fix the error message printed when attempting and failing to create the
call path root incorrectly references the call return process.

this change fixes the message to properly reference the failure to create
the call path root.

Signed-off-by: Chris Phlipot <cphlipot0@gmail.com>
---
 tools/perf/util/scripting-engines/trace-event-python.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
index 091bce6..1546b74 100644
--- a/tools/perf/util/scripting-engines/trace-event-python.c
+++ b/tools/perf/util/scripting-engines/trace-event-python.c
@@ -1062,7 +1062,7 @@ static void set_table_handlers(struct tables *tables)
 			tables->dbe.cpr = call_path_root__new();
 
 		if (!tables->dbe.cpr)
-			Py_FatalError("failed to create calls processor");
+			Py_FatalError("failed to create call path root");
 	}
 
 	tables->db_export_mode = true;
-- 
2.7.4

Re: [PATCH 1/2] perf tools: fix handling of zero-length symbols.

[Arnaldo Carvalho de Melo]

Em Sat, May 07, 2016 at 02:16:59AM -0700, Chris Phlipot escreveu:
> This change introduces a fix to symbols__find, so that it is able to find
> symbols of length zero (where start==end)
> 
> The current code has the following problem:
> -The current implementation of symbols__find is unable to find any symbols
>  of length zero.
> -The db-export framework explicitly creates zero length symbols at
>  locations where no symbol currently exists.
> 
> The combination of the two above behaviors results in behavior similar to
> the example below.

Ok, but you made the unlikely case be the first test, how about this one
liner instead?

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 415c4f6d98fd..7a0917569fb3 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -301,7 +301,7 @@ static struct symbol *symbols__find(struct rb_root *symbols, u64 ip)
 
 		if (ip < s->start)
 			n = n->rb_left;
-		else if (ip >= s->end)
+		else if (ip > s->end || (ip == s->end && ip != s->start)
 			n = n->rb_right;
 		else
 			return s;


> +++ b/tools/perf/util/symbol.c
> @@ -299,7 +299,9 @@ static struct symbol *symbols__find(struct rb_root *symbols, u64 ip)
>  	while (n) {
>  		struct symbol *s = rb_entry(n, struct symbol, rb_node);
>  
> -		if (ip < s->start)
> +		if (ip == s->start && s->start == s->end)
> +			return s;
> +		else if (ip < s->start)
>  			n = n->rb_left;
>  		else if (ip >= s->end)
>  			n = n->rb_right;
> -- 
> 2.7.4

Re: [PATCH 1/2] perf tools: fix handling of zero-length symbols.

[Chris Phlipot]

> On May 9, 2016, at 10:06 AM, Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> wrote:
> 
> Em Sat, May 07, 2016 at 02:16:59AM -0700, Chris Phlipot escreveu:
>> This change introduces a fix to symbols__find, so that it is able to find
>> symbols of length zero (where start==end)
>> 
>> The current code has the following problem:
>> -The current implementation of symbols__find is unable to find any symbols
>> of length zero.
>> -The db-export framework explicitly creates zero length symbols at
>> locations where no symbol currently exists.
>> 
>> The combination of the two above behaviors results in behavior similar to
>> the example below.
> 
> Ok, but you made the unlikely case be the first test, how about this one
> liner instead?
> 
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 415c4f6d98fd..7a0917569fb3 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -301,7 +301,7 @@ static struct symbol *symbols__find(struct rb_root *symbols, u64 ip)
> 
>        if (ip < s->start)
>            n = n->rb_left;
> -        else if (ip >= s->end)
> +        else if (ip > s->end || (ip == s->end && ip != s->start)
>            n = n->rb_right;
>        else
>            return s;
> 
> 

I agree. This looks like a better fix.

>> +++ b/tools/perf/util/symbol.c
>> @@ -299,7 +299,9 @@ static struct symbol *symbols__find(struct rb_root *symbols, u64 ip)
>>    while (n) {
>>        struct symbol *s = rb_entry(n, struct symbol, rb_node);
>> 
>> -        if (ip < s->start)
>> +        if (ip == s->start && s->start == s->end)
>> +            return s;
>> +        else if (ip < s->start)
>>            n = n->rb_left;
>>        else if (ip >= s->end)
>>            n = n->rb_right;
>> -- 
>> 2.7.4

[tip:perf/core] perf script: Fix incorrect python db-export error message

[tip-bot for Chris Phlipot]

Commit-ID:  aff633406ca2772554bad7b37f2dfbc409b6ea74
Gitweb:     http://git.kernel.org/tip/aff633406ca2772554bad7b37f2dfbc409b6ea74
Author:     Chris Phlipot <cphlipot0@gmail.com>
AuthorDate: Sat, 7 May 2016 02:17:00 -0700
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 9 May 2016 14:08:39 -0300

perf script: Fix incorrect python db-export error message

Fix the error message printed when attempting and failing to create the
call path root incorrectly references the call return process.

This change fixes the message to properly reference the failure to
create the call path root.

Signed-off-by: Chris Phlipot <cphlipot0@gmail.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1462612620-25008-2-git-send-email-cphlipot0@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/scripting-engines/trace-event-python.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
index 091bce6..1546b74 100644
--- a/tools/perf/util/scripting-engines/trace-event-python.c
+++ b/tools/perf/util/scripting-engines/trace-event-python.c
@@ -1062,7 +1062,7 @@ static void set_table_handlers(struct tables *tables)
 			tables->dbe.cpr = call_path_root__new();
 
 		if (!tables->dbe.cpr)
-			Py_FatalError("failed to create calls processor");
+			Py_FatalError("failed to create call path root");
 	}
 
 	tables->db_export_mode = true;

[tip:perf/core] perf symbols: Fix handling of zero-length symbols.

[tip-bot for Chris Phlipot]

Commit-ID:  9c7b37cd63d0d910c531233209286f169993cbd9
Gitweb:     http://git.kernel.org/tip/9c7b37cd63d0d910c531233209286f169993cbd9
Author:     Chris Phlipot <cphlipot0@gmail.com>
AuthorDate: Sat, 7 May 2016 02:16:59 -0700
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 9 May 2016 18:40:03 -0300

perf symbols: Fix handling of zero-length symbols.

This change introduces a fix to symbols__find, so that it is able to
find symbols of length zero (where start == end).

The current code has the following problem:

- The current implementation of symbols__find is unable to find any symbols
  of length zero.

- The db-export framework explicitly creates zero length symbols at
  locations where no symbol currently exists.

The combination of the two above behaviors results in behavior similar
to the example below.

1. addr_location is created for a sample, but symbol is unable to be
   resolved.

2. db export creates an "unknown" symbol of length zero at that address
   and inserts it into the dso.

3. A new sample comes in at the same address, but symbol__find is unable
   to find the zero length symbol, so it is still unresolved.

4. db export sees the symbol is unresolved, and allocated a duplicate
   symbol, even though it already did this in step 2.

This behavior continues every time an address without symbol information
is seen, which causes a very large number of these symbols to be
allocated.

The effect of this fix can be observed by looking at the contents of an
exported database before/after the fix (generated with
scripts/python/export-to-postgresql.py)

Ex.
BEFORE THE CHANGE:

  example_db=# select count(*) from symbols;
   count
  --------
   900213
  (1 row)

  example_db=# select count(*) from symbols where symbols.name='unknown';
   count
  --------
   897355
  (1 row)

  example_db=# select count(*) from symbols where symbols.name!='unknown';
   count
  -------
    2858
  (1 row)

AFTER THE CHANGE:

  example_db=# select count(*) from symbols;
   count
  -------
   25217
  (1 row)

  example_db=# select count(*) from symbols where name='unknown';
   count
  -------
   22359
  (1 row)

  example_db=# select count(*) from symbols where name!='unknown';
   count
  -------
    2858
  (1 row)

Signed-off-by: Chris Phlipot <cphlipot0@gmail.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1462612620-25008-1-git-send-email-cphlipot0@gmail.com
[ Moved the test to later in the rb_tree tests, as this not the likely case ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/symbol.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 415c4f6..2946295 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -301,7 +301,7 @@ static struct symbol *symbols__find(struct rb_root *symbols, u64 ip)
 
 		if (ip < s->start)
 			n = n->rb_left;
-		else if (ip >= s->end)
+		else if (ip > s->end || (ip == s->end && ip != s->start))
 			n = n->rb_right;
 		else
 			return s;