linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Frederic Weisbecker <fweisbec@gmail.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@infradead.org>,
	linux-kernel@vger.kernel.org, Ingo Molnar <mingo@elte.hu>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>
Subject: Re: callchain sampling bug in perf?
Date: Sun, 22 Aug 2010 07:20:14 +0200	[thread overview]
Message-ID: <20100822052013.GF5258@nowhere> (raw)
In-Reply-To: <20100821144625.GA19711@infradead.org>

[-- Attachment #1: Type: text/plain, Size: 808 bytes --]

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.



[-- Attachment #2: callchain.diff --]
[-- Type: text/x-diff, Size: 2957 bytes --]

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;
 		}

  reply	other threads:[~2010-08-22  5:49 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-08-15 22:53 callchain sampling bug in perf? Christoph Hellwig
2010-08-19  0:57 ` Frederic Weisbecker
     [not found]   ` <20100819085700.GB8782@infradead.org>
2010-08-19 15:04     ` Arnaldo Carvalho de Melo
2010-08-20  9:16       ` Christoph Hellwig
2010-08-20 19:12         ` Arnaldo Carvalho de Melo
2010-08-21  2:29           ` Frederic Weisbecker
2010-08-21 14:44           ` Christoph Hellwig
2010-08-21  2:47         ` Frederic Weisbecker
2010-08-21 14:42           ` Christoph Hellwig
2010-08-21 14:46             ` Christoph Hellwig
2010-08-22  5:20               ` Frederic Weisbecker [this message]
2010-08-22  8:11                 ` Christoph Hellwig
2010-08-22  0:49             ` Frederic Weisbecker

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20100822052013.GF5258@nowhere \
    --to=fweisbec@gmail.com \
    --cc=a.p.zijlstra@chello.nl \
    --cc=acme@infradead.org \
    --cc=hch@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).