[PATCH] perf: Fix symbol resolution on old ppc64 ABI

[PATCH] perf: Fix symbol resolution on old ppc64 ABI

[Anton Blanchard]

The synthetic symbol creation code has an issue with the old ppc64
ABI. We end up with duplicate symbols of different sizes that overlap.

To fix this, walk all of the symbols and remove any duplicates that
are the length of a function descriptor.

Before:     
     
     5.46%      ppc64_cpu  [kernel.kallsyms]     [k] 0xc0000000003a2ddc
                     |
                     --- 0xc0000000003a2dc0
                        |
                        |--99.99%-- 0xc0000000003a2dd4
                        |          |
                        |          |--99.40%-- .cpu_node_mask

After:

     5.51%      ppc64_cpu  [kernel.kallsyms]     [k] .__bitmap_weight
                     |
                     --- .__bitmap_weight
                        |          
                        |--99.41%-- .__bitmap_weight
                        |          |          
                        |          |--89.55%-- .cpu_node_mask

Signed-off-by: Anton Blanchard <anton@samba.org>
---

I'd prefer not to add a ppc64 specific hack here, but I'm not sure
how we can fix this in a simpler way.

Index: linux-2.6-tip/tools/perf/util/symbol.c
===================================================================
--- linux-2.6-tip.orig/tools/perf/util/symbol.c	2011-07-24 15:06:14.423324103 +1000
+++ linux-2.6-tip/tools/perf/util/symbol.c	2011-07-24 15:21:42.149736397 +1000
@@ -107,6 +107,58 @@ static void symbols__fixup_end(struct rb
 		curr->end = roundup(curr->start, 4096);
 }
 
+/*
+ * There are two 64bit PowerPC ABIs. The old version creates symbols at the
+ * first instruction of each function by adding a '.' to the start of a symbol
+ * name. A function descriptor called foo has a a symbol called .foo at the
+ * address of the first instruction.
+ *
+ * The new version doesn't create these '.' symbols and instead requires us
+ * to synthesize symbols by parsing the function descriptor section (which
+ * provides a mapping of function name to instruction address, amongst other
+ * things).
+ *
+ * If we created synthetic symbols for an old style object then we end up
+ * with duplicate symbols. The '.' symbol that covers the entire function,
+ * and a fake synthetic symbol that is the length of a function descriptor.
+ *
+ * To fix this, we walk all of the symbols and remove any duplicates that
+ * are the length of a function descriptor.
+ */
+
+/* A ppc64 function descriptor contains 3 64bit values */
+#define PPC64_FUNCTION_DESCRIPTOR_LEN  (3 * sizeof(u64))
+
+static void symbols__fixup_ppc64(struct rb_root *symbols)
+{
+	struct rb_node *nd;
+	struct symbol *curr, *next;
+
+	nd = rb_first(symbols);
+
+	while (nd) {
+		curr = rb_entry(nd, struct symbol, rb_node);
+again:
+		nd = rb_next(&curr->rb_node);
+		next = rb_entry(nd, struct symbol, rb_node);
+
+		if (!nd)
+			break;
+
+		if (curr->start == next->start) {
+			if ((curr->end - curr->start + 1) ==
+					PPC64_FUNCTION_DESCRIPTOR_LEN) {
+				nd = rb_next(&curr->rb_node);
+				rb_erase(&curr->rb_node, symbols);
+			} else if ((next->end - next->start + 1) ==
+					PPC64_FUNCTION_DESCRIPTOR_LEN) {
+				rb_erase(&next->rb_node, symbols);
+				goto again;
+			}
+		}
+	}
+}
+
 static void __map_groups__fixup_end(struct map_groups *mg, enum map_type type)
 {
 	struct map *prev, *curr;
@@ -1278,6 +1330,9 @@ new_symbol:
 	 * For misannotated, zeroed, ASM function sizes.
 	 */
 	if (nr > 0) {
+		if (ehdr.e_machine == EM_PPC64)
+			symbols__fixup_ppc64(&dso->symbols[map->type]);
+
 		symbols__fixup_end(&dso->symbols[map->type]);
 		if (kmap) {
 			/*

Re: [PATCH] perf: Fix symbol resolution on old ppc64 ABI

[Ingo Molnar]

* Anton Blanchard <anton@samba.org> wrote:

> The synthetic symbol creation code has an issue with the old ppc64 
> ABI. We end up with duplicate symbols of different sizes that 
> overlap.
> 
> To fix this, walk all of the symbols and remove any duplicates that 
> are the length of a function descriptor.

> I'd prefer not to add a ppc64 specific hack here, but I'm not sure 
> how we can fix this in a simpler way.

Symbol space problems are common on other architectures as well.

We could use a heuristic: when symbols are overlapping then we could 
throw away the one that is smaller. This would implicitly cover the 
ppc64 case, right?

I'd also suggest we warn somewhere that a symbol has been thrown 
away, if verbosity is turned off. (i.e. don't warn by default.)

Thanks,

	Ingo

Re: [PATCH] perf: Fix symbol resolution on old ppc64 ABI

[Anton Blanchard]

Hi Ingo,

> > The synthetic symbol creation code has an issue with the old ppc64 
> > ABI. We end up with duplicate symbols of different sizes that 
> > overlap.
> > 
> > To fix this, walk all of the symbols and remove any duplicates that 
> > are the length of a function descriptor.
> 
> > I'd prefer not to add a ppc64 specific hack here, but I'm not sure 
> > how we can fix this in a simpler way.
> 
> Symbol space problems are common on other architectures as well.
> 
> We could use a heuristic: when symbols are overlapping then we could 
> throw away the one that is smaller. This would implicitly cover the 
> ppc64 case, right?
> 
> I'd also suggest we warn somewhere that a symbol has been thrown 
> away, if verbosity is turned off. (i.e. don't warn by default.)

It would cover most cases. There would be an issue with functions that
are less than 24 bytes in length (ie 6 instructions). Tiny functions
are probably rare enough that we can live with it.

Anton

Re: [PATCH] perf: Fix symbol resolution on old ppc64 ABI

[Ingo Molnar]

* Anton Blanchard <anton@samba.org> wrote:

> Hi Ingo,
> 
> > > The synthetic symbol creation code has an issue with the old ppc64 
> > > ABI. We end up with duplicate symbols of different sizes that 
> > > overlap.
> > > 
> > > To fix this, walk all of the symbols and remove any duplicates that 
> > > are the length of a function descriptor.
> > 
> > > I'd prefer not to add a ppc64 specific hack here, but I'm not sure 
> > > how we can fix this in a simpler way.
> > 
> > Symbol space problems are common on other architectures as well.
> > 
> > We could use a heuristic: when symbols are overlapping then we could 
> > throw away the one that is smaller. This would implicitly cover the 
> > ppc64 case, right?
> > 
> > I'd also suggest we warn somewhere that a symbol has been thrown 
> > away, if verbosity is turned off. (i.e. don't warn by default.)
> 
> It would cover most cases. There would be an issue with functions 
> that are less than 24 bytes in length (ie 6 instructions). Tiny 
> functions are probably rare enough that we can live with it.

Do such tiny functions get packed tightly, below 24 bytes? Another 
sanitization pass could solve that issue, to trim partially 
overlapping symbol regions, right?

Thanks,

	Ingo

Re: [PATCH] perf: Fix symbol resolution on old ppc64 ABI

[Anton Blanchard]

Hi Ingo,

> > It would cover most cases. There would be an issue with functions 
> > that are less than 24 bytes in length (ie 6 instructions). Tiny 
> > functions are probably rare enough that we can live with it.
> 
> Do such tiny functions get packed tightly, below 24 bytes? Another 
> sanitization pass could solve that issue, to trim partially 
> overlapping symbol regions, right?

That would work I think. Should I cook up a patch?

Anton

Re: [PATCH] perf: Fix symbol resolution on old ppc64 ABI

[Ingo Molnar]

* Anton Blanchard <anton@samba.org> wrote:

> Hi Ingo,
> 
> > > It would cover most cases. There would be an issue with 
> > > functions that are less than 24 bytes in length (ie 6 
> > > instructions). Tiny functions are probably rare enough that we 
> > > can live with it.
> > 
> > Do such tiny functions get packed tightly, below 24 bytes? 
> > Another sanitization pass could solve that issue, to trim 
> > partially overlapping symbol regions, right?
> 
> That would work I think. Should I cook up a patch?

Yeah, would be nice - if Arnaldo agrees with the approach too.

Thanks,

	Ingo