Re: callchain sampling bug in perf?

From: Frederic Weisbecker
Date: Sun Aug 22 2010 - 01:49:35 EST


On Sat, Aug 21, 2010 at 10:46:25AM -0400, Christoph Hellwig wrote:
> On Sat, Aug 21, 2010 at 10:42:39AM -0400, Christoph Hellwig wrote:
> > It does seem to fix the bug for some cases but not all. Default perf
> > report in TUI and the normal command line seem to get it right. perf
> > report -g flat still shows the old problem. perf report -g flat,0.0
> > shows callgraphs, but just as before they just show the 0.<n>
> > percentages.
>
> -g graph also shows the same issue of not beeing able to expand the
> python line. It seems like only the fractal case was fixed by that
> patch.
>


Now does it work with the patch in attachment?
It includes the previous one.

It still needs a few cleanups but the idea is there.
Ah and it applies in tip:perf/core but latest -linus
should be fine.

Thanks.


diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index f231f43..e0b5183 100644
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -379,7 +379,6 @@ static void filter_context(struct ip_callchain *old, struct resolved_chain *new,
new->nr = j;
}

-
int append_chain(struct callchain_node *root, struct ip_callchain *chain,
struct map_symbol *syms, u64 period)
{
@@ -404,3 +403,71 @@ end:

return 0;
}
+
+static int
+merge_chain_branch(struct callchain_node *dst, struct callchain_node *src,
+ struct ip_callchain *chain, struct map_symbol *syms)
+{
+ struct callchain_node *child;
+ struct callchain_list *list;
+ int old_pos = chain->nr;
+ int err = 0;
+
+ list_for_each_entry(list, &src->val, list) {
+ if (chain->nr == 4096) {
+ err = -ENOSPC;
+ goto out;
+ }
+
+ chain->ips[chain->nr] = list->ip;
+ syms[chain->nr] = list->ms;
+ chain->nr++;
+ }
+
+ if (src->hit)
+ err = append_chain(dst, chain, syms, src->hit);
+
+ if (!err) {
+ chain_for_each_child(child, src) {
+ err = merge_chain_branch(dst, child, chain, syms);
+ if (err)
+ break;
+ }
+ }
+
+out:
+ chain->nr = old_pos;
+
+ return err;
+}
+
+int merge_chain(struct callchain_node *dst, struct callchain_node *src)
+{
+ struct callchain_node *child;
+ struct ip_callchain *chain;
+ struct map_symbol *syms;
+ int err = 0;
+
+ chain = malloc(sizeof(*chain) + 4096 * sizeof(u64));
+ if (!chain)
+ return -ENOMEM;
+
+ chain->nr = 0;
+
+ syms = malloc(sizeof(*syms) * 4096);
+ if (!syms) {
+ free(chain);
+ return -ENOMEM;
+ }
+
+ chain_for_each_child(child, src) {
+ err = merge_chain_branch(dst, child, chain, syms);
+ if (err)
+ break;
+ }
+
+ free(chain);
+ free(syms);
+
+ return err;
+}
diff --git a/tools/perf/util/callchain.h b/tools/perf/util/callchain.h
index 624a96c..da5cd09 100644
--- a/tools/perf/util/callchain.h
+++ b/tools/perf/util/callchain.h
@@ -50,6 +50,7 @@ static inline void callchain_init(struct callchain_node *node)
INIT_LIST_HEAD(&node->children);
INIT_LIST_HEAD(&node->val);

+ node->children_hit = 0;
node->parent = NULL;
node->hit = 0;
}
@@ -62,6 +63,7 @@ static inline u64 cumul_hits(struct callchain_node *node)
int register_callchain_param(struct callchain_param *param);
int append_chain(struct callchain_node *root, struct ip_callchain *chain,
struct map_symbol *syms, u64 period);
+int merge_chain(struct callchain_node *dst, struct callchain_node *src);

bool ip_callchain__valid(struct ip_callchain *chain, const event_t *event);
#endif /* __PERF_CALLCHAIN_H */
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index be22ae6..5ba3de7 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -226,6 +226,7 @@ static bool collapse__insert_entry(struct rb_root *root, struct hist_entry *he)

if (!cmp) {
iter->period += he->period;
+ merge_chain(iter->callchain, he->callchain);
hist_entry__free(he);
return false;
}