[PATCHSET 0/8] perf tools: Fix scalability problem on callchain merging (v4)

[PATCHSET 0/8] perf tools: Fix scalability problem on callchain merging (v4)

[Namhyung Kim]

Hello,

This is a new version of callchain improvement patchset.  I found and
fixed bugs in the previous version.  I verified that it produced
exactly same output before and after applying rbtree conversion patch
(#1).  However after Frederic's new comm infrastructure patches are
applied it'd be little different.

The patches are on 'perf/callchain-v4' branch in my tree

  git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git


Please test!

Thanks,
Namhyung


Frederic Weisbecker (4):
  perf tools: Use an accessor to read thread comm
  perf tools: Add time argument on comm setting
  perf tools: Add new comm infrastructure
  perf tools: Compare hists comm by addresses

Namhyung Kim (4):
  perf callchain: Convert children list to rbtree
  perf ui/progress: Add new helper functions for progress bar
  perf tools: Show progress on histogram collapsing
  perf tools: Get current comm instead of last one

 tools/perf/Makefile                                |   2 +
 tools/perf/builtin-annotate.c                      |   2 +-
 tools/perf/builtin-diff.c                          |   2 +-
 tools/perf/builtin-kmem.c                          |   2 +-
 tools/perf/builtin-lock.c                          |   2 +-
 tools/perf/builtin-report.c                        |  10 +-
 tools/perf/builtin-sched.c                         |  16 +--
 tools/perf/builtin-script.c                        |   6 +-
 tools/perf/builtin-top.c                           |   6 +-
 tools/perf/builtin-trace.c                         |  17 +--
 tools/perf/tests/code-reading.c                    |   2 +-
 tools/perf/tests/hists_link.c                      |   8 +-
 tools/perf/ui/browsers/hists.c                     |  10 +-
 tools/perf/ui/progress.c                           |  18 +++
 tools/perf/ui/progress.h                           |  10 ++
 tools/perf/util/callchain.c                        | 147 ++++++++++++++++-----
 tools/perf/util/callchain.h                        |  11 +-
 tools/perf/util/comm.c                             | 121 +++++++++++++++++
 tools/perf/util/comm.h                             |  21 +++
 tools/perf/util/event.c                            |  32 ++---
 tools/perf/util/hist.c                             |   8 +-
 tools/perf/util/hist.h                             |   3 +-
 tools/perf/util/machine.c                          |  39 +++---
 tools/perf/util/machine.h                          |  21 ++-
 .../perf/util/scripting-engines/trace-event-perl.c |   2 +-
 .../util/scripting-engines/trace-event-python.c    |   4 +-
 tools/perf/util/session.c                          |  26 ++--
 tools/perf/util/sort.c                             |  19 ++-
 tools/perf/util/sort.h                             |   1 +
 tools/perf/util/thread.c                           | 103 +++++++++++----
 tools/perf/util/thread.h                           |  10 +-
 31 files changed, 504 insertions(+), 177 deletions(-)
 create mode 100644 tools/perf/util/comm.c
 create mode 100644 tools/perf/util/comm.h

-- 
1.7.11.7

[PATCH 1/8] perf callchain: Convert children list to rbtree

[Namhyung Kim]

From: Namhyung Kim <namhyung.kim@lge.com>

Current collapse stage has a scalability problem which can be
reproduced easily with parallel kernel build.  This is because it
needs to traverse every children of callchain linearly during the
collapse/merge stage.  Convert it to rbtree reduced the overhead
significantly.

On my 400MB perf.data file which recorded with make -j32 kernel build:

  $ time perf --no-pager report --stdio > /dev/null

before:
  real	6m22.073s
  user	6m18.683s
  sys	0m0.706s

after:
  real	0m20.780s
  user	0m19.962s
  sys	0m0.689s

During the perf report the overhead on append_chain_children went down
from 96.69% to 18.16%:

  -  18.16%  perf  perf                [.] append_chain_children
     - append_chain_children
        - 77.48% append_chain_children
           + 69.79% merge_chain_branch
           - 22.96% append_chain_children
              + 67.44% merge_chain_branch
              + 30.15% append_chain_children
              + 2.41% callchain_append
           + 7.25% callchain_append
        + 12.26% callchain_append
        + 10.22% merge_chain_branch
  +  11.58%  perf  perf                [.] dso__find_symbol
  +   8.02%  perf  perf                [.] sort__comm_cmp
  +   5.48%  perf  libc-2.17.so        [.] malloc_consolidate

Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Link: http://lkml.kernel.org/n/tip-d9tcfow6stbrp4btvgs51y67@git.kernel.org
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/callchain.c | 147 +++++++++++++++++++++++++++++++++-----------
 tools/perf/util/callchain.h |  11 ++--
 2 files changed, 116 insertions(+), 42 deletions(-)

diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index 482f68081cd8..e3970e3eaacf 100644
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -21,12 +21,6 @@
 
 __thread struct callchain_cursor callchain_cursor;
 
-#define chain_for_each_child(child, parent)	\
-	list_for_each_entry(child, &parent->children, siblings)
-
-#define chain_for_each_child_safe(child, next, parent)	\
-	list_for_each_entry_safe(child, next, &parent->children, siblings)
-
 static void
 rb_insert_callchain(struct rb_root *root, struct callchain_node *chain,
 		    enum chain_mode mode)
@@ -71,10 +65,16 @@ static void
 __sort_chain_flat(struct rb_root *rb_root, struct callchain_node *node,
 		  u64 min_hit)
 {
+	struct rb_node *n;
 	struct callchain_node *child;
 
-	chain_for_each_child(child, node)
+	n = rb_first(&node->rb_root_in);
+	while (n) {
+		child = rb_entry(n, struct callchain_node, rb_node_in);
+		n = rb_next(n);
+
 		__sort_chain_flat(rb_root, child, min_hit);
+	}
 
 	if (node->hit && node->hit >= min_hit)
 		rb_insert_callchain(rb_root, node, CHAIN_FLAT);
@@ -94,11 +94,16 @@ sort_chain_flat(struct rb_root *rb_root, struct callchain_root *root,
 static void __sort_chain_graph_abs(struct callchain_node *node,
 				   u64 min_hit)
 {
+	struct rb_node *n;
 	struct callchain_node *child;
 
 	node->rb_root = RB_ROOT;
+	n = rb_first(&node->rb_root_in);
+
+	while (n) {
+		child = rb_entry(n, struct callchain_node, rb_node_in);
+		n = rb_next(n);
 
-	chain_for_each_child(child, node) {
 		__sort_chain_graph_abs(child, min_hit);
 		if (callchain_cumul_hits(child) >= min_hit)
 			rb_insert_callchain(&node->rb_root, child,
@@ -117,13 +122,18 @@ sort_chain_graph_abs(struct rb_root *rb_root, struct callchain_root *chain_root,
 static void __sort_chain_graph_rel(struct callchain_node *node,
 				   double min_percent)
 {
+	struct rb_node *n;
 	struct callchain_node *child;
 	u64 min_hit;
 
 	node->rb_root = RB_ROOT;
 	min_hit = ceil(node->children_hit * min_percent);
 
-	chain_for_each_child(child, node) {
+	n = rb_first(&node->rb_root_in);
+	while (n) {
+		child = rb_entry(n, struct callchain_node, rb_node_in);
+		n = rb_next(n);
+
 		__sort_chain_graph_rel(child, min_percent);
 		if (callchain_cumul_hits(child) >= min_hit)
 			rb_insert_callchain(&node->rb_root, child,
@@ -173,19 +183,26 @@ create_child(struct callchain_node *parent, bool inherit_children)
 		return NULL;
 	}
 	new->parent = parent;
-	INIT_LIST_HEAD(&new->children);
 	INIT_LIST_HEAD(&new->val);
 
 	if (inherit_children) {
-		struct callchain_node *next;
+		struct rb_node *n;
+		struct callchain_node *child;
+
+		new->rb_root_in = parent->rb_root_in;
+		parent->rb_root_in = RB_ROOT;
 
-		list_splice(&parent->children, &new->children);
-		INIT_LIST_HEAD(&parent->children);
+		n = rb_first(&new->rb_root_in);
+		while (n) {
+			child = rb_entry(n, struct callchain_node, rb_node_in);
+			child->parent = new;
+			n = rb_next(n);
+		}
 
-		chain_for_each_child(next, new)
-			next->parent = new;
+		/* make it the first child */
+		rb_link_node(&new->rb_node_in, NULL, &parent->rb_root_in.rb_node);
+		rb_insert_color(&new->rb_node_in, &parent->rb_root_in);
 	}
-	list_add_tail(&new->siblings, &parent->children);
 
 	return new;
 }
@@ -223,7 +240,7 @@ fill_node(struct callchain_node *node, struct callchain_cursor *cursor)
 	}
 }
 
-static void
+static struct callchain_node *
 add_child(struct callchain_node *parent,
 	  struct callchain_cursor *cursor,
 	  u64 period)
@@ -235,6 +252,19 @@ add_child(struct callchain_node *parent,
 
 	new->children_hit = 0;
 	new->hit = period;
+	return new;
+}
+
+static s64 match_chain(struct callchain_cursor_node *node,
+		      struct callchain_list *cnode)
+{
+	struct symbol *sym = node->sym;
+
+	if (cnode->ms.sym && sym &&
+	    callchain_param.key == CCKEY_FUNCTION)
+		return cnode->ms.sym->start - sym->start;
+	else
+		return cnode->ip - node->ip;
 }
 
 /*
@@ -272,9 +302,33 @@ split_add_child(struct callchain_node *parent,
 
 	/* create a new child for the new branch if any */
 	if (idx_total < cursor->nr) {
+		struct callchain_node *first;
+		struct callchain_list *cnode;
+		struct callchain_cursor_node *node;
+		struct rb_node *p, **pp;
+
 		parent->hit = 0;
-		add_child(parent, cursor, period);
 		parent->children_hit += period;
+
+		node = callchain_cursor_current(cursor);
+		new = add_child(parent, cursor, period);
+
+		/*
+		 * This is second child since we moved parent's children
+		 * to new (first) child above.
+		 */
+		p = parent->rb_root_in.rb_node;
+		first = rb_entry(p, struct callchain_node, rb_node_in);
+		cnode = list_first_entry(&first->val, struct callchain_list,
+					 list);
+
+		if (match_chain(node, cnode) < 0)
+			pp = &p->rb_left;
+		else
+			pp = &p->rb_right;
+
+		rb_link_node(&new->rb_node_in, p, pp);
+		rb_insert_color(&new->rb_node_in, &parent->rb_root_in);
 	} else {
 		parent->hit = period;
 	}
@@ -291,16 +345,40 @@ append_chain_children(struct callchain_node *root,
 		      u64 period)
 {
 	struct callchain_node *rnode;
+	struct callchain_cursor_node *node;
+	struct rb_node **p = &root->rb_root_in.rb_node;
+	struct rb_node *parent = NULL;
+
+	node = callchain_cursor_current(cursor);
+	if (!node)
+		return;
 
 	/* lookup in childrens */
-	chain_for_each_child(rnode, root) {
-		unsigned int ret = append_chain(rnode, cursor, period);
+	while (*p) {
+		s64 ret;
+		struct callchain_list *cnode;
 
-		if (!ret)
+		parent = *p;
+		rnode = rb_entry(parent, struct callchain_node, rb_node_in);
+		cnode = list_first_entry(&rnode->val, struct callchain_list,
+					 list);
+
+		/* just check first entry */
+		ret = match_chain(node, cnode);
+		if (ret == 0) {
+			append_chain(rnode, cursor, period);
 			goto inc_children_hit;
+		}
+
+		if (ret < 0)
+			p = &parent->rb_left;
+		else
+			p = &parent->rb_right;
 	}
 	/* nothing in children, add to the current node */
-	add_child(root, cursor, period);
+	rnode = add_child(root, cursor, period);
+	rb_link_node(&rnode->rb_node_in, parent, p);
+	rb_insert_color(&rnode->rb_node_in, &root->rb_root_in);
 
 inc_children_hit:
 	root->children_hit += period;
@@ -325,28 +403,20 @@ append_chain(struct callchain_node *root,
 	 */
 	list_for_each_entry(cnode, &root->val, list) {
 		struct callchain_cursor_node *node;
-		struct symbol *sym;
 
 		node = callchain_cursor_current(cursor);
 		if (!node)
 			break;
 
-		sym = node->sym;
-
-		if (cnode->ms.sym && sym &&
-		    callchain_param.key == CCKEY_FUNCTION) {
-			if (cnode->ms.sym->start != sym->start)
-				break;
-		} else if (cnode->ip != node->ip)
+		if (match_chain(node, cnode) != 0)
 			break;
 
-		if (!found)
-			found = true;
+		found = true;
 
 		callchain_cursor_advance(cursor);
 	}
 
-	/* matches not, relay on the parent */
+	/* matches not, relay no the parent */
 	if (!found) {
 		cursor->curr = curr_snap;
 		cursor->pos = start;
@@ -395,8 +465,9 @@ merge_chain_branch(struct callchain_cursor *cursor,
 		   struct callchain_node *dst, struct callchain_node *src)
 {
 	struct callchain_cursor_node **old_last = cursor->last;
-	struct callchain_node *child, *next_child;
+	struct callchain_node *child;
 	struct callchain_list *list, *next_list;
+	struct rb_node *n;
 	int old_pos = cursor->nr;
 	int err = 0;
 
@@ -412,12 +483,16 @@ merge_chain_branch(struct callchain_cursor *cursor,
 		append_chain_children(dst, cursor, src->hit);
 	}
 
-	chain_for_each_child_safe(child, next_child, src) {
+	n = rb_first(&src->rb_root_in);
+	while (n) {
+		child = container_of(n, struct callchain_node, rb_node_in);
+		n = rb_next(n);
+		rb_erase(&child->rb_node_in, &src->rb_root_in);
+
 		err = merge_chain_branch(cursor, dst, child);
 		if (err)
 			break;
 
-		list_del(&child->siblings);
 		free(child);
 	}
 
diff --git a/tools/perf/util/callchain.h b/tools/perf/util/callchain.h
index 2b585bc308cf..7bb36022377f 100644
--- a/tools/perf/util/callchain.h
+++ b/tools/perf/util/callchain.h
@@ -21,11 +21,11 @@ enum chain_order {
 
 struct callchain_node {
 	struct callchain_node	*parent;
-	struct list_head	siblings;
-	struct list_head	children;
 	struct list_head	val;
-	struct rb_node		rb_node; /* to sort nodes in an rbtree */
-	struct rb_root		rb_root; /* sorted tree of children */
+	struct rb_node		rb_node_in; /* to insert nodes in an rbtree */
+	struct rb_node		rb_node;    /* to sort nodes in an output tree */
+	struct rb_root		rb_root_in; /* input tree of children */
+	struct rb_root		rb_root;    /* sorted output tree of children */
 	unsigned int		val_nr;
 	u64			hit;
 	u64			children_hit;
@@ -86,13 +86,12 @@ extern __thread struct callchain_cursor callchain_cursor;
 
 static inline void callchain_init(struct callchain_root *root)
 {
-	INIT_LIST_HEAD(&root->node.siblings);
-	INIT_LIST_HEAD(&root->node.children);
 	INIT_LIST_HEAD(&root->node.val);
 
 	root->node.parent = NULL;
 	root->node.hit = 0;
 	root->node.children_hit = 0;
+	root->node.rb_root_in = RB_ROOT;
 	root->max_depth = 0;
 }
 
-- 
1.7.11.7

[PATCH 2/8] perf ui/progress: Add new helper functions for progress bar

[Namhyung Kim]

From: Namhyung Kim <namhyung.kim@lge.com>

Introduce ui_progress_setup() and ui_progress__advance() to separate
out from the session logic.  It'll be used by other places in the
upcoming patch.

Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/progress.c  | 18 ++++++++++++++++++
 tools/perf/ui/progress.h  | 10 ++++++++++
 tools/perf/util/session.c | 24 ++++++++++++------------
 3 files changed, 40 insertions(+), 12 deletions(-)

diff --git a/tools/perf/ui/progress.c b/tools/perf/ui/progress.c
index 3ec695607a4d..b5c22c6b1d93 100644
--- a/tools/perf/ui/progress.c
+++ b/tools/perf/ui/progress.c
@@ -19,6 +19,24 @@ void ui_progress__update(u64 curr, u64 total, const char *title)
 	return progress_fns->update(curr, total, title);
 }
 
+void ui_progress__setup(struct perf_progress *p, u64 total)
+{
+	p->curr = 0;
+	p->next = p->unit = total / 16;
+	p->total = total;
+
+}
+
+void ui_progress__advance(struct perf_progress *p, u64 adv, const char *title)
+{
+	p->curr += adv;
+
+	if (p->curr >= p->next) {
+		p->next += p->unit;
+		ui_progress__update(p->curr, p->total, title);
+	}
+}
+
 void ui_progress__finish(void)
 {
 	if (progress_fns->finish)
diff --git a/tools/perf/ui/progress.h b/tools/perf/ui/progress.h
index 257cc224f9cf..3bd23e825953 100644
--- a/tools/perf/ui/progress.h
+++ b/tools/perf/ui/progress.h
@@ -8,10 +8,20 @@ struct ui_progress {
 	void (*finish)(void);
 };
 
+struct perf_progress {
+	u64 curr;
+	u64 next;
+	u64 unit;
+	u64 total;
+};
+
 extern struct ui_progress *progress_fns;
 
 void ui_progress__init(void);
 
+void ui_progress__setup(struct perf_progress *p, u64 total);
+void ui_progress__advance(struct perf_progress *p, u64 adv, const char *title);
+
 void ui_progress__update(u64 curr, u64 total, const char *title);
 void ui_progress__finish(void);
 
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 6c1d4447c5b4..9f62be8bc167 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -538,13 +538,16 @@ static int flush_sample_queue(struct perf_session *s,
 	struct perf_sample sample;
 	u64 limit = os->next_flush;
 	u64 last_ts = os->last_sample ? os->last_sample->timestamp : 0ULL;
-	unsigned idx = 0, progress_next = os->nr_samples / 16;
 	bool show_progress = limit == ULLONG_MAX;
+	struct perf_progress prog;
 	int ret;
 
 	if (!tool->ordered_samples || !limit)
 		return 0;
 
+	if (show_progress)
+		ui_progress__setup(&prog, os->nr_samples);
+
 	list_for_each_entry_safe(iter, tmp, head, list) {
 		if (iter->timestamp > limit)
 			break;
@@ -562,10 +565,10 @@ static int flush_sample_queue(struct perf_session *s,
 		os->last_flush = iter->timestamp;
 		list_del(&iter->list);
 		list_add(&iter->list, &os->sample_cache);
-		if (show_progress && (++idx >= progress_next)) {
-			progress_next += os->nr_samples / 16;
-			ui_progress__update(idx, os->nr_samples,
-					    "Processing time ordered events...");
+
+		if (show_progress) {
+			ui_progress__advance(&prog, 1,
+					     "Processing time ordered events...");
 		}
 	}
 
@@ -1310,12 +1313,13 @@ int __perf_session__process_events(struct perf_session *session,
 				   u64 data_offset, u64 data_size,
 				   u64 file_size, struct perf_tool *tool)
 {
-	u64 head, page_offset, file_offset, file_pos, progress_next;
+	u64 head, page_offset, file_offset, file_pos;
 	int err, mmap_prot, mmap_flags, map_idx = 0;
 	size_t	mmap_size;
 	char *buf, *mmaps[NUM_MMAPS];
 	union perf_event *event;
 	uint32_t size;
+	struct perf_progress prog;
 
 	perf_tool__fill_defaults(tool);
 
@@ -1326,7 +1330,7 @@ int __perf_session__process_events(struct perf_session *session,
 	if (data_offset + data_size < file_size)
 		file_size = data_offset + data_size;
 
-	progress_next = file_size / 16;
+	ui_progress__setup(&prog, file_size);
 
 	mmap_size = MMAP_SIZE;
 	if (mmap_size > file_size)
@@ -1381,11 +1385,7 @@ more:
 	head += size;
 	file_pos += size;
 
-	if (file_pos >= progress_next) {
-		progress_next += file_size / 16;
-		ui_progress__update(file_pos, file_size,
-				    "Processing events...");
-	}
+	ui_progress__advance(&prog, size, "Processing events...");
 
 	if (file_pos < file_size)
 		goto more;
-- 
1.7.11.7

[PATCH 3/8] perf tools: Show progress on histogram collapsing

[Namhyung Kim]

From: Namhyung Kim <namhyung.kim@lge.com>

It can take quite amount of time so add progress bar UI to inform user.

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-annotate.c |  2 +-
 tools/perf/builtin-diff.c     |  2 +-
 tools/perf/builtin-report.c   | 10 +++++++++-
 tools/perf/builtin-top.c      |  4 ++--
 tools/perf/tests/hists_link.c |  2 +-
 tools/perf/util/hist.c        |  5 ++++-
 tools/perf/util/hist.h        |  3 ++-
 7 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 577c3a1cffea..958d5e280b1f 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -242,7 +242,7 @@ static int __cmd_annotate(struct perf_annotate *ann)
 
 		if (nr_samples > 0) {
 			total_nr_samples += nr_samples;
-			hists__collapse_resort(hists);
+			hists__collapse_resort(hists, NULL);
 			hists__output_resort(hists);
 
 			if (symbol_conf.event_group &&
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index f28799e94f2a..f4ab7d210105 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -367,7 +367,7 @@ static void perf_evlist__collapse_resort(struct perf_evlist *evlist)
 	list_for_each_entry(evsel, &evlist->entries, node) {
 		struct hists *hists = &evsel->hists;
 
-		hists__collapse_resort(hists);
+		hists__collapse_resort(hists, NULL);
 	}
 }
 
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 295025e6b26e..3390d9a4e3bf 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -489,6 +489,7 @@ static int __cmd_report(struct perf_report *rep)
 	struct map *kernel_map;
 	struct kmap *kernel_kmap;
 	const char *help = "For a higher level overview, try: perf report --sort comm,dso";
+	struct perf_progress prog;
 
 	signal(SIGINT, sig_handler);
 
@@ -550,13 +551,19 @@ static int __cmd_report(struct perf_report *rep)
 	}
 
 	nr_samples = 0;
+	list_for_each_entry(pos, &session->evlist->entries, node)
+		nr_samples += pos->hists.nr_entries;
+
+	ui_progress__setup(&prog, nr_samples);
+
+	nr_samples = 0;
 	list_for_each_entry(pos, &session->evlist->entries, node) {
 		struct hists *hists = &pos->hists;
 
 		if (pos->idx == 0)
 			hists->symbol_filter_str = rep->symbol_filter_str;
 
-		hists__collapse_resort(hists);
+		hists__collapse_resort(hists, &prog);
 		nr_samples += hists->stats.nr_events[PERF_RECORD_SAMPLE];
 
 		/* Non-group events are considered as leader */
@@ -568,6 +575,7 @@ static int __cmd_report(struct perf_report *rep)
 			hists__link(leader_hists, hists);
 		}
 	}
+	ui_progress__finish();
 
 	if (nr_samples == 0) {
 		ui__error("The %s file has no samples!\n", session->filename);
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index cf4bba07175a..0cb077f823de 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -287,7 +287,7 @@ static void perf_top__print_sym_table(struct perf_top *top)
 		return;
 	}
 
-	hists__collapse_resort(&top->sym_evsel->hists);
+	hists__collapse_resort(&top->sym_evsel->hists, NULL);
 	hists__output_resort(&top->sym_evsel->hists);
 	hists__decay_entries(&top->sym_evsel->hists,
 			     top->hide_user_symbols,
@@ -553,7 +553,7 @@ static void perf_top__sort_new_samples(void *arg)
 	if (t->evlist->selected != NULL)
 		t->sym_evsel = t->evlist->selected;
 
-	hists__collapse_resort(&t->sym_evsel->hists);
+	hists__collapse_resort(&t->sym_evsel->hists, NULL);
 	hists__output_resort(&t->sym_evsel->hists);
 	hists__decay_entries(&t->sym_evsel->hists,
 			     t->hide_user_symbols,
diff --git a/tools/perf/tests/hists_link.c b/tools/perf/tests/hists_link.c
index 4228ffc0d968..3b5faaa5c8cf 100644
--- a/tools/perf/tests/hists_link.c
+++ b/tools/perf/tests/hists_link.c
@@ -465,7 +465,7 @@ int test__hists_link(void)
 		goto out;
 
 	list_for_each_entry(evsel, &evlist->entries, node) {
-		hists__collapse_resort(&evsel->hists);
+		hists__collapse_resort(&evsel->hists, NULL);
 
 		if (verbose > 2)
 			print_hists(&evsel->hists);
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index fa695ce1662a..a2d58e7abdcf 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -395,6 +395,7 @@ static struct hist_entry *add_hist_entry(struct hists *hists,
 	if (!he)
 		return NULL;
 
+	hists->nr_entries++;
 	rb_link_node(&he->rb_node_in, parent, p);
 	rb_insert_color(&he->rb_node_in, hists->entries_in);
 out:
@@ -599,7 +600,7 @@ static void hists__apply_filters(struct hists *hists, struct hist_entry *he)
 	hists__filter_entry_by_symbol(hists, he);
 }
 
-void hists__collapse_resort(struct hists *hists)
+void hists__collapse_resort(struct hists *hists, struct perf_progress *prog)
 {
 	struct rb_root *root;
 	struct rb_node *next;
@@ -624,6 +625,8 @@ void hists__collapse_resort(struct hists *hists)
 			 */
 			hists__apply_filters(hists, n);
 		}
+		if (prog)
+			ui_progress__advance(prog, 1, "Merging related events...");
 	}
 }
 
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 7f29792efc58..f3d6cf6bd7d4 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -5,6 +5,7 @@
 #include <pthread.h>
 #include "callchain.h"
 #include "header.h"
+#include "ui/progress.h"
 
 extern struct callchain_param callchain_param;
 
@@ -104,7 +105,7 @@ struct hist_entry *__hists__add_mem_entry(struct hists *self,
 					  u64 weight);
 
 void hists__output_resort(struct hists *self);
-void hists__collapse_resort(struct hists *self);
+void hists__collapse_resort(struct hists *self, struct perf_progress *prog);
 
 void hists__decay_entries(struct hists *hists, bool zap_user, bool zap_kernel);
 void hists__output_recalc_col_len(struct hists *hists, int max_rows);
-- 
1.7.11.7

[PATCH 4/8] perf tools: Use an accessor to read thread comm

[Namhyung Kim]

From: Frederic Weisbecker <fweisbec@gmail.com>

As the thread comm is going to be implemented by way of a more
complicated data structure than just a pointer to a string from the
thread struct, convert the readers of comm to use an accessor instead
of accessing it directly. The accessor will be later overriden to
support an enhanced comm implementation.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/n/tip-wr683zwy94hmj4ibogmnv9ce@git.kernel.org
[ Fixed up some minor const pointer issues ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-kmem.c                              |  2 +-
 tools/perf/builtin-lock.c                              |  2 +-
 tools/perf/builtin-sched.c                             | 16 ++++++++--------
 tools/perf/builtin-script.c                            |  6 +++---
 tools/perf/builtin-trace.c                             |  5 +++--
 tools/perf/tests/hists_link.c                          |  2 +-
 tools/perf/ui/browsers/hists.c                         | 10 +++++-----
 tools/perf/util/event.c                                |  4 ++--
 tools/perf/util/scripting-engines/trace-event-perl.c   |  2 +-
 tools/perf/util/scripting-engines/trace-event-python.c |  4 ++--
 tools/perf/util/sort.c                                 | 10 +++++-----
 tools/perf/util/thread.c                               |  7 ++++++-
 tools/perf/util/thread.h                               |  1 +
 13 files changed, 39 insertions(+), 32 deletions(-)

diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c
index 9b5f077fee5b..1a85296f6af1 100644
--- a/tools/perf/builtin-kmem.c
+++ b/tools/perf/builtin-kmem.c
@@ -314,7 +314,7 @@ static int process_sample_event(struct perf_tool *tool __maybe_unused,
 		return -1;
 	}
 
-	dump_printf(" ... thread: %s:%d\n", thread->comm, thread->tid);
+	dump_printf(" ... thread: %s:%d\n", thread__comm_curr(thread), thread->tid);
 
 	if (evsel->handler.func != NULL) {
 		tracepoint_handler f = evsel->handler.func;
diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
index 6a9076f165f4..69670c6bc165 100644
--- a/tools/perf/builtin-lock.c
+++ b/tools/perf/builtin-lock.c
@@ -766,7 +766,7 @@ static void dump_threads(void)
 	while (node) {
 		st = container_of(node, struct thread_stat, rb);
 		t = perf_session__findnew(session, st->tid);
-		pr_info("%10d: %s\n", st->tid, t->comm);
+		pr_info("%10d: %s\n", st->tid, thread__comm_curr(t));
 		node = rb_next(node);
 	};
 }
diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index d8c51b2f263f..2699719dfde2 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -737,12 +737,12 @@ static int replay_fork_event(struct perf_sched *sched,
 
 	if (verbose) {
 		printf("fork event\n");
-		printf("... parent: %s/%d\n", parent->comm, parent->tid);
-		printf("...  child: %s/%d\n", child->comm, child->tid);
+		printf("... parent: %s/%d\n", thread__comm_curr(parent), parent->tid);
+		printf("...  child: %s/%d\n", thread__comm_curr(child), child->tid);
 	}
 
-	register_pid(sched, parent->tid, parent->comm);
-	register_pid(sched, child->tid, child->comm);
+	register_pid(sched, parent->tid, thread__comm_curr(parent));
+	register_pid(sched, child->tid, thread__comm_curr(child));
 	return 0;
 }
 
@@ -1077,7 +1077,7 @@ static int latency_migrate_task_event(struct perf_sched *sched,
 	if (!atoms) {
 		if (thread_atoms_insert(sched, migrant))
 			return -1;
-		register_pid(sched, migrant->tid, migrant->comm);
+		register_pid(sched, migrant->tid, thread__comm_curr(migrant));
 		atoms = thread_atoms_search(&sched->atom_root, migrant, &sched->cmp_pid);
 		if (!atoms) {
 			pr_err("migration-event: Internal tree error");
@@ -1111,13 +1111,13 @@ static void output_lat_thread(struct perf_sched *sched, struct work_atoms *work_
 	/*
 	 * Ignore idle threads:
 	 */
-	if (!strcmp(work_list->thread->comm, "swapper"))
+	if (!strcmp(thread__comm_curr(work_list->thread), "swapper"))
 		return;
 
 	sched->all_runtime += work_list->total_runtime;
 	sched->all_count   += work_list->nb_atoms;
 
-	ret = printf("  %s:%d ", work_list->thread->comm, work_list->thread->tid);
+	ret = printf("  %s:%d ", thread__comm_curr(work_list->thread), work_list->thread->tid);
 
 	for (i = 0; i < 24 - ret; i++)
 		printf(" ");
@@ -1334,7 +1334,7 @@ static int map_switch_event(struct perf_sched *sched, struct perf_evsel *evsel,
 	printf("  %12.6f secs ", (double)timestamp/1e9);
 	if (new_shortname) {
 		printf("%s => %s:%d\n",
-			sched_in->shortname, sched_in->comm, sched_in->tid);
+		       sched_in->shortname, thread__comm_curr(sched_in), sched_in->tid);
 	} else {
 		printf("\n");
 	}
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 7f31a3ded1b6..50258aa8cea1 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -290,11 +290,11 @@ static void print_sample_start(struct perf_sample *sample,
 
 	if (PRINT_FIELD(COMM)) {
 		if (latency_format)
-			printf("%8.8s ", thread->comm);
+			printf("%8.8s ", thread__comm_curr(thread));
 		else if (PRINT_FIELD(IP) && symbol_conf.use_callchain)
-			printf("%s ", thread->comm);
+			printf("%s ", thread__comm_curr(thread));
 		else
-			printf("%16s ", thread->comm);
+			printf("%16s ", thread__comm_curr(thread));
 	}
 
 	if (PRINT_FIELD(PID) && PRINT_FIELD(TID))
diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 1bb8f154852a..a96fd47b369b 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -1025,7 +1025,8 @@ static size_t trace__fprintf_entry_head(struct trace *trace, struct thread *thre
 
 	if (trace->multiple_threads) {
 		if (trace->show_comm)
-			printed += fprintf(fp, "%.14s/", thread->comm);
+			printed += fprintf(fp, "%.14s/",
+					   thread__comm_curr(thread));
 		printed += fprintf(fp, "%d ", thread->tid);
 	}
 
@@ -1730,7 +1731,7 @@ static size_t trace__fprintf_thread_summary(struct trace *trace, FILE *fp)
 		else if (ratio > 5.0)
 			color = PERF_COLOR_YELLOW;
 
-		printed += color_fprintf(fp, color, "%20s", thread->comm);
+		printed += color_fprintf(fp, color, "%20s", thread__comm_curr(thread));
 		printed += fprintf(fp, " - %-5d :%11lu   [", thread->tid, ttrace->nr_events);
 		printed += color_fprintf(fp, color, "%5.1f%%", ratio);
 		printed += fprintf(fp, " ] %10.3f ms\n", ttrace->runtime_ms);
diff --git a/tools/perf/tests/hists_link.c b/tools/perf/tests/hists_link.c
index 3b5faaa5c8cf..735cff2c52ce 100644
--- a/tools/perf/tests/hists_link.c
+++ b/tools/perf/tests/hists_link.c
@@ -419,7 +419,7 @@ static void print_hists(struct hists *hists)
 		he = rb_entry(node, struct hist_entry, rb_node_in);
 
 		pr_info("%2d: entry: %-8s [%-8s] %20s: period = %"PRIu64"\n",
-			i, he->thread->comm, he->ms.map->dso->short_name,
+			i, thread__comm_curr(he->thread), he->ms.map->dso->short_name,
 			he->ms.sym->name, he->stat.period);
 
 		i++;
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 7ef36c360471..abb68e24d858 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1255,7 +1255,7 @@ static int hists__browser_title(struct hists *hists, char *bf, size_t size,
 	if (thread)
 		printed += scnprintf(bf + printed, size - printed,
 				    ", Thread: %s(%d)",
-				    (thread->comm_set ? thread->comm : ""),
+				     (thread->comm_set ? thread__comm_curr(thread) : ""),
 				    thread->tid);
 	if (dso)
 		printed += scnprintf(bf + printed, size - printed,
@@ -1578,7 +1578,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 		if (thread != NULL &&
 		    asprintf(&options[nr_options], "Zoom %s %s(%d) thread",
 			     (browser->hists->thread_filter ? "out of" : "into"),
-			     (thread->comm_set ? thread->comm : ""),
+			     (thread->comm_set ? thread__comm_curr(thread) : ""),
 			     thread->tid) > 0)
 			zoom_thread = nr_options++;
 
@@ -1598,7 +1598,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 			struct symbol *sym;
 
 			if (asprintf(&options[nr_options], "Run scripts for samples of thread [%s]",
-				browser->he_selection->thread->comm) > 0)
+				     thread__comm_curr(browser->he_selection->thread)) > 0)
 				scripts_comm = nr_options++;
 
 			sym = browser->he_selection->ms.sym;
@@ -1701,7 +1701,7 @@ zoom_out_thread:
 				sort_thread.elide = false;
 			} else {
 				ui_helpline__fpush("To zoom out press <- or -> + \"Zoom out of %s(%d) thread\"",
-						   thread->comm_set ? thread->comm : "",
+						   thread->comm_set ? thread__comm_curr(thread) : "",
 						   thread->tid);
 				browser->hists->thread_filter = thread;
 				sort_thread.elide = true;
@@ -1717,7 +1717,7 @@ do_scripts:
 			memset(script_opt, 0, 64);
 
 			if (choice == scripts_comm)
-				sprintf(script_opt, " -c %s ", browser->he_selection->thread->comm);
+				sprintf(script_opt, " -c %s ", thread__comm_curr(browser->he_selection->thread));
 
 			if (choice == scripts_symbol)
 				sprintf(script_opt, " -S %s ", browser->he_selection->ms.sym->name);
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 9b393e7dca6f..d639bfe29309 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -725,10 +725,10 @@ int perf_event__preprocess_sample(const union perf_event *event,
 		return -1;
 
 	if (symbol_conf.comm_list &&
-	    !strlist__has_entry(symbol_conf.comm_list, thread->comm))
+	    !strlist__has_entry(symbol_conf.comm_list, thread__comm_curr(thread)))
 		goto out_filtered;
 
-	dump_printf(" ... thread: %s:%d\n", thread->comm, thread->tid);
+	dump_printf(" ... thread: %s:%d\n", thread__comm_curr(thread), thread->tid);
 	/*
 	 * Have we already created the kernel maps for this machine?
 	 *
diff --git a/tools/perf/util/scripting-engines/trace-event-perl.c b/tools/perf/util/scripting-engines/trace-event-perl.c
index a85e4ae5f3ac..c4065ed3ae5a 100644
--- a/tools/perf/util/scripting-engines/trace-event-perl.c
+++ b/tools/perf/util/scripting-engines/trace-event-perl.c
@@ -273,7 +273,7 @@ static void perl_process_tracepoint(union perf_event *perf_event __maybe_unused,
 	int cpu = sample->cpu;
 	void *data = sample->raw_data;
 	unsigned long long nsecs = sample->time;
-	char *comm = thread->comm;
+	const char *comm = thread__comm_curr(thread);
 
 	dSP;
 
diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
index cc75a3cef388..b532675be3c1 100644
--- a/tools/perf/util/scripting-engines/trace-event-python.c
+++ b/tools/perf/util/scripting-engines/trace-event-python.c
@@ -239,7 +239,7 @@ static void python_process_tracepoint(union perf_event *perf_event
 	int cpu = sample->cpu;
 	void *data = sample->raw_data;
 	unsigned long long nsecs = sample->time;
-	char *comm = thread->comm;
+	const char *comm = thread__comm_curr(thread);
 
 	t = PyTuple_New(MAX_FIELDS);
 	if (!t)
@@ -378,7 +378,7 @@ static void python_process_general_event(union perf_event *perf_event
 	PyDict_SetItemString(dict, "raw_buf", PyString_FromStringAndSize(
 			(const char *)sample->raw_data, sample->raw_size));
 	PyDict_SetItemString(dict, "comm",
-			PyString_FromString(thread->comm));
+			     PyString_FromString(thread__comm_curr(thread)));
 	if (al->map) {
 		PyDict_SetItemString(dict, "dso",
 			PyString_FromString(al->map->dso->name));
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 082480579861..65fe958ce30b 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -42,7 +42,7 @@ static int repsep_snprintf(char *bf, size_t size, const char *fmt, ...)
 	return n;
 }
 
-static int64_t cmp_null(void *l, void *r)
+static int64_t cmp_null(const void *l, const void *r)
 {
 	if (!l && !r)
 		return 0;
@@ -64,7 +64,7 @@ static int hist_entry__thread_snprintf(struct hist_entry *self, char *bf,
 				       size_t size, unsigned int width)
 {
 	return repsep_snprintf(bf, size, "%*s:%5d", width - 6,
-			      self->thread->comm ?: "", self->thread->tid);
+			       thread__comm_curr(self->thread) ?: "", self->thread->tid);
 }
 
 struct sort_entry sort_thread = {
@@ -85,8 +85,8 @@ sort__comm_cmp(struct hist_entry *left, struct hist_entry *right)
 static int64_t
 sort__comm_collapse(struct hist_entry *left, struct hist_entry *right)
 {
-	char *comm_l = left->thread->comm;
-	char *comm_r = right->thread->comm;
+	const char *comm_l = thread__comm_curr(left->thread);
+	const char *comm_r = thread__comm_curr(right->thread);
 
 	if (!comm_l || !comm_r)
 		return cmp_null(comm_l, comm_r);
@@ -97,7 +97,7 @@ sort__comm_collapse(struct hist_entry *left, struct hist_entry *right)
 static int hist_entry__comm_snprintf(struct hist_entry *self, char *bf,
 				     size_t size, unsigned int width)
 {
-	return repsep_snprintf(bf, size, "%*s", width, self->thread->comm);
+	return repsep_snprintf(bf, size, "%*s", width, thread__comm_curr(self->thread));
 }
 
 struct sort_entry sort_comm = {
diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
index e3d4a550a703..bb50b9fda9c2 100644
--- a/tools/perf/util/thread.c
+++ b/tools/perf/util/thread.c
@@ -45,6 +45,11 @@ int thread__set_comm(struct thread *self, const char *comm)
 	return err;
 }
 
+const char *thread__comm_curr(const struct thread *thread)
+{
+	return thread->comm;
+}
+
 int thread__comm_len(struct thread *self)
 {
 	if (!self->comm_len) {
@@ -58,7 +63,7 @@ int thread__comm_len(struct thread *self)
 
 size_t thread__fprintf(struct thread *thread, FILE *fp)
 {
-	return fprintf(fp, "Thread %d %s\n", thread->tid, thread->comm) +
+	return fprintf(fp, "Thread %d %s\n", thread->tid, thread__comm_curr(thread)) +
 	       map_groups__fprintf(&thread->mg, verbose, fp);
 }
 
diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
index 4ebbb40d46d4..84db214d90bd 100644
--- a/tools/perf/util/thread.h
+++ b/tools/perf/util/thread.h
@@ -35,6 +35,7 @@ static inline void thread__exited(struct thread *thread)
 
 int thread__set_comm(struct thread *self, const char *comm);
 int thread__comm_len(struct thread *self);
+const char *thread__comm_curr(const struct thread *thread);
 void thread__insert_map(struct thread *self, struct map *map);
 int thread__fork(struct thread *self, struct thread *parent);
 size_t thread__fprintf(struct thread *thread, FILE *fp);
-- 
1.7.11.7

[PATCH 5/8] perf tools: Add time argument on comm setting

[Namhyung Kim]

From: Frederic Weisbecker <fweisbec@gmail.com>

This way we can later delimit a lifecycle for the comm and map a hist to
a precise comm:timeslice couple.

Comm and fork events that don't have PERF_SAMPLE_TIME samples can only
send 0 value as a timestamp and thus should overwrite any previous comm
on a given thread because there is no sensible way to keep track of all
the comms lifecycles in a thread without time informations.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/n/tip-6tyow99vgmmtt9qwr2u2lqd7@git.kernel.org
[ Made it cope with PERF_RECORD_MMAP2 ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-top.c        |  2 +-
 tools/perf/builtin-trace.c      | 12 ++++++------
 tools/perf/tests/code-reading.c |  2 +-
 tools/perf/tests/hists_link.c   |  4 ++--
 tools/perf/util/event.c         | 28 ++++++++++++++--------------
 tools/perf/util/machine.c       | 39 ++++++++++++++++++++++-----------------
 tools/perf/util/machine.h       | 21 ++++++++++++++-------
 tools/perf/util/session.c       |  2 +-
 tools/perf/util/thread.c        |  6 ++++--
 tools/perf/util/thread.h        |  4 ++--
 10 files changed, 67 insertions(+), 53 deletions(-)

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 0cb077f823de..bb5364c87c3f 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -856,7 +856,7 @@ static void perf_top__mmap_read_idx(struct perf_top *top, int idx)
 						   &sample, machine);
 		} else if (event->header.type < PERF_RECORD_MAX) {
 			hists__inc_nr_events(&evsel->hists, event->header.type);
-			machine__process_event(machine, event);
+			machine__process_event(machine, event, &sample);
 		} else
 			++session->stats.nr_unknown_events;
 	}
diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index a96fd47b369b..f1d0dc48ba34 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -1034,7 +1034,7 @@ static size_t trace__fprintf_entry_head(struct trace *trace, struct thread *thre
 }
 
 static int trace__process_event(struct trace *trace, struct machine *machine,
-				union perf_event *event)
+				union perf_event *event, struct perf_sample *sample)
 {
 	int ret = 0;
 
@@ -1042,9 +1042,9 @@ static int trace__process_event(struct trace *trace, struct machine *machine,
 	case PERF_RECORD_LOST:
 		color_fprintf(trace->output, PERF_COLOR_RED,
 			      "LOST %" PRIu64 " events!\n", event->lost.lost);
-		ret = machine__process_lost_event(machine, event);
+		ret = machine__process_lost_event(machine, event, sample);
 	default:
-		ret = machine__process_event(machine, event);
+		ret = machine__process_event(machine, event, sample);
 		break;
 	}
 
@@ -1053,11 +1053,11 @@ static int trace__process_event(struct trace *trace, struct machine *machine,
 
 static int trace__tool_process(struct perf_tool *tool,
 			       union perf_event *event,
-			       struct perf_sample *sample __maybe_unused,
+			       struct perf_sample *sample,
 			       struct machine *machine)
 {
 	struct trace *trace = container_of(tool, struct trace, tool);
-	return trace__process_event(trace, machine, event);
+	return trace__process_event(trace, machine, event, sample);
 }
 
 static int trace__symbols_init(struct trace *trace, struct perf_evlist *evlist)
@@ -1581,7 +1581,7 @@ again:
 				trace->base_time = sample.time;
 
 			if (type != PERF_RECORD_SAMPLE) {
-				trace__process_event(trace, &trace->host, event);
+				trace__process_event(trace, &trace->host, event, &sample);
 				continue;
 			}
 
diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
index 6fb781d5586c..38d233a27de6 100644
--- a/tools/perf/tests/code-reading.c
+++ b/tools/perf/tests/code-reading.c
@@ -276,7 +276,7 @@ static int process_event(struct machine *machine, struct perf_evlist *evlist,
 		return process_sample_event(machine, evlist, event, state);
 
 	if (event->header.type < PERF_RECORD_MAX)
-		return machine__process_event(machine, event);
+		return machine__process_event(machine, event, NULL);
 
 	return 0;
 }
diff --git a/tools/perf/tests/hists_link.c b/tools/perf/tests/hists_link.c
index 735cff2c52ce..56f811d55a6f 100644
--- a/tools/perf/tests/hists_link.c
+++ b/tools/perf/tests/hists_link.c
@@ -93,7 +93,7 @@ static struct machine *setup_fake_machine(struct machines *machines)
 		if (thread == NULL)
 			goto out;
 
-		thread__set_comm(thread, fake_threads[i].comm);
+		thread__set_comm(thread, fake_threads[i].comm, 0);
 	}
 
 	for (i = 0; i < ARRAY_SIZE(fake_mmap_info); i++) {
@@ -110,7 +110,7 @@ static struct machine *setup_fake_machine(struct machines *machines)
 		strcpy(fake_mmap_event.mmap.filename,
 		       fake_mmap_info[i].filename);
 
-		machine__process_mmap_event(machine, &fake_mmap_event);
+		machine__process_mmap_event(machine, &fake_mmap_event, NULL);
 	}
 
 	for (i = 0; i < ARRAY_SIZE(fake_symbols); i++) {
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index d639bfe29309..c7b2458b47a6 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -516,18 +516,18 @@ size_t perf_event__fprintf_comm(union perf_event *event, FILE *fp)
 
 int perf_event__process_comm(struct perf_tool *tool __maybe_unused,
 			     union perf_event *event,
-			     struct perf_sample *sample __maybe_unused,
+			     struct perf_sample *sample,
 			     struct machine *machine)
 {
-	return machine__process_comm_event(machine, event);
+	return machine__process_comm_event(machine, event, sample);
 }
 
 int perf_event__process_lost(struct perf_tool *tool __maybe_unused,
 			     union perf_event *event,
-			     struct perf_sample *sample __maybe_unused,
+			     struct perf_sample *sample,
 			     struct machine *machine)
 {
-	return machine__process_lost_event(machine, event);
+	return machine__process_lost_event(machine, event, sample);
 }
 
 size_t perf_event__fprintf_mmap(union perf_event *event, FILE *fp)
@@ -550,18 +550,18 @@ size_t perf_event__fprintf_mmap2(union perf_event *event, FILE *fp)
 
 int perf_event__process_mmap(struct perf_tool *tool __maybe_unused,
 			     union perf_event *event,
-			     struct perf_sample *sample __maybe_unused,
+			     struct perf_sample *sample,
 			     struct machine *machine)
 {
-	return machine__process_mmap_event(machine, event);
+	return machine__process_mmap_event(machine, event, sample);
 }
 
 int perf_event__process_mmap2(struct perf_tool *tool __maybe_unused,
 			     union perf_event *event,
-			     struct perf_sample *sample __maybe_unused,
+			     struct perf_sample *sample,
 			     struct machine *machine)
 {
-	return machine__process_mmap2_event(machine, event);
+	return machine__process_mmap2_event(machine, event, sample);
 }
 
 size_t perf_event__fprintf_task(union perf_event *event, FILE *fp)
@@ -573,18 +573,18 @@ size_t perf_event__fprintf_task(union perf_event *event, FILE *fp)
 
 int perf_event__process_fork(struct perf_tool *tool __maybe_unused,
 			     union perf_event *event,
-			     struct perf_sample *sample __maybe_unused,
+			     struct perf_sample *sample,
 			     struct machine *machine)
 {
-	return machine__process_fork_event(machine, event);
+	return machine__process_fork_event(machine, event, sample);
 }
 
 int perf_event__process_exit(struct perf_tool *tool __maybe_unused,
 			     union perf_event *event,
-			     struct perf_sample *sample __maybe_unused,
+			     struct perf_sample *sample,
 			     struct machine *machine)
 {
-	return machine__process_exit_event(machine, event);
+	return machine__process_exit_event(machine, event, sample);
 }
 
 size_t perf_event__fprintf(union perf_event *event, FILE *fp)
@@ -615,10 +615,10 @@ size_t perf_event__fprintf(union perf_event *event, FILE *fp)
 
 int perf_event__process(struct perf_tool *tool __maybe_unused,
 			union perf_event *event,
-			struct perf_sample *sample __maybe_unused,
+			struct perf_sample *sample,
 			struct machine *machine)
 {
-	return machine__process_event(machine, event);
+	return machine__process_event(machine, event, sample);
 }
 
 void thread__find_addr_map(struct thread *self,
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 933d14f287ca..b8fc8a0d03ec 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -40,7 +40,7 @@ int machine__init(struct machine *machine, const char *root_dir, pid_t pid)
 			return -ENOMEM;
 
 		snprintf(comm, sizeof(comm), "[guest/%d]", pid);
-		thread__set_comm(thread, comm);
+		thread__set_comm(thread, comm, 0);
 	}
 
 	return 0;
@@ -314,7 +314,8 @@ struct thread *machine__find_thread(struct machine *machine, pid_t tid)
 	return __machine__findnew_thread(machine, 0, tid, false);
 }
 
-int machine__process_comm_event(struct machine *machine, union perf_event *event)
+int machine__process_comm_event(struct machine *machine, union perf_event *event,
+				struct perf_sample *sample)
 {
 	struct thread *thread = machine__findnew_thread(machine,
 							event->comm.pid,
@@ -323,7 +324,7 @@ int machine__process_comm_event(struct machine *machine, union perf_event *event
 	if (dump_trace)
 		perf_event__fprintf_comm(event, stdout);
 
-	if (thread == NULL || thread__set_comm(thread, event->comm.comm)) {
+	if (thread == NULL || thread__set_comm(thread, event->comm.comm, sample->time)) {
 		dump_printf("problem processing PERF_RECORD_COMM, skipping event.\n");
 		return -1;
 	}
@@ -332,7 +333,7 @@ int machine__process_comm_event(struct machine *machine, union perf_event *event
 }
 
 int machine__process_lost_event(struct machine *machine __maybe_unused,
-				union perf_event *event)
+				union perf_event *event, struct perf_sample *sample __maybe_unused)
 {
 	dump_printf(": id:%" PRIu64 ": lost:%" PRIu64 "\n",
 		    event->lost.id, event->lost.lost);
@@ -998,7 +999,8 @@ out_problem:
 }
 
 int machine__process_mmap2_event(struct machine *machine,
-				 union perf_event *event)
+				 union perf_event *event,
+				 struct perf_sample *sample __maybe_unused)
 {
 	u8 cpumode = event->header.misc & PERF_RECORD_MISC_CPUMODE_MASK;
 	struct thread *thread;
@@ -1045,7 +1047,8 @@ out_problem:
 	return 0;
 }
 
-int machine__process_mmap_event(struct machine *machine, union perf_event *event)
+int machine__process_mmap_event(struct machine *machine, union perf_event *event,
+				struct perf_sample *sample __maybe_unused)
 {
 	u8 cpumode = event->header.misc & PERF_RECORD_MISC_CPUMODE_MASK;
 	struct thread *thread;
@@ -1102,7 +1105,8 @@ static void machine__remove_thread(struct machine *machine, struct thread *th)
 	list_add_tail(&th->node, &machine->dead_threads);
 }
 
-int machine__process_fork_event(struct machine *machine, union perf_event *event)
+int machine__process_fork_event(struct machine *machine, union perf_event *event,
+				struct perf_sample *sample)
 {
 	struct thread *thread = machine__find_thread(machine, event->fork.tid);
 	struct thread *parent = machine__findnew_thread(machine,
@@ -1119,7 +1123,7 @@ int machine__process_fork_event(struct machine *machine, union perf_event *event
 		perf_event__fprintf_task(event, stdout);
 
 	if (thread == NULL || parent == NULL ||
-	    thread__fork(thread, parent) < 0) {
+	    thread__fork(thread, parent, sample->time) < 0) {
 		dump_printf("problem processing PERF_RECORD_FORK, skipping event.\n");
 		return -1;
 	}
@@ -1127,8 +1131,8 @@ int machine__process_fork_event(struct machine *machine, union perf_event *event
 	return 0;
 }
 
-int machine__process_exit_event(struct machine *machine __maybe_unused,
-				union perf_event *event)
+int machine__process_exit_event(struct machine *machine, union perf_event *event,
+				struct perf_sample *sample __maybe_unused)
 {
 	struct thread *thread = machine__find_thread(machine, event->fork.tid);
 
@@ -1141,23 +1145,24 @@ int machine__process_exit_event(struct machine *machine __maybe_unused,
 	return 0;
 }
 
-int machine__process_event(struct machine *machine, union perf_event *event)
+int machine__process_event(struct machine *machine, union perf_event *event,
+			   struct perf_sample *sample)
 {
 	int ret;
 
 	switch (event->header.type) {
 	case PERF_RECORD_COMM:
-		ret = machine__process_comm_event(machine, event); break;
+		ret = machine__process_comm_event(machine, event, sample); break;
 	case PERF_RECORD_MMAP:
-		ret = machine__process_mmap_event(machine, event); break;
+		ret = machine__process_mmap_event(machine, event, sample); break;
 	case PERF_RECORD_MMAP2:
-		ret = machine__process_mmap2_event(machine, event); break;
+		ret = machine__process_mmap2_event(machine, event, sample); break;
 	case PERF_RECORD_FORK:
-		ret = machine__process_fork_event(machine, event); break;
+		ret = machine__process_fork_event(machine, event, sample); break;
 	case PERF_RECORD_EXIT:
-		ret = machine__process_exit_event(machine, event); break;
+		ret = machine__process_exit_event(machine, event, sample); break;
 	case PERF_RECORD_LOST:
-		ret = machine__process_lost_event(machine, event); break;
+		ret = machine__process_lost_event(machine, event, sample); break;
 	default:
 		ret = -1;
 		break;
diff --git a/tools/perf/util/machine.h b/tools/perf/util/machine.h
index 58a6be1fc739..1db29d8ce595 100644
--- a/tools/perf/util/machine.h
+++ b/tools/perf/util/machine.h
@@ -40,13 +40,20 @@ struct map *machine__kernel_map(struct machine *machine, enum map_type type)
 
 struct thread *machine__find_thread(struct machine *machine, pid_t tid);
 
-int machine__process_comm_event(struct machine *machine, union perf_event *event);
-int machine__process_exit_event(struct machine *machine, union perf_event *event);
-int machine__process_fork_event(struct machine *machine, union perf_event *event);
-int machine__process_lost_event(struct machine *machine, union perf_event *event);
-int machine__process_mmap_event(struct machine *machine, union perf_event *event);
-int machine__process_mmap2_event(struct machine *machine, union perf_event *event);
-int machine__process_event(struct machine *machine, union perf_event *event);
+int machine__process_comm_event(struct machine *machine, union perf_event *event,
+				struct perf_sample *sample);
+int machine__process_exit_event(struct machine *machine, union perf_event *event,
+				struct perf_sample *sample);
+int machine__process_fork_event(struct machine *machine, union perf_event *event,
+				struct perf_sample *sample);
+int machine__process_lost_event(struct machine *machine, union perf_event *event,
+				struct perf_sample *sample);
+int machine__process_mmap_event(struct machine *machine, union perf_event *event,
+				struct perf_sample *sample);
+int machine__process_mmap2_event(struct machine *machine, union perf_event *event,
+				 struct perf_sample *sample);
+int machine__process_event(struct machine *machine, union perf_event *event,
+				struct perf_sample *sample);
 
 typedef void (*machine__process_t)(struct machine *machine, void *data);
 
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 9f62be8bc167..8de3ab388187 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1130,7 +1130,7 @@ static struct thread *perf_session__register_idle_thread(struct perf_session *se
 {
 	struct thread *thread = perf_session__findnew(self, 0);
 
-	if (thread == NULL || thread__set_comm(thread, "swapper")) {
+	if (thread == NULL || thread__set_comm(thread, "swapper", 0)) {
 		pr_err("problem inserting idle task.\n");
 		thread = NULL;
 	}
diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
index bb50b9fda9c2..509abed750e6 100644
--- a/tools/perf/util/thread.c
+++ b/tools/perf/util/thread.c
@@ -31,7 +31,8 @@ void thread__delete(struct thread *self)
 	free(self);
 }
 
-int thread__set_comm(struct thread *self, const char *comm)
+int thread__set_comm(struct thread *self, const char *comm,
+		     u64 timestamp __maybe_unused)
 {
 	int err;
 
@@ -73,7 +74,8 @@ void thread__insert_map(struct thread *self, struct map *map)
 	map_groups__insert(&self->mg, map);
 }
 
-int thread__fork(struct thread *self, struct thread *parent)
+int thread__fork(struct thread *self, struct thread *parent,
+		 u64 timestamp __maybe_unused)
 {
 	int i;
 
diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
index 84db214d90bd..b738c9948508 100644
--- a/tools/perf/util/thread.h
+++ b/tools/perf/util/thread.h
@@ -33,11 +33,11 @@ static inline void thread__exited(struct thread *thread)
 	thread->dead = true;
 }
 
-int thread__set_comm(struct thread *self, const char *comm);
+int thread__set_comm(struct thread *self, const char *comm, u64 timestamp);
 int thread__comm_len(struct thread *self);
 const char *thread__comm_curr(const struct thread *thread);
 void thread__insert_map(struct thread *self, struct map *map);
-int thread__fork(struct thread *self, struct thread *parent);
+int thread__fork(struct thread *self, struct thread *parent, u64 timestamp);
 size_t thread__fprintf(struct thread *thread, FILE *fp);
 
 static inline struct map *thread__find_map(struct thread *self,
-- 
1.7.11.7

[PATCH 6/8] perf tools: Add new comm infrastructure

[Namhyung Kim]

From: Frederic Weisbecker <fweisbec@gmail.com>

This new comm infrastructure provides two features:

1) It keeps track of all comms lifecycle for a given thread. This
way we can associate a timeframe to any thread comm, as long as
PERF_SAMPLE_TIME samples are joined to comm and fork events.

As a result we should have more precise comm sorted hists with
seperated entries for pre and post exec time after a fork.

2) It also makes sure that a given comm string is not duplicated
but rather shared among the threads that refer to it. This way the
threads comm can be compared against pointer values from the sort
infrastructure.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/n/tip-hwjf70b2wve9m2kosxiq8bb3@git.kernel.org
[ Fixed up minor const pointer issues and removed 'self' usage ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/Makefile      |   2 +
 tools/perf/util/comm.c   | 106 +++++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/comm.h   |  20 +++++++++
 tools/perf/util/thread.c | 100 +++++++++++++++++++++++++++++++-------------
 tools/perf/util/thread.h |   3 +-
 5 files changed, 202 insertions(+), 29 deletions(-)
 create mode 100644 tools/perf/util/comm.c
 create mode 100644 tools/perf/util/comm.h

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index b62e12d2ba6b..42001f387e7f 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -272,6 +272,7 @@ LIB_H += util/color.h
 LIB_H += util/values.h
 LIB_H += util/sort.h
 LIB_H += util/hist.h
+LIB_H += util/comm.h
 LIB_H += util/thread.h
 LIB_H += util/thread_map.h
 LIB_H += util/trace-event.h
@@ -339,6 +340,7 @@ LIB_OBJS += $(OUTPUT)util/machine.o
 LIB_OBJS += $(OUTPUT)util/map.o
 LIB_OBJS += $(OUTPUT)util/pstack.o
 LIB_OBJS += $(OUTPUT)util/session.o
+LIB_OBJS += $(OUTPUT)util/comm.o
 LIB_OBJS += $(OUTPUT)util/thread.o
 LIB_OBJS += $(OUTPUT)util/thread_map.o
 LIB_OBJS += $(OUTPUT)util/trace-event-parse.o
diff --git a/tools/perf/util/comm.c b/tools/perf/util/comm.c
new file mode 100644
index 000000000000..0cfb55202537
--- /dev/null
+++ b/tools/perf/util/comm.c
@@ -0,0 +1,106 @@
+#include "comm.h"
+#include "util.h"
+#include <stdlib.h>
+#include <stdio.h>
+
+struct comm_str {
+	char *str;
+	struct rb_node rb_node;
+	int ref;
+};
+
+/* Should perhaps be moved to struct machine */
+static struct rb_root comm_str_root;
+
+static void comm_str_get(struct comm_str *cs)
+{
+	cs->ref++;
+}
+
+static void comm_str_put(struct comm_str *cs)
+{
+	if (!--cs->ref) {
+		rb_erase(&cs->rb_node, &comm_str_root);
+		free(cs->str);
+		free(cs);
+	}
+}
+
+static struct comm_str *comm_str_alloc(const char *str)
+{
+	struct comm_str *cs;
+
+	cs = zalloc(sizeof(*cs));
+	if (!cs)
+		return NULL;
+
+	cs->str = strdup(str);
+	if (!cs->str) {
+		free(cs);
+		return NULL;
+	}
+
+	return cs;
+}
+
+static struct comm_str *comm_str_findnew(const char *str, struct rb_root *root)
+{
+	struct rb_node **p = &root->rb_node;
+	struct rb_node *parent = NULL;
+	struct comm_str *iter, *new;
+	int cmp;
+
+	while (*p != NULL) {
+		parent = *p;
+		iter = rb_entry(parent, struct comm_str, rb_node);
+
+		cmp = strcmp(str, iter->str);
+		if (!cmp)
+			return iter;
+
+		if (cmp < 0)
+			p = &(*p)->rb_left;
+		else
+			p = &(*p)->rb_right;
+	}
+
+	new = comm_str_alloc(str);
+	if (!new)
+		return NULL;
+
+	rb_link_node(&new->rb_node, parent, p);
+	rb_insert_color(&new->rb_node, root);
+
+	return new;
+}
+
+struct comm *comm__new(const char *str, u64 timestamp)
+{
+	struct comm *comm = zalloc(sizeof(*comm));
+
+	if (!comm)
+		return NULL;
+
+	comm->start = timestamp;
+
+	comm->comm_str = comm_str_findnew(str, &comm_str_root);
+	if (!comm->comm_str) {
+		free(comm);
+		return NULL;
+	}
+
+	comm_str_get(comm->comm_str);
+
+	return comm;
+}
+
+void comm__free(struct comm *comm)
+{
+	comm_str_put(comm->comm_str);
+	free(comm);
+}
+
+const char *comm__str(const struct comm *comm)
+{
+	return comm->comm_str->str;
+}
diff --git a/tools/perf/util/comm.h b/tools/perf/util/comm.h
new file mode 100644
index 000000000000..f62d215bede2
--- /dev/null
+++ b/tools/perf/util/comm.h
@@ -0,0 +1,20 @@
+#ifndef __PERF_COMM_H
+#define __PERF_COMM_H
+
+#include "../perf.h"
+#include <linux/rbtree.h>
+#include <linux/list.h>
+
+struct comm_str;
+
+struct comm {
+	struct comm_str *comm_str;
+	u64 start;
+	struct list_head list;
+};
+
+void comm__free(struct comm *comm);
+struct comm *comm__new(const char *str, u64 timestamp);
+const char *comm__str(const struct comm *comm);
+
+#endif  /* __PERF_COMM_H */
diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
index 509abed750e6..9cb01566eff1 100644
--- a/tools/perf/util/thread.c
+++ b/tools/perf/util/thread.c
@@ -6,9 +6,12 @@
 #include "thread.h"
 #include "util.h"
 #include "debug.h"
+#include "comm.h"
 
 struct thread *thread__new(pid_t pid, pid_t tid)
 {
+	char *comm_str;
+	struct comm *comm;
 	struct thread *self = zalloc(sizeof(*self));
 
 	if (self != NULL) {
@@ -16,47 +19,88 @@ struct thread *thread__new(pid_t pid, pid_t tid)
 		self->pid_ = pid;
 		self->tid = tid;
 		self->ppid = -1;
-		self->comm = malloc(32);
-		if (self->comm)
-			snprintf(self->comm, 32, ":%d", self->tid);
+		INIT_LIST_HEAD(&self->comm_list);
+
+		comm_str = malloc(32);
+		if (!comm_str)
+			goto err_thread;
+
+		snprintf(comm_str, 32, ":%d", tid);
+		comm = comm__new(comm_str, 0);
+		free(comm_str);
+		if (!comm)
+			goto err_thread;
+
+		list_add(&comm->list, &self->comm_list);
 	}
 
 	return self;
+
+err_thread:
+	free(self);
+	return NULL;
 }
 
 void thread__delete(struct thread *self)
 {
+	struct comm *comm, *tmp;
+
 	map_groups__exit(&self->mg);
-	free(self->comm);
+	list_for_each_entry_safe(comm, tmp, &self->comm_list, list) {
+		list_del(&comm->list);
+		comm__free(comm);
+	}
+
 	free(self);
 }
 
-int thread__set_comm(struct thread *self, const char *comm,
-		     u64 timestamp __maybe_unused)
+static struct comm *curr_comm(const struct thread *thread)
 {
-	int err;
-
-	if (self->comm)
-		free(self->comm);
-	self->comm = strdup(comm);
-	err = self->comm == NULL ? -ENOMEM : 0;
-	if (!err) {
-		self->comm_set = true;
+	if (list_empty(&thread->comm_list))
+		return NULL;
+
+	return list_first_entry(&thread->comm_list, struct comm, list);
+}
+
+/* CHECKME: time should always be 0 if event aren't ordered */
+int thread__set_comm(struct thread *thread, const char *str, u64 timestamp)
+{
+	struct comm *new, *curr = curr_comm(thread);
+
+	/* Override latest entry if it had no specific time coverage */
+	if (!curr->start) {
+		list_del(&curr->list);
+		comm__free(curr);
 	}
-	return err;
+
+	new = comm__new(str, timestamp);
+	if (!new)
+		return -ENOMEM;
+
+	list_add(&new->list, &thread->comm_list);
+	thread->comm_set = true;
+
+	return 0;
 }
 
 const char *thread__comm_curr(const struct thread *thread)
 {
-	return thread->comm;
+	const struct comm *comm = curr_comm(thread);
+
+	if (!comm)
+		return NULL;
+
+	return comm__str(comm);
 }
 
+/* CHECKME: it should probably better return the max comm len from its comm list */
 int thread__comm_len(struct thread *self)
 {
 	if (!self->comm_len) {
-		if (!self->comm)
+		const char *comm = thread__comm_curr(self);
+		if (!comm)
 			return 0;
-		self->comm_len = strlen(self->comm);
+		self->comm_len = strlen(comm);
 	}
 
 	return self->comm_len;
@@ -74,25 +118,25 @@ void thread__insert_map(struct thread *self, struct map *map)
 	map_groups__insert(&self->mg, map);
 }
 
-int thread__fork(struct thread *self, struct thread *parent,
-		 u64 timestamp __maybe_unused)
+int thread__fork(struct thread *thread, struct thread *parent, u64 timestamp)
 {
-	int i;
+	int i, err;
 
 	if (parent->comm_set) {
-		if (self->comm)
-			free(self->comm);
-		self->comm = strdup(parent->comm);
-		if (!self->comm)
+		const char *comm = thread__comm_curr(parent);
+		if (!comm)
 			return -ENOMEM;
-		self->comm_set = true;
+		err = thread__set_comm(thread, comm, timestamp);
+		if (!err)
+			return err;
+		thread->comm_set = true;
 	}
 
 	for (i = 0; i < MAP__NR_TYPES; ++i)
-		if (map_groups__clone(&self->mg, &parent->mg, i) < 0)
+		if (map_groups__clone(&thread->mg, &parent->mg, i) < 0)
 			return -ENOMEM;
 
-	self->ppid = parent->tid;
+	thread->ppid = parent->tid;
 
 	return 0;
 }
diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
index b738c9948508..0ec37bae88d0 100644
--- a/tools/perf/util/thread.h
+++ b/tools/perf/util/thread.h
@@ -2,6 +2,7 @@
 #define __PERF_THREAD_H
 
 #include <linux/rbtree.h>
+#include <linux/list.h>
 #include <unistd.h>
 #include <sys/types.h>
 #include "symbol.h"
@@ -18,7 +19,7 @@ struct thread {
 	char			shortname[3];
 	bool			comm_set;
 	bool			dead; /* if set thread has exited */
-	char			*comm;
+	struct list_head	comm_list;
 	int			comm_len;
 
 	void			*priv;
-- 
1.7.11.7

[PATCH 7/8] perf tools: Compare hists comm by addresses

[Namhyung Kim]

From: Frederic Weisbecker <fweisbec@gmail.com>

Now that comm strings are allocated only once and refcounted to be shared
among threads, these can now be safely compared by addresses. This
should remove most hists collapses on post processing.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Stephane Eranian <eranian@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/sort.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 65fe958ce30b..48e6a38132c9 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -79,7 +79,8 @@ struct sort_entry sort_thread = {
 static int64_t
 sort__comm_cmp(struct hist_entry *left, struct hist_entry *right)
 {
-	return right->thread->tid - left->thread->tid;
+	/* Compare the addr that should be unique among comm */
+	return thread__comm_curr(right->thread) - thread__comm_curr(left->thread);
 }
 
 static int64_t
-- 
1.7.11.7

[PATCH 8/8] perf tools: Get current comm instead of last one

[Namhyung Kim]

From: Namhyung Kim <namhyung.kim@lge.com>

At insert time, a hist entry should reference comm at the time
otherwise it'll get the last comm anyway.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Link: http://lkml.kernel.org/n/tip-n6pykiiymtgmcjs834go2t8x@git.kernel.org
[ Fixed up const pointer issues ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/comm.c   | 15 +++++++++++++++
 tools/perf/util/comm.h   |  1 +
 tools/perf/util/hist.c   |  3 +++
 tools/perf/util/sort.c   | 14 +++++---------
 tools/perf/util/sort.h   |  1 +
 tools/perf/util/thread.c |  6 +++---
 tools/perf/util/thread.h |  2 ++
 7 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/tools/perf/util/comm.c b/tools/perf/util/comm.c
index 0cfb55202537..ecd3c9938d3e 100644
--- a/tools/perf/util/comm.c
+++ b/tools/perf/util/comm.c
@@ -94,6 +94,21 @@ struct comm *comm__new(const char *str, u64 timestamp)
 	return comm;
 }
 
+void comm__override(struct comm *comm, const char *str, u64 timestamp)
+{
+	struct comm_str *old = comm->comm_str;
+
+	comm->comm_str = comm_str_findnew(str, &comm_str_root);
+	if (!comm->comm_str) {
+		comm->comm_str = old;
+		return;
+	}
+
+	comm->start = timestamp;
+	comm_str_get(comm->comm_str);
+	comm_str_put(old);
+}
+
 void comm__free(struct comm *comm)
 {
 	comm_str_put(comm->comm_str);
diff --git a/tools/perf/util/comm.h b/tools/perf/util/comm.h
index f62d215bede2..b2c364b37468 100644
--- a/tools/perf/util/comm.h
+++ b/tools/perf/util/comm.h
@@ -16,5 +16,6 @@ struct comm {
 void comm__free(struct comm *comm);
 struct comm *comm__new(const char *str, u64 timestamp);
 const char *comm__str(const struct comm *comm);
+void comm__override(struct comm *self, const char *str, u64 timestamp);
 
 #endif  /* __PERF_COMM_H */
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index a2d58e7abdcf..1aa97b012a09 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -412,6 +412,7 @@ struct hist_entry *__hists__add_mem_entry(struct hists *self,
 {
 	struct hist_entry entry = {
 		.thread	= al->thread,
+		.comm = curr_comm(al->thread),
 		.ms = {
 			.map	= al->map,
 			.sym	= al->sym,
@@ -442,6 +443,7 @@ struct hist_entry *__hists__add_branch_entry(struct hists *self,
 {
 	struct hist_entry entry = {
 		.thread	= al->thread,
+		.comm = curr_comm(al->thread),
 		.ms = {
 			.map	= bi->to.map,
 			.sym	= bi->to.sym,
@@ -471,6 +473,7 @@ struct hist_entry *__hists__add_entry(struct hists *self,
 {
 	struct hist_entry entry = {
 		.thread	= al->thread,
+		.comm = curr_comm(al->thread),
 		.ms = {
 			.map	= al->map,
 			.sym	= al->sym,
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 48e6a38132c9..eefc0e623966 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -1,5 +1,6 @@
 #include "sort.h"
 #include "hist.h"
+#include "comm.h"
 #include "symbol.h"
 
 regex_t		parent_regex;
@@ -80,25 +81,20 @@ static int64_t
 sort__comm_cmp(struct hist_entry *left, struct hist_entry *right)
 {
 	/* Compare the addr that should be unique among comm */
-	return thread__comm_curr(right->thread) - thread__comm_curr(left->thread);
+	return comm__str(right->comm) - comm__str(left->comm);
 }
 
 static int64_t
 sort__comm_collapse(struct hist_entry *left, struct hist_entry *right)
 {
-	const char *comm_l = thread__comm_curr(left->thread);
-	const char *comm_r = thread__comm_curr(right->thread);
-
-	if (!comm_l || !comm_r)
-		return cmp_null(comm_l, comm_r);
-
-	return strcmp(comm_l, comm_r);
+	/* Compare the addr that should be unique among comm */
+	return comm__str(right->comm) - comm__str(left->comm);
 }
 
 static int hist_entry__comm_snprintf(struct hist_entry *self, char *bf,
 				     size_t size, unsigned int width)
 {
-	return repsep_snprintf(bf, size, "%*s", width, thread__comm_curr(self->thread));
+	return repsep_snprintf(bf, size, "%*s", width, comm__str(self->comm));
 }
 
 struct sort_entry sort_comm = {
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index 4e80dbd271e7..4d0153f83e3c 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -84,6 +84,7 @@ struct hist_entry {
 	struct he_stat		stat;
 	struct map_symbol	ms;
 	struct thread		*thread;
+	struct comm		*comm;
 	u64			ip;
 	s32			cpu;
 
diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
index 9cb01566eff1..444de7a8527f 100644
--- a/tools/perf/util/thread.c
+++ b/tools/perf/util/thread.c
@@ -54,7 +54,7 @@ void thread__delete(struct thread *self)
 	free(self);
 }
 
-static struct comm *curr_comm(const struct thread *thread)
+struct comm *curr_comm(const struct thread *thread)
 {
 	if (list_empty(&thread->comm_list))
 		return NULL;
@@ -69,8 +69,8 @@ int thread__set_comm(struct thread *thread, const char *str, u64 timestamp)
 
 	/* Override latest entry if it had no specific time coverage */
 	if (!curr->start) {
-		list_del(&curr->list);
-		comm__free(curr);
+		comm__override(curr, str, timestamp);
+		return 0;
 	}
 
 	new = comm__new(str, timestamp);
diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
index 0ec37bae88d0..196c6c2dfbe1 100644
--- a/tools/perf/util/thread.h
+++ b/tools/perf/util/thread.h
@@ -26,6 +26,7 @@ struct thread {
 };
 
 struct machine;
+struct comm;
 
 struct thread *thread__new(pid_t pid, pid_t tid);
 void thread__delete(struct thread *self);
@@ -36,6 +37,7 @@ static inline void thread__exited(struct thread *thread)
 
 int thread__set_comm(struct thread *self, const char *comm, u64 timestamp);
 int thread__comm_len(struct thread *self);
+struct comm *curr_comm(const struct thread *self);
 const char *thread__comm_curr(const struct thread *thread);
 void thread__insert_map(struct thread *self, struct map *map);
 int thread__fork(struct thread *self, struct thread *parent, u64 timestamp);
-- 
1.7.11.7

Re: [PATCHSET 0/8] perf tools: Fix scalability problem on callchain merging (v4)

[Ingo Molnar]

* Namhyung Kim <namhyung@kernel.org> wrote:

> Hello,
> 
> This is a new version of callchain improvement patchset.  I found and
> fixed bugs in the previous version.  I verified that it produced
> exactly same output before and after applying rbtree conversion patch
> (#1).  However after Frederic's new comm infrastructure patches are
> applied it'd be little different.
> 
> The patches are on 'perf/callchain-v4' branch in my tree
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
> 
> 
> Please test!
> 
> Thanks,
> Namhyung
> 
> 
> Frederic Weisbecker (4):
>   perf tools: Use an accessor to read thread comm
>   perf tools: Add time argument on comm setting
>   perf tools: Add new comm infrastructure
>   perf tools: Compare hists comm by addresses
> 
> Namhyung Kim (4):
>   perf callchain: Convert children list to rbtree
>   perf ui/progress: Add new helper functions for progress bar
>   perf tools: Show progress on histogram collapsing
>   perf tools: Get current comm instead of last one

>  31 files changed, 504 insertions(+), 177 deletions(-)

I pulled this into a test-branch and resolved the builtin-trace.c conflict 
with tip:master.

I tried your new code out with Linus's testcase of a parallel kernel 
build:

   perf record -g -- make -j8 bzImage

The 'perf record' session went mostly fine except for lost events:

  Kernel: arch/x86/boot/bzImage is ready  (#25)
  [ perf record: Woken up 553 times to write data ]
  [ perf record: Captured and wrote 196.811 MB perf.data (~8598789 samples) ]
  Warning:
  Processed 2631982 events and lost 54 chunks!

  Check IO/CPU overload!

So, for completeness I'll mention that perf fails in two ways here:

  - UI bug: it prints some scary warnings about 'IO/CPU overload' but 
    does not give any idea what the user could do about it. At minimum it 
    should print something about increasing --mmap-pages. It should also 
    print the current value of --mmap-pages so that the user knows to what 
    value to increase it ...

  - defaults bug: this isn't an extreme workload on an extreme box - it's
    a relatively bog standard system with 8 cores and 16 CPUs. The kernel
    build was mild as well, with -j8. So losing events in perf record is
    absolutely unacceptable. A solution might be to automatically increase 
    the ring-buffer if -g is used, in expectation of a higher data rate.

Once perf record was done I had the ~200 MB perf.data - and the 'perf 
report' session went much smoother than ever before: the analysis went 
very quickly, it finished within 10 seconds and displayed a nice progress 
bar. So this is entirely usable IMO.

One small 'perf report' annoyance is that during the analysis passes 
missing symbol printouts flash in the TUI - without the user having a 
chance to read them. So those messages should either linger, or be 
displayed at a later stage, for example when the user is confronted with 
non-resolved symbols like:

+  28.63%             cc1  cc1                                [.] 0x00000000006a92cb                                  ◆

that is the point where the message would be useful - but nothing 
indicates at all that this is an undesirable symbol entry and nothing 
helps the user what to do about it!

A solution might be to display non-intrusive messages about non-resolved 
symbols when such a symbol is manipulated (its children are openened, or 
annotation is attempted).

Here there is a second annoyance: on the detailed screen the 'annotate' 
entry is simply missing when the symbol has not been resolved. If I hit 
'a' on the symbol entry itself in the graph view, then sometimes 
annotation works - sometimes it does not and there's no UI feedback 
whatsoever why it's not working.

I'm not suggesting to change the keyboard flow - that is very smooth - but 
display information about failed annotations in the status line at the 
bottom of the screen would be very useful. Remember: it's _always_ an UI 
bug if the user hits a key and absolutely nothing happens. At minimum a 
low-key, non-intrusive 'key X not bound' message should appear in the 
status line bottom. Deterministic action/reaction sequences are utterly 
important when interacting with computers.

Anyway, very nice progress with the tree here!

Thanks,

	Ingo

Re: [PATCHSET 0/8] perf tools: Fix scalability problem on callchain merging (v4)

[David Ahern]

On 9/26/13 2:58 AM, Namhyung Kim wrote:
> Hello,
>
> This is a new version of callchain improvement patchset.  I found and
> fixed bugs in the previous version.  I verified that it produced
> exactly same output before and after applying rbtree conversion patch
> (#1).  However after Frederic's new comm infrastructure patches are
> applied it'd be little different.
>
> The patches are on 'perf/callchain-v4' branch in my tree
>
>    git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
>

Given recent breakage, has the patchset been run through any tests on 
older kernels that do not support sample_id_all? e.g., 2.6.34 (WRL4). 
What about tests with the other perf commands -- script to dump events, 
trace on a file with with multiple processes -- to verify no impact on 
comm output, especially multithreaded processes with named threads. I 
can certainly do those tests in time, but can't guarantee a timeframe 
and want to make sure it gets done before merging.

Thanks,
David

Re: [PATCHSET 0/8] perf tools: Fix scalability problem on callchain merging (v4)

[Arnaldo Carvalho de Melo]

Em Thu, Sep 26, 2013 at 07:46:36AM -0600, David Ahern escreveu:
> On 9/26/13 2:58 AM, Namhyung Kim wrote:
> >This is a new version of callchain improvement patchset.  I found and
> >fixed bugs in the previous version.  I verified that it produced
> >exactly same output before and after applying rbtree conversion patch
> >(#1).  However after Frederic's new comm infrastructure patches are
> >applied it'd be little different.

> >The patches are on 'perf/callchain-v4' branch in my tree

> >   git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git

> Given recent breakage, has the patchset been run through any tests
> on older kernels that do not support sample_id_all? e.g., 2.6.34
> (WRL4). What about tests with the other perf commands -- script to
> dump events, trace on a file with with multiple processes -- to
> verify no impact on comm output, especially multithreaded processes
> with named threads. I can certainly do those tests in time, but
> can't guarantee a timeframe and want to make sure it gets done
> before merging.

Right, and I don't think this is perf/urgent material at this point in
time, when merged will be for 3.13.

- Arnaldo

Re: [PATCHSET 0/8] perf tools: Fix scalability problem on callchain merging (v4)

[Namhyung Kim]

Hi Ingo,

On Thu, 26 Sep 2013 11:34:26 +0200, Ingo Molnar wrote:
> * Namhyung Kim <namhyung@kernel.org> wrote:
>
>> Hello,
>> 
>> This is a new version of callchain improvement patchset.  I found and
>> fixed bugs in the previous version.  I verified that it produced
>> exactly same output before and after applying rbtree conversion patch
>> (#1).  However after Frederic's new comm infrastructure patches are
>> applied it'd be little different.
>> 
>> The patches are on 'perf/callchain-v4' branch in my tree
>> 
>>   git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
>
> I pulled this into a test-branch and resolved the builtin-trace.c conflict 
> with tip:master.
>
> I tried your new code out with Linus's testcase of a parallel kernel 
> build:
>
>    perf record -g -- make -j8 bzImage
>
> The 'perf record' session went mostly fine except for lost events:
>
>   Kernel: arch/x86/boot/bzImage is ready  (#25)
>   [ perf record: Woken up 553 times to write data ]
>   [ perf record: Captured and wrote 196.811 MB perf.data (~8598789 samples) ]
>   Warning:
>   Processed 2631982 events and lost 54 chunks!
>
>   Check IO/CPU overload!
>
> So, for completeness I'll mention that perf fails in two ways here:
>
>   - UI bug: it prints some scary warnings about 'IO/CPU overload' but 
>     does not give any idea what the user could do about it. At minimum it 
>     should print something about increasing --mmap-pages. It should also 
>     print the current value of --mmap-pages so that the user knows to what 
>     value to increase it ...
>
>   - defaults bug: this isn't an extreme workload on an extreme box - it's
>     a relatively bog standard system with 8 cores and 16 CPUs. The kernel
>     build was mild as well, with -j8. So losing events in perf record is
>     absolutely unacceptable. A solution might be to automatically increase 
>     the ring-buffer if -g is used, in expectation of a higher data rate.

Both look sensible.  I'll try to cook patches for them.

>
> Once perf record was done I had the ~200 MB perf.data - and the 'perf 
> report' session went much smoother than ever before: the analysis went 
> very quickly, it finished within 10 seconds and displayed a nice progress 
> bar. So this is entirely usable IMO.
>
> One small 'perf report' annoyance is that during the analysis passes 
> missing symbol printouts flash in the TUI - without the user having a 
> chance to read them. So those messages should either linger, or be 
> displayed at a later stage, for example when the user is confronted with 
> non-resolved symbols like:
>
> +  28.63%             cc1  cc1                                [.] 0x00000000006a92cb                                  ◆
>
> that is the point where the message would be useful - but nothing 
> indicates at all that this is an undesirable symbol entry and nothing 
> helps the user what to do about it!
>
> A solution might be to display non-intrusive messages about non-resolved 
> symbols when such a symbol is manipulated (its children are openened, or 
> annotation is attempted).

Hmm.. I'll think about it more how to do it.

>
> Here there is a second annoyance: on the detailed screen the 'annotate' 
> entry is simply missing when the symbol has not been resolved. If I hit 
> 'a' on the symbol entry itself in the graph view, then sometimes 
> annotation works - sometimes it does not and there's no UI feedback 
> whatsoever why it's not working.
>
> I'm not suggesting to change the keyboard flow - that is very smooth - but 
> display information about failed annotations in the status line at the 
> bottom of the screen would be very useful. Remember: it's _always_ an UI 
> bug if the user hits a key and absolutely nothing happens. At minimum a 
> low-key, non-intrusive 'key X not bound' message should appear in the 
> status line bottom. Deterministic action/reaction sequences are utterly 
> important when interacting with computers.
>
> Anyway, very nice progress with the tree here!

Hehe, thanks!
Namhyung

Re: [PATCHSET 0/8] perf tools: Fix scalability problem on callchain merging (v4)

[Namhyung Kim]

Hi,

On Thu, 26 Sep 2013 11:07:49 -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Sep 26, 2013 at 07:46:36AM -0600, David Ahern escreveu:
>> On 9/26/13 2:58 AM, Namhyung Kim wrote:
>> >This is a new version of callchain improvement patchset.  I found and
>> >fixed bugs in the previous version.  I verified that it produced
>> >exactly same output before and after applying rbtree conversion patch
>> >(#1).  However after Frederic's new comm infrastructure patches are
>> >applied it'd be little different.
>
>> >The patches are on 'perf/callchain-v4' branch in my tree
>
>> >   git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
>
>> Given recent breakage, has the patchset been run through any tests
>> on older kernels that do not support sample_id_all? e.g., 2.6.34
>> (WRL4). What about tests with the other perf commands -- script to
>> dump events, trace on a file with with multiple processes -- to
>> verify no impact on comm output, especially multithreaded processes
>> with named threads. I can certainly do those tests in time, but
>> can't guarantee a timeframe and want to make sure it gets done
>> before merging.
>
> Right, and I don't think this is perf/urgent material at this point in
> time, when merged will be for 3.13.

Agreed.  It definitely needs more testing.  I'll try to do some testing
mentioned above.

Thanks,
Namhyung

Re: [PATCH 8/8] perf tools: Get current comm instead of last one

[Frederic Weisbecker]

On Thu, Sep 26, 2013 at 05:58:10PM +0900, Namhyung Kim wrote:
> From: Namhyung Kim <namhyung.kim@lge.com>
> 
> At insert time, a hist entry should reference comm at the time
> otherwise it'll get the last comm anyway.
> 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Link: http://lkml.kernel.org/n/tip-n6pykiiymtgmcjs834go2t8x@git.kernel.org
> [ Fixed up const pointer issues ]
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> ---
>  tools/perf/util/comm.c   | 15 +++++++++++++++
>  tools/perf/util/comm.h   |  1 +
>  tools/perf/util/hist.c   |  3 +++
>  tools/perf/util/sort.c   | 14 +++++---------
>  tools/perf/util/sort.h   |  1 +
>  tools/perf/util/thread.c |  6 +++---
>  tools/perf/util/thread.h |  2 ++
>  7 files changed, 30 insertions(+), 12 deletions(-)
> 
> diff --git a/tools/perf/util/comm.c b/tools/perf/util/comm.c
> index 0cfb55202537..ecd3c9938d3e 100644
> --- a/tools/perf/util/comm.c
> +++ b/tools/perf/util/comm.c
> @@ -94,6 +94,21 @@ struct comm *comm__new(const char *str, u64 timestamp)
>  	return comm;
>  }
>  
> +void comm__override(struct comm *comm, const char *str, u64 timestamp)
> +{
> +	struct comm_str *old = comm->comm_str;
> +
> +	comm->comm_str = comm_str_findnew(str, &comm_str_root);
> +	if (!comm->comm_str) {
> +		comm->comm_str = old;
> +		return;
> +	}
> +
> +	comm->start = timestamp;
> +	comm_str_get(comm->comm_str);
> +	comm_str_put(old);
> +}
> +
>  void comm__free(struct comm *comm)
>  {
>  	comm_str_put(comm->comm_str);
> diff --git a/tools/perf/util/comm.h b/tools/perf/util/comm.h
> index f62d215bede2..b2c364b37468 100644
> --- a/tools/perf/util/comm.h
> +++ b/tools/perf/util/comm.h
> @@ -16,5 +16,6 @@ struct comm {
>  void comm__free(struct comm *comm);
>  struct comm *comm__new(const char *str, u64 timestamp);
>  const char *comm__str(const struct comm *comm);
> +void comm__override(struct comm *self, const char *str, u64 timestamp);
>  
>  #endif  /* __PERF_COMM_H */
> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> index a2d58e7abdcf..1aa97b012a09 100644
> --- a/tools/perf/util/hist.c
> +++ b/tools/perf/util/hist.c
> @@ -412,6 +412,7 @@ struct hist_entry *__hists__add_mem_entry(struct hists *self,
>  {
>  	struct hist_entry entry = {
>  		.thread	= al->thread,
> +		.comm = curr_comm(al->thread),

So now that we have an accessor for that, it clashes a bit with existing names.

I suggest we rename thread__comm_curr() -> thread__comm_str()
And rename curr_comm() -> thread__comm()

Hmm?

Re: [PATCH 1/8] perf callchain: Convert children list to rbtree

[Frederic Weisbecker]

On Thu, Sep 26, 2013 at 05:58:03PM +0900, Namhyung Kim wrote:
> From: Namhyung Kim <namhyung.kim@lge.com>
> 
> Current collapse stage has a scalability problem which can be
> reproduced easily with parallel kernel build.  This is because it
> needs to traverse every children of callchain linearly during the
> collapse/merge stage.  Convert it to rbtree reduced the overhead
> significantly.
> 
> On my 400MB perf.data file which recorded with make -j32 kernel build:
> 
>   $ time perf --no-pager report --stdio > /dev/null
> 
> before:
>   real	6m22.073s
>   user	6m18.683s
>   sys	0m0.706s
> 
> after:
>   real	0m20.780s
>   user	0m19.962s
>   sys	0m0.689s
> 
> During the perf report the overhead on append_chain_children went down
> from 96.69% to 18.16%:
> 
>   -  18.16%  perf  perf                [.] append_chain_children
>      - append_chain_children
>         - 77.48% append_chain_children
>            + 69.79% merge_chain_branch
>            - 22.96% append_chain_children
>               + 67.44% merge_chain_branch
>               + 30.15% append_chain_children
>               + 2.41% callchain_append
>            + 7.25% callchain_append
>         + 12.26% callchain_append
>         + 10.22% merge_chain_branch
>   +  11.58%  perf  perf                [.] dso__find_symbol
>   +   8.02%  perf  perf                [.] sort__comm_cmp
>   +   5.48%  perf  libc-2.17.so        [.] malloc_consolidate
> 
> Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Jiri Olsa <jolsa@redhat.com>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Link: http://lkml.kernel.org/n/tip-d9tcfow6stbrp4btvgs51y67@git.kernel.org
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Have you tested this patchset when collapsing is not used?
There are fair chances that this patchset does not only improve collapsing
but also callchain insertion in general. So it's probably a win in any case. But
still it would be nice to make sure that it's the case because we are getting
rid of collapsing anyway.

The test that could tell us about that is to run "perf report -s sym" and compare the
time it takes to complete before and after this patch, because "-s sym" shouldn't
involve collapses.

Sorting by anything that is not comm should do the trick in fact.

Thanks.

Re: [PATCH 8/8] perf tools: Get current comm instead of last one

[Namhyung Kim]

Hi Frederic,

On Wed, 2 Oct 2013 12:01:51 +0200, Frederic Weisbecker wrote:
> On Thu, Sep 26, 2013 at 05:58:10PM +0900, Namhyung Kim wrote:
>> From: Namhyung Kim <namhyung.kim@lge.com>
>> 
>> At insert time, a hist entry should reference comm at the time
>> otherwise it'll get the last comm anyway.

[SNIP]

>> --- a/tools/perf/util/comm.h
>> +++ b/tools/perf/util/comm.h
>> @@ -16,5 +16,6 @@ struct comm {
>>  void comm__free(struct comm *comm);
>>  struct comm *comm__new(const char *str, u64 timestamp);
>>  const char *comm__str(const struct comm *comm);
>> +void comm__override(struct comm *self, const char *str, u64 timestamp);
>>  
>>  #endif  /* __PERF_COMM_H */
>> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
>> index a2d58e7abdcf..1aa97b012a09 100644
>> --- a/tools/perf/util/hist.c
>> +++ b/tools/perf/util/hist.c
>> @@ -412,6 +412,7 @@ struct hist_entry *__hists__add_mem_entry(struct hists *self,
>>  {
>>  	struct hist_entry entry = {
>>  		.thread	= al->thread,
>> +		.comm = curr_comm(al->thread),
>
> So now that we have an accessor for that, it clashes a bit with existing names.
>
> I suggest we rename thread__comm_curr() -> thread__comm_str()
> And rename curr_comm() -> thread__comm()
>
> Hmm?

Okay, I'll rename and resend.

Thanks,
Namhyung

Re: [PATCH 1/8] perf callchain: Convert children list to rbtree

[Namhyung Kim]

On Wed, 2 Oct 2013 12:18:28 +0200, Frederic Weisbecker wrote:
> On Thu, Sep 26, 2013 at 05:58:03PM +0900, Namhyung Kim wrote:
>> From: Namhyung Kim <namhyung.kim@lge.com>
>> 
>> Current collapse stage has a scalability problem which can be
>> reproduced easily with parallel kernel build.  This is because it
>> needs to traverse every children of callchain linearly during the
>> collapse/merge stage.  Convert it to rbtree reduced the overhead
>> significantly.
>> 
>> On my 400MB perf.data file which recorded with make -j32 kernel build:
>> 
>>   $ time perf --no-pager report --stdio > /dev/null
>> 
>> before:
>>   real	6m22.073s
>>   user	6m18.683s
>>   sys	0m0.706s
>> 
>> after:
>>   real	0m20.780s
>>   user	0m19.962s
>>   sys	0m0.689s
>> 
>> During the perf report the overhead on append_chain_children went down
>> from 96.69% to 18.16%:
>> 
>>   -  18.16%  perf  perf                [.] append_chain_children
>>      - append_chain_children
>>         - 77.48% append_chain_children
>>            + 69.79% merge_chain_branch
>>            - 22.96% append_chain_children
>>               + 67.44% merge_chain_branch
>>               + 30.15% append_chain_children
>>               + 2.41% callchain_append
>>            + 7.25% callchain_append
>>         + 12.26% callchain_append
>>         + 10.22% merge_chain_branch
>>   +  11.58%  perf  perf                [.] dso__find_symbol
>>   +   8.02%  perf  perf                [.] sort__comm_cmp
>>   +   5.48%  perf  libc-2.17.so        [.] malloc_consolidate
>> 
>> Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
>> Cc: Jiri Olsa <jolsa@redhat.com>
>> Cc: Frederic Weisbecker <fweisbec@gmail.com>
>> Link: http://lkml.kernel.org/n/tip-d9tcfow6stbrp4btvgs51y67@git.kernel.org
>> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
>
> Have you tested this patchset when collapsing is not used?
> There are fair chances that this patchset does not only improve collapsing
> but also callchain insertion in general. So it's probably a win in any case. But
> still it would be nice to make sure that it's the case because we are getting
> rid of collapsing anyway.
>
> The test that could tell us about that is to run "perf report -s sym" and compare the
> time it takes to complete before and after this patch, because "-s sym" shouldn't
> involve collapses.
>
> Sorting by anything that is not comm should do the trick in fact.

Yes, I have similar result when collapsing is not used.  Actually when I
ran "perf report -s sym", the performance improves higher since it'd
insert more callchains in a hist entry.

Thanks,
Namhyung

Re: [PATCH 1/8] perf callchain: Convert children list to rbtree

[Frederic Weisbecker]

On Tue, Oct 08, 2013 at 11:03:16AM +0900, Namhyung Kim wrote:
> On Wed, 2 Oct 2013 12:18:28 +0200, Frederic Weisbecker wrote:
> > On Thu, Sep 26, 2013 at 05:58:03PM +0900, Namhyung Kim wrote:
> >> From: Namhyung Kim <namhyung.kim@lge.com>
> >> 
> >> Current collapse stage has a scalability problem which can be
> >> reproduced easily with parallel kernel build.  This is because it
> >> needs to traverse every children of callchain linearly during the
> >> collapse/merge stage.  Convert it to rbtree reduced the overhead
> >> significantly.
> >> 
> >> On my 400MB perf.data file which recorded with make -j32 kernel build:
> >> 
> >>   $ time perf --no-pager report --stdio > /dev/null
> >> 
> >> before:
> >>   real	6m22.073s
> >>   user	6m18.683s
> >>   sys	0m0.706s
> >> 
> >> after:
> >>   real	0m20.780s
> >>   user	0m19.962s
> >>   sys	0m0.689s
> >> 
> >> During the perf report the overhead on append_chain_children went down
> >> from 96.69% to 18.16%:
> >> 
> >>   -  18.16%  perf  perf                [.] append_chain_children
> >>      - append_chain_children
> >>         - 77.48% append_chain_children
> >>            + 69.79% merge_chain_branch
> >>            - 22.96% append_chain_children
> >>               + 67.44% merge_chain_branch
> >>               + 30.15% append_chain_children
> >>               + 2.41% callchain_append
> >>            + 7.25% callchain_append
> >>         + 12.26% callchain_append
> >>         + 10.22% merge_chain_branch
> >>   +  11.58%  perf  perf                [.] dso__find_symbol
> >>   +   8.02%  perf  perf                [.] sort__comm_cmp
> >>   +   5.48%  perf  libc-2.17.so        [.] malloc_consolidate
> >> 
> >> Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
> >> Cc: Jiri Olsa <jolsa@redhat.com>
> >> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> >> Link: http://lkml.kernel.org/n/tip-d9tcfow6stbrp4btvgs51y67@git.kernel.org
> >> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> >
> > Have you tested this patchset when collapsing is not used?
> > There are fair chances that this patchset does not only improve collapsing
> > but also callchain insertion in general. So it's probably a win in any case. But
> > still it would be nice to make sure that it's the case because we are getting
> > rid of collapsing anyway.
> >
> > The test that could tell us about that is to run "perf report -s sym" and compare the
> > time it takes to complete before and after this patch, because "-s sym" shouldn't
> > involve collapses.
> >
> > Sorting by anything that is not comm should do the trick in fact.
> 
> Yes, I have similar result when collapsing is not used.  Actually when I
> ran "perf report -s sym", the performance improves higher since it'd
> insert more callchains in a hist entry.

Great! I'll have a closer look and review on the callchain patches then. Please
resend these along the comm batch.

Thanks again!

Re: [PATCH 1/8] perf callchain: Convert children list to rbtree

[Namhyung Kim]

Hi Frederic,

On Tue, 8 Oct 2013 21:22:45 +0200, Frederic Weisbecker wrote:
> On Tue, Oct 08, 2013 at 11:03:16AM +0900, Namhyung Kim wrote:
>> On Wed, 2 Oct 2013 12:18:28 +0200, Frederic Weisbecker wrote:
>> > Have you tested this patchset when collapsing is not used?
>> > There are fair chances that this patchset does not only improve collapsing
>> > but also callchain insertion in general. So it's probably a win in any case. But
>> > still it would be nice to make sure that it's the case because we are getting
>> > rid of collapsing anyway.
>> >
>> > The test that could tell us about that is to run "perf report -s sym" and compare the
>> > time it takes to complete before and after this patch, because "-s sym" shouldn't
>> > involve collapses.
>> >
>> > Sorting by anything that is not comm should do the trick in fact.
>> 
>> Yes, I have similar result when collapsing is not used.  Actually when I
>> ran "perf report -s sym", the performance improves higher since it'd
>> insert more callchains in a hist entry.
>
> Great! I'll have a closer look and review on the callchain patches then. Please
> resend these along the comm batch.

I'll do it later today or tomorrow.

Thanks,
Namhyung

[PATCH 6/8] perf tools: Add new comm infrastructure

[Namhyung Kim]

From: Frederic Weisbecker <fweisbec@gmail.com>

This new comm infrastructure provides two features:

1) It keeps track of all comms lifecycle for a given thread. This
way we can associate a timeframe to any thread comm, as long as
PERF_SAMPLE_TIME samples are joined to comm and fork events.

As a result we should have more precise comm sorted hists with
seperated entries for pre and post exec time after a fork.

2) It also makes sure that a given comm string is not duplicated
but rather shared among the threads that refer to it. This way the
threads comm can be compared against pointer values from the sort
infrastructure.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/n/tip-hwjf70b2wve9m2kosxiq8bb3@git.kernel.org
[ Rename some accessor functions ]
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
[ Fixed up minor const pointer issues and removed 'self' usage ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Makefile      |   2 +
 tools/perf/util/comm.c   | 106 +++++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/comm.h   |  20 +++++++++
 tools/perf/util/thread.c | 100 +++++++++++++++++++++++++++++++-------------
 tools/perf/util/thread.h |   3 +-
 5 files changed, 202 insertions(+), 29 deletions(-)
 create mode 100644 tools/perf/util/comm.c
 create mode 100644 tools/perf/util/comm.h

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index b62e12d2ba6b..42001f387e7f 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -272,6 +272,7 @@ LIB_H += util/color.h
 LIB_H += util/values.h
 LIB_H += util/sort.h
 LIB_H += util/hist.h
+LIB_H += util/comm.h
 LIB_H += util/thread.h
 LIB_H += util/thread_map.h
 LIB_H += util/trace-event.h
@@ -339,6 +340,7 @@ LIB_OBJS += $(OUTPUT)util/machine.o
 LIB_OBJS += $(OUTPUT)util/map.o
 LIB_OBJS += $(OUTPUT)util/pstack.o
 LIB_OBJS += $(OUTPUT)util/session.o
+LIB_OBJS += $(OUTPUT)util/comm.o
 LIB_OBJS += $(OUTPUT)util/thread.o
 LIB_OBJS += $(OUTPUT)util/thread_map.o
 LIB_OBJS += $(OUTPUT)util/trace-event-parse.o
diff --git a/tools/perf/util/comm.c b/tools/perf/util/comm.c
new file mode 100644
index 000000000000..0cfb55202537
--- /dev/null
+++ b/tools/perf/util/comm.c
@@ -0,0 +1,106 @@
+#include "comm.h"
+#include "util.h"
+#include <stdlib.h>
+#include <stdio.h>
+
+struct comm_str {
+	char *str;
+	struct rb_node rb_node;
+	int ref;
+};
+
+/* Should perhaps be moved to struct machine */
+static struct rb_root comm_str_root;
+
+static void comm_str_get(struct comm_str *cs)
+{
+	cs->ref++;
+}
+
+static void comm_str_put(struct comm_str *cs)
+{
+	if (!--cs->ref) {
+		rb_erase(&cs->rb_node, &comm_str_root);
+		free(cs->str);
+		free(cs);
+	}
+}
+
+static struct comm_str *comm_str_alloc(const char *str)
+{
+	struct comm_str *cs;
+
+	cs = zalloc(sizeof(*cs));
+	if (!cs)
+		return NULL;
+
+	cs->str = strdup(str);
+	if (!cs->str) {
+		free(cs);
+		return NULL;
+	}
+
+	return cs;
+}
+
+static struct comm_str *comm_str_findnew(const char *str, struct rb_root *root)
+{
+	struct rb_node **p = &root->rb_node;
+	struct rb_node *parent = NULL;
+	struct comm_str *iter, *new;
+	int cmp;
+
+	while (*p != NULL) {
+		parent = *p;
+		iter = rb_entry(parent, struct comm_str, rb_node);
+
+		cmp = strcmp(str, iter->str);
+		if (!cmp)
+			return iter;
+
+		if (cmp < 0)
+			p = &(*p)->rb_left;
+		else
+			p = &(*p)->rb_right;
+	}
+
+	new = comm_str_alloc(str);
+	if (!new)
+		return NULL;
+
+	rb_link_node(&new->rb_node, parent, p);
+	rb_insert_color(&new->rb_node, root);
+
+	return new;
+}
+
+struct comm *comm__new(const char *str, u64 timestamp)
+{
+	struct comm *comm = zalloc(sizeof(*comm));
+
+	if (!comm)
+		return NULL;
+
+	comm->start = timestamp;
+
+	comm->comm_str = comm_str_findnew(str, &comm_str_root);
+	if (!comm->comm_str) {
+		free(comm);
+		return NULL;
+	}
+
+	comm_str_get(comm->comm_str);
+
+	return comm;
+}
+
+void comm__free(struct comm *comm)
+{
+	comm_str_put(comm->comm_str);
+	free(comm);
+}
+
+const char *comm__str(const struct comm *comm)
+{
+	return comm->comm_str->str;
+}
diff --git a/tools/perf/util/comm.h b/tools/perf/util/comm.h
new file mode 100644
index 000000000000..f62d215bede2
--- /dev/null
+++ b/tools/perf/util/comm.h
@@ -0,0 +1,20 @@
+#ifndef __PERF_COMM_H
+#define __PERF_COMM_H
+
+#include "../perf.h"
+#include <linux/rbtree.h>
+#include <linux/list.h>
+
+struct comm_str;
+
+struct comm {
+	struct comm_str *comm_str;
+	u64 start;
+	struct list_head list;
+};
+
+void comm__free(struct comm *comm);
+struct comm *comm__new(const char *str, u64 timestamp);
+const char *comm__str(const struct comm *comm);
+
+#endif  /* __PERF_COMM_H */
diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
index 205b6368f175..095b725d0cd4 100644
--- a/tools/perf/util/thread.c
+++ b/tools/perf/util/thread.c
@@ -6,9 +6,12 @@
 #include "thread.h"
 #include "util.h"
 #include "debug.h"
+#include "comm.h"
 
 struct thread *thread__new(pid_t pid, pid_t tid)
 {
+	char *comm_str;
+	struct comm *comm;
 	struct thread *self = zalloc(sizeof(*self));
 
 	if (self != NULL) {
@@ -16,47 +19,88 @@ struct thread *thread__new(pid_t pid, pid_t tid)
 		self->pid_ = pid;
 		self->tid = tid;
 		self->ppid = -1;
-		self->comm = malloc(32);
-		if (self->comm)
-			snprintf(self->comm, 32, ":%d", self->tid);
+		INIT_LIST_HEAD(&self->comm_list);
+
+		comm_str = malloc(32);
+		if (!comm_str)
+			goto err_thread;
+
+		snprintf(comm_str, 32, ":%d", tid);
+		comm = comm__new(comm_str, 0);
+		free(comm_str);
+		if (!comm)
+			goto err_thread;
+
+		list_add(&comm->list, &self->comm_list);
 	}
 
 	return self;
+
+err_thread:
+	free(self);
+	return NULL;
 }
 
 void thread__delete(struct thread *self)
 {
+	struct comm *comm, *tmp;
+
 	map_groups__exit(&self->mg);
-	free(self->comm);
+	list_for_each_entry_safe(comm, tmp, &self->comm_list, list) {
+		list_del(&comm->list);
+		comm__free(comm);
+	}
+
 	free(self);
 }
 
-int thread__set_comm(struct thread *self, const char *comm,
-		     u64 timestamp __maybe_unused)
+static struct comm *thread__comm(const struct thread *thread)
 {
-	int err;
-
-	if (self->comm)
-		free(self->comm);
-	self->comm = strdup(comm);
-	err = self->comm == NULL ? -ENOMEM : 0;
-	if (!err) {
-		self->comm_set = true;
+	if (list_empty(&thread->comm_list))
+		return NULL;
+
+	return list_first_entry(&thread->comm_list, struct comm, list);
+}
+
+/* CHECKME: time should always be 0 if event aren't ordered */
+int thread__set_comm(struct thread *thread, const char *str, u64 timestamp)
+{
+	struct comm *new, *curr = thread__comm(thread);
+
+	/* Override latest entry if it had no specific time coverage */
+	if (!curr->start) {
+		list_del(&curr->list);
+		comm__free(curr);
 	}
-	return err;
+
+	new = comm__new(str, timestamp);
+	if (!new)
+		return -ENOMEM;
+
+	list_add(&new->list, &thread->comm_list);
+	thread->comm_set = true;
+
+	return 0;
 }
 
 const char *thread__comm_str(const struct thread *thread)
 {
-	return thread->comm;
+	const struct comm *comm = thread__comm(thread);
+
+	if (!comm)
+		return NULL;
+
+	return comm__str(comm);
 }
 
+/* CHECKME: it should probably better return the max comm len from its comm list */
 int thread__comm_len(struct thread *self)
 {
 	if (!self->comm_len) {
-		if (!self->comm)
+		const char *comm = thread__comm_str(self);
+		if (!comm)
 			return 0;
-		self->comm_len = strlen(self->comm);
+		self->comm_len = strlen(comm);
 	}
 
 	return self->comm_len;
@@ -74,25 +118,25 @@ void thread__insert_map(struct thread *self, struct map *map)
 	map_groups__insert(&self->mg, map);
 }
 
-int thread__fork(struct thread *self, struct thread *parent,
-		 u64 timestamp __maybe_unused)
+int thread__fork(struct thread *thread, struct thread *parent, u64 timestamp)
 {
-	int i;
+	int i, err;
 
 	if (parent->comm_set) {
-		if (self->comm)
-			free(self->comm);
-		self->comm = strdup(parent->comm);
-		if (!self->comm)
+		const char *comm = thread__comm_str(parent);
+		if (!comm)
 			return -ENOMEM;
-		self->comm_set = true;
+		err = thread__set_comm(thread, comm, timestamp);
+		if (!err)
+			return err;
+		thread->comm_set = true;
 	}
 
 	for (i = 0; i < MAP__NR_TYPES; ++i)
-		if (map_groups__clone(&self->mg, &parent->mg, i) < 0)
+		if (map_groups__clone(&thread->mg, &parent->mg, i) < 0)
 			return -ENOMEM;
 
-	self->ppid = parent->tid;
+	thread->ppid = parent->tid;
 
 	return 0;
 }
diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
index b7cdf64c2304..307d2ec60123 100644
--- a/tools/perf/util/thread.h
+++ b/tools/perf/util/thread.h
@@ -2,6 +2,7 @@
 #define __PERF_THREAD_H
 
 #include <linux/rbtree.h>
+#include <linux/list.h>
 #include <unistd.h>
 #include <sys/types.h>
 #include "symbol.h"
@@ -18,7 +19,7 @@ struct thread {
 	char			shortname[3];
 	bool			comm_set;
 	bool			dead; /* if set thread has exited */
-	char			*comm;
+	struct list_head	comm_list;
 	int			comm_len;
 
 	void			*priv;
-- 
1.7.11.7

Re: [PATCH 6/8] perf tools: Add new comm infrastructure

[Frederic Weisbecker]

2013/10/11 Namhyung Kim <namhyung@kernel.org>:
> From: Frederic Weisbecker <fweisbec@gmail.com>
>
> This new comm infrastructure provides two features:
>
> 1) It keeps track of all comms lifecycle for a given thread. This
> way we can associate a timeframe to any thread comm, as long as
> PERF_SAMPLE_TIME samples are joined to comm and fork events.
>
> As a result we should have more precise comm sorted hists with
> seperated entries for pre and post exec time after a fork.
>
> 2) It also makes sure that a given comm string is not duplicated
> but rather shared among the threads that refer to it. This way the
> threads comm can be compared against pointer values from the sort
> infrastructure.
>
> Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Jiri Olsa <jolsa@redhat.com>
> Cc: David Ahern <dsahern@gmail.com>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Stephane Eranian <eranian@google.com>
> Link: http://lkml.kernel.org/n/tip-hwjf70b2wve9m2kosxiq8bb3@git.kernel.org
> [ Rename some accessor functions ]
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> [ Fixed up minor const pointer issues and removed 'self' usage ]
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Hi,

I was wondering about the fate of these patches. I can resend these or
do any rebase if you need to.

Thanks.

Re: [PATCH 6/8] perf tools: Add new comm infrastructure

[Arnaldo Carvalho de Melo]

Em Fri, Oct 25, 2013 at 11:56:31AM +0100, Frederic Weisbecker escreveu:
> 2013/10/11 Namhyung Kim <namhyung@kernel.org>:
> > From: Frederic Weisbecker <fweisbec@gmail.com>

> > This new comm infrastructure provides two features:

> I was wondering about the fate of these patches. I can resend these or
> do any rebase if you need to.

So I applied the first few patches, but deferred working on this part,
the comm one, because Jiri Olsa had some problems with it, IIRC, Jiri?
Can you refresh my mind and ellaborate on it?

Thanks!

- Arnaldo

Re: [PATCH 6/8] perf tools: Add new comm infrastructure

[David Ahern]

On 10/25/13 7:04 AM, Arnaldo Carvalho de Melo wrote:
> Em Fri, Oct 25, 2013 at 11:56:31AM +0100, Frederic Weisbecker escreveu:
>> 2013/10/11 Namhyung Kim <namhyung@kernel.org>:
>>> From: Frederic Weisbecker <fweisbec@gmail.com>
>
>>> This new comm infrastructure provides two features:
>
>> I was wondering about the fate of these patches. I can resend these or
>> do any rebase if you need to.
>
> So I applied the first few patches, but deferred working on this part,
> the comm one, because Jiri Olsa had some problems with it, IIRC, Jiri?
> Can you refresh my mind and ellaborate on it?

I also saw some comm related problems with Namhyung's tree. Not sure if 
it contained all of Frederic's patches. I have not had time to chase 
down what is going on -- just that comm's coming out of perf-script were 
not correct.

David

Re: [PATCH 6/8] perf tools: Add new comm infrastructure

[Frederic Weisbecker]

2013/10/25 David Ahern <dsahern@gmail.com>:
> On 10/25/13 7:04 AM, Arnaldo Carvalho de Melo wrote:
>>
>> Em Fri, Oct 25, 2013 at 11:56:31AM +0100, Frederic Weisbecker escreveu:
>>>
>>> 2013/10/11 Namhyung Kim <namhyung@kernel.org>:
>>>>
>>>> From: Frederic Weisbecker <fweisbec@gmail.com>
>>
>>
>>>> This new comm infrastructure provides two features:
>>
>>
>>> I was wondering about the fate of these patches. I can resend these or
>>> do any rebase if you need to.
>>
>>
>> So I applied the first few patches, but deferred working on this part,
>> the comm one, because Jiri Olsa had some problems with it, IIRC, Jiri?
>> Can you refresh my mind and ellaborate on it?
>
>
> I also saw some comm related problems with Namhyung's tree. Not sure if it
> contained all of Frederic's patches. I have not had time to chase down what
> is going on -- just that comm's coming out of perf-script were not correct.

Oh I see. It's possible that my massive conversion to use the comm
accessor got blind at some point and left over a few things. I
remember that I only lightly tested that new comm infrastructure. I
mean I tested a lot of "perf report -s foo,bar" combinations for
performance comparisons but I haven't tested the perf script and all
the other perf tools.

I'll rebase these patches and test that wider before resending.

Thanks!

Re: [PATCH 6/8] perf tools: Add new comm infrastructure

[Arnaldo Carvalho de Melo]

Em Fri, Oct 25, 2013 at 07:12:36PM +0100, Frederic Weisbecker escreveu:
> Oh I see. It's possible that my massive conversion to use the comm
> accessor got blind at some point and left over a few things. I
> remember that I only lightly tested that new comm infrastructure. I
> mean I tested a lot of "perf report -s foo,bar" combinations for
> performance comparisons but I haven't tested the perf script and all
> the other perf tools.
> 
> I'll rebase these patches and test that wider before resending.

Thanks for doing that!

- Arnaldo

Re: [PATCH 6/8] perf tools: Add new comm infrastructure

[David Ahern]

On 10/25/13 12:12 PM, Frederic Weisbecker wrote:
> Oh I see. It's possible that my massive conversion to use the comm
> accessor got blind at some point and left over a few things. I
> remember that I only lightly tested that new comm infrastructure. I
> mean I tested a lot of "perf report -s foo,bar" combinations for
> performance comparisons but I haven't tested the perf script and all
> the other perf tools.
>
> I'll rebase these patches and test that wider before resending.

specifically, I see stuff like perf forking ls and comm still shows as 
perf even though there is COMM record with the rename to ls. I believe 
the test case was something like:

perf sched record -- ls
perf script

David

Re: [PATCH 6/8] perf tools: Add new comm infrastructure

[Namhyung Kim]

Hi David,

On Fri, 25 Oct 2013 12:19:07 -0600, David Ahern wrote:
> On 10/25/13 12:12 PM, Frederic Weisbecker wrote:
>> Oh I see. It's possible that my massive conversion to use the comm
>> accessor got blind at some point and left over a few things. I
>> remember that I only lightly tested that new comm infrastructure. I
>> mean I tested a lot of "perf report -s foo,bar" combinations for
>> performance comparisons but I haven't tested the perf script and all
>> the other perf tools.
>>
>> I'll rebase these patches and test that wider before resending.
>
> specifically, I see stuff like perf forking ls and comm still shows as
> perf even though there is COMM record with the rename to ls. I believe
> the test case was something like:
>
> perf sched record -- ls
> perf script

Hmm.. did you try my latest v5 patchset?  I couldn't reproduce the case
at least for the command lines above.

  $ perf script
            perf 24810 [007] 1546517.815809: sched:sched_stat_runtime: comm=perf pid=24810 ru
            perf 24810 [007] 1546517.815909: sched:sched_wakeup: comm=perf pid=24811 prio=120
         swapper     0 [008] 1546517.815913: sched:sched_switch: prev_comm=swapper/8 prev_pid
            perf 24810 [007] 1546517.815953: sched:sched_stat_runtime: comm=perf pid=24810 ru
            perf 24810 [007] 1546517.815957: sched:sched_switch: prev_comm=perf prev_pid=2481
            perf 24811 [008] 1546517.815992: sched:sched_wakeup: comm=migration/8 pid=48 prio
            perf 24811 [008] 1546517.815993: sched:sched_stat_runtime: comm=perf pid=24811 ru
            perf 24811 [008] 1546517.815993: sched:sched_switch: prev_comm=perf prev_pid=2481
     migration/8    48 [008] 1546517.815996: sched:sched_migrate_task: comm=perf pid=24811 pr
     migration/8    48 [008] 1546517.816000: sched:sched_switch: prev_comm=migration/8 prev_p
         swapper     0 [009] 1546517.816002: sched:sched_switch: prev_comm=swapper/9 prev_pid
              ls 24811 [009] 1546517.816808: sched:sched_stat_runtime: comm=ls pid=24811 runt


Here, the process 24811 has only 3 samples before COMM event

  $ perf report -D | grep 24811 | grep -v MMAP | head
  0 0 0x629b0 [0x38]: PERF_RECORD_COMM: perf:24811
  8 1546517815992058 0x65f50 [0x68]: PERF_RECORD_SAMPLE(IP, 1): 24811/24811: 0xffffffff81091512 period: 1 addr: 0
   ... thread: perf:24811
  8 1546517815993189 0x65fb8 [0x70]: PERF_RECORD_SAMPLE(IP, 1): 24811/24811: 0xffffffff81099d25 period: 83314 addr: 0
   ... thread: perf:24811
  8 1546517815993975 0x66028 [0x80]: PERF_RECORD_SAMPLE(IP, 1): 24811/24811: 0xffffffff81659d60 period: 1 addr: 0
   ... thread: perf:24811
  9 1546517816224342 0x66378 [0x38]: PERF_RECORD_COMM: ls:24811
  9 1546517816808637 0x667f0 [0x70]: PERF_RECORD_SAMPLE(IP, 1): 24811/24811: 0xffffffff81099d25 period: 810663 addr: 0
   ... thread: ls:24811

  $ perf version
  perf version 3.11.ge9eb20


Thanks,
Namhyung

Re: [PATCH 6/8] perf tools: Add new comm infrastructure

[Frederic Weisbecker]

2013/10/28 Namhyung Kim <namhyung@kernel.org>:
> Hi David,
>
> On Fri, 25 Oct 2013 12:19:07 -0600, David Ahern wrote:
>> On 10/25/13 12:12 PM, Frederic Weisbecker wrote:
>>> Oh I see. It's possible that my massive conversion to use the comm
>>> accessor got blind at some point and left over a few things. I
>>> remember that I only lightly tested that new comm infrastructure. I
>>> mean I tested a lot of "perf report -s foo,bar" combinations for
>>> performance comparisons but I haven't tested the perf script and all
>>> the other perf tools.
>>>
>>> I'll rebase these patches and test that wider before resending.
>>
>> specifically, I see stuff like perf forking ls and comm still shows as
>> perf even though there is COMM record with the rename to ls. I believe
>> the test case was something like:
>>
>> perf sched record -- ls
>> perf script
>
> Hmm.. did you try my latest v5 patchset?  I couldn't reproduce the case
> at least for the command lines above.
>
>   $ perf script
>             perf 24810 [007] 1546517.815809: sched:sched_stat_runtime: comm=perf pid=24810 ru
>             perf 24810 [007] 1546517.815909: sched:sched_wakeup: comm=perf pid=24811 prio=120
>          swapper     0 [008] 1546517.815913: sched:sched_switch: prev_comm=swapper/8 prev_pid
>             perf 24810 [007] 1546517.815953: sched:sched_stat_runtime: comm=perf pid=24810 ru
>             perf 24810 [007] 1546517.815957: sched:sched_switch: prev_comm=perf prev_pid=2481
>             perf 24811 [008] 1546517.815992: sched:sched_wakeup: comm=migration/8 pid=48 prio
>             perf 24811 [008] 1546517.815993: sched:sched_stat_runtime: comm=perf pid=24811 ru
>             perf 24811 [008] 1546517.815993: sched:sched_switch: prev_comm=perf prev_pid=2481
>      migration/8    48 [008] 1546517.815996: sched:sched_migrate_task: comm=perf pid=24811 pr
>      migration/8    48 [008] 1546517.816000: sched:sched_switch: prev_comm=migration/8 prev_p
>          swapper     0 [009] 1546517.816002: sched:sched_switch: prev_comm=swapper/9 prev_pid
>               ls 24811 [009] 1546517.816808: sched:sched_stat_runtime: comm=ls pid=24811 runt
>
>
> Here, the process 24811 has only 3 samples before COMM event
>
>   $ perf report -D | grep 24811 | grep -v MMAP | head
>   0 0 0x629b0 [0x38]: PERF_RECORD_COMM: perf:24811
>   8 1546517815992058 0x65f50 [0x68]: PERF_RECORD_SAMPLE(IP, 1): 24811/24811: 0xffffffff81091512 period: 1 addr: 0
>    ... thread: perf:24811
>   8 1546517815993189 0x65fb8 [0x70]: PERF_RECORD_SAMPLE(IP, 1): 24811/24811: 0xffffffff81099d25 period: 83314 addr: 0
>    ... thread: perf:24811
>   8 1546517815993975 0x66028 [0x80]: PERF_RECORD_SAMPLE(IP, 1): 24811/24811: 0xffffffff81659d60 period: 1 addr: 0
>    ... thread: perf:24811
>   9 1546517816224342 0x66378 [0x38]: PERF_RECORD_COMM: ls:24811
>   9 1546517816808637 0x667f0 [0x70]: PERF_RECORD_SAMPLE(IP, 1): 24811/24811: 0xffffffff81099d25 period: 810663 addr: 0
>    ... thread: ls:24811
>
>   $ perf version
>   perf version 3.11.ge9eb20
>

Ah cool! Could you please remind me the name of that branch so that I
can do some tests and work on top of it?

Thanks.

Re: [PATCH 6/8] perf tools: Add new comm infrastructure

[Namhyung Kim]

Hi Frederic,

On Mon, 28 Oct 2013 10:09:46 +0100, Frederic Weisbecker wrote:
> Ah cool! Could you please remind me the name of that branch so that I
> can do some tests and work on top of it?

  git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
    perf/callchain-v5

Thanks,
Namhyung

Re: [PATCH 6/8] perf tools: Add new comm infrastructure

[Frederic Weisbecker]

On Mon, Oct 28, 2013 at 06:15:26PM +0900, Namhyung Kim wrote:
> Hi Frederic,
> 
> On Mon, 28 Oct 2013 10:09:46 +0100, Frederic Weisbecker wrote:
> > Ah cool! Could you please remind me the name of that branch so that I
> > can do some tests and work on top of it?
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
>     perf/callchain-v5

Hmm, I tried to rebase on top of tip:perf/core and git is getting confused I guess with
which is applied and which is out of tree. There have been a lot of changes since
then and thus quite some conflicts.

If you don't mind, I would love if you rebase against latest tip:perf/core as you
know better than me what has been applied and what hasn't.

Thanks!

Re: [PATCH 6/8] perf tools: Add new comm infrastructure

[Arnaldo Carvalho de Melo]

Em Mon, Oct 28, 2013 at 11:12:50AM +0100, Frederic Weisbecker escreveu:
> On Mon, Oct 28, 2013 at 06:15:26PM +0900, Namhyung Kim wrote:
> > Hi Frederic,
> > 
> > On Mon, 28 Oct 2013 10:09:46 +0100, Frederic Weisbecker wrote:
> > > Ah cool! Could you please remind me the name of that branch so that I
> > > can do some tests and work on top of it?
> > 
> >   git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
> >     perf/callchain-v5
> 
> Hmm, I tried to rebase on top of tip:perf/core and git is getting confused I guess with
> which is applied and which is out of tree. There have been a lot of changes since
> then and thus quite some conflicts.
> 
> If you don't mind, I would love if you rebase against latest tip:perf/core as you
> know better than me what has been applied and what hasn't.

I'll try to do it now.

- Arnaldo

Re: [PATCH 6/8] perf tools: Add new comm infrastructure

[Arnaldo Carvalho de Melo]

Em Mon, Oct 28, 2013 at 09:43:09AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Mon, Oct 28, 2013 at 11:12:50AM +0100, Frederic Weisbecker escreveu:
> > On Mon, Oct 28, 2013 at 06:15:26PM +0900, Namhyung Kim wrote:
> > > On Mon, 28 Oct 2013 10:09:46 +0100, Frederic Weisbecker wrote:
> > > > Ah cool! Could you please remind me the name of that branch so that I
> > > > can do some tests and work on top of it?

> > >   git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
> > >     perf/callchain-v5

> > Hmm, I tried to rebase on top of tip:perf/core and git is getting confused I guess with
> > which is applied and which is out of tree. There have been a lot of changes since
> > then and thus quite some conflicts.
> > 
> > If you don't mind, I would love if you rebase against latest tip:perf/core as you
> > know better than me what has been applied and what hasn't.
 
> I'll try to do it now.

Can you please try with:

	git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git
	  perf/comm

And share your results? I fixed up everything wrt recent flux in my
perf/core branch, did just basic testing here, but so far it looks ok
(and fast!).

- Arnaldo

Re: [PATCH 6/8] perf tools: Add new comm infrastructure

[Frederic Weisbecker]

On Mon, Oct 28, 2013 at 11:29:12AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Mon, Oct 28, 2013 at 09:43:09AM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Mon, Oct 28, 2013 at 11:12:50AM +0100, Frederic Weisbecker escreveu:
> > > On Mon, Oct 28, 2013 at 06:15:26PM +0900, Namhyung Kim wrote:
> > > > On Mon, 28 Oct 2013 10:09:46 +0100, Frederic Weisbecker wrote:
> > > > > Ah cool! Could you please remind me the name of that branch so that I
> > > > > can do some tests and work on top of it?
> 
> > > >   git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
> > > >     perf/callchain-v5
> 
> > > Hmm, I tried to rebase on top of tip:perf/core and git is getting confused I guess with
> > > which is applied and which is out of tree. There have been a lot of changes since
> > > then and thus quite some conflicts.
> > > 
> > > If you don't mind, I would love if you rebase against latest tip:perf/core as you
> > > know better than me what has been applied and what hasn't.
>  
> > I'll try to do it now.
> 
> Can you please try with:
> 
> 	git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git
> 	  perf/comm
> 
> And share your results? I fixed up everything wrt recent flux in my
> perf/core branch, did just basic testing here, but so far it looks ok
> (and fast!).

Ok I tried perf report, perf script and perf top and it looks good.
In fact it looks better than the tip tree. I fixes some bugs that I can
see in tip:/perf/core:

     $ perf record perf bench sched messaging
     $ perf report --stdio -s comm
     write failure on standard output: Bad file descriptor

Not sure where that comes from, but it's fixed in your tree. May be that's
on the fixes before the comm patches in your tree.

Also it differentiate between pre-exec and post-fork events, which looks
more precise (and it fixes some comm mangling as well):


Before:

     # Overhead          Command
     # ........  ...............
     #
       100.00%  sched-me

After:

     # Overhead          Command
     # ........  ...............
     #
        99.89%  sched-messaging
        0.11%             perf


Thanks!

Re: [PATCH 6/8] perf tools: Add new comm infrastructure

[Arnaldo Carvalho de Melo]

Em Mon, Oct 28, 2013 at 05:05:30PM +0100, Frederic Weisbecker escreveu:
> On Mon, Oct 28, 2013 at 11:29:12AM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Mon, Oct 28, 2013 at 09:43:09AM -0300, Arnaldo Carvalho de Melo escreveu:
> > > Em Mon, Oct 28, 2013 at 11:12:50AM +0100, Frederic Weisbecker escreveu:
> > > > On Mon, Oct 28, 2013 at 06:15:26PM +0900, Namhyung Kim wrote:
> > > > > On Mon, 28 Oct 2013 10:09:46 +0100, Frederic Weisbecker wrote:
> > > > > > Ah cool! Could you please remind me the name of that branch so that I
> > > > > > can do some tests and work on top of it?
> > 
> > > > >   git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
> > > > >     perf/callchain-v5
> > 
> > > > Hmm, I tried to rebase on top of tip:perf/core and git is getting confused I guess with
> > > > which is applied and which is out of tree. There have been a lot of changes since
> > > > then and thus quite some conflicts.
> > > > 
> > > > If you don't mind, I would love if you rebase against latest tip:perf/core as you
> > > > know better than me what has been applied and what hasn't.
> >  
> > > I'll try to do it now.
> > 
> > Can you please try with:
> > 
> > 	git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git
> > 	  perf/comm
> > 
> > And share your results? I fixed up everything wrt recent flux in my
> > perf/core branch, did just basic testing here, but so far it looks ok
> > (and fast!).
> 
> Ok I tried perf report, perf script and perf top and it looks good.
> In fact it looks better than the tip tree. I fixes some bugs that I can

tip.tip has the borked disabling of MMAP2 that may explain some of the
problems you described here

> see in tip:/perf/core:
> 
>      $ perf record perf bench sched messaging
>      $ perf report --stdio -s comm
>      write failure on standard output: Bad file descriptor
> 
> Not sure where that comes from, but it's fixed in your tree. May be that's
> on the fixes before the comm patches in your tree.
> 
> Also it differentiate between pre-exec and post-fork events, which looks
> more precise (and it fixes some comm mangling as well):
> 
> 
> Before:
> 
>      # Overhead          Command
>      # ........  ...............
>      #
>        100.00%  sched-me

I noticed the above, just on the --stdio tho, checking why this is so...
Can you try without --stdio, even using --tui?

- Arnaldo

> 
> After:
> 
>      # Overhead          Command
>      # ........  ...............
>      #
>         99.89%  sched-messaging
>         0.11%             perf
> 
> 
> Thanks!

Re: [PATCH 6/8] perf tools: Add new comm infrastructure

[Arnaldo Carvalho de Melo]

Em Mon, Oct 28, 2013 at 02:01:11PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Mon, Oct 28, 2013 at 05:05:30PM +0100, Frederic Weisbecker escreveu:
> > 
> > Also it differentiate between pre-exec and post-fork events, which looks
> > more precise (and it fixes some comm mangling as well):
> > 
> > 
> > Before:
> > 
> >      # Overhead          Command
> >      # ........  ...............
> >      #
> >        100.00%  sched-me
> 
> I noticed the above, just on the --stdio tho, checking why this is so...
> Can you try without --stdio, even using --tui?

Thanks for this report, if you try my perf/urgent now it should be
fixed, was a problem fixed by Jiri Olsa that I thought affected only
perf/core.

I cherry picked it and it fixed the problem for me, can you check that?

- Arnaldo

Re: [PATCH 6/8] perf tools: Add new comm infrastructure

[Frederic Weisbecker]

On Mon, Oct 28, 2013 at 02:48:43PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Mon, Oct 28, 2013 at 02:01:11PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Mon, Oct 28, 2013 at 05:05:30PM +0100, Frederic Weisbecker escreveu:
> > > 
> > > Also it differentiate between pre-exec and post-fork events, which looks
> > > more precise (and it fixes some comm mangling as well):
> > > 
> > > 
> > > Before:
> > > 
> > >      # Overhead          Command
> > >      # ........  ...............
> > >      #
> > >        100.00%  sched-me
> > 
> > I noticed the above, just on the --stdio tho, checking why this is so...
> > Can you try without --stdio, even using --tui?
> 
> Thanks for this report, if you try my perf/urgent now it should be
> fixed, was a problem fixed by Jiri Olsa that I thought affected only
> perf/core.
> 
> I cherry picked it and it fixed the problem for me, can you check that?

Yep, I checked your perf/urgent queue 8e50d384cc1d5afd2989cf0f7093756ed7164eb2 and it's
fixed there, although I can't find the particular patch fixing this, but it works :)

Thanks.

> 
> - Arnaldo

Re: [PATCH 6/8] perf tools: Add new comm infrastructure

[Arnaldo Carvalho de Melo]

Em Tue, Oct 29, 2013 at 10:20:33AM +0100, Frederic Weisbecker escreveu:
> On Mon, Oct 28, 2013 at 02:48:43PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Mon, Oct 28, 2013 at 02:01:11PM -0300, Arnaldo Carvalho de Melo escreveu:
> > > Em Mon, Oct 28, 2013 at 05:05:30PM +0100, Frederic Weisbecker escreveu:
> > > I noticed the above, just on the --stdio tho, checking why this is so...
> > > Can you try without --stdio, even using --tui?

> > Thanks for this report, if you try my perf/urgent now it should be
> > fixed, was a problem fixed by Jiri Olsa that I thought affected only
> > perf/core.

> > I cherry picked it and it fixed the problem for me, can you check that?

> Yep, I checked your perf/urgent queue 8e50d384cc1d5afd2989cf0f7093756ed7164eb2 and it's
> fixed there, although I can't find the particular patch fixing this, but it works :)

This is the one, by Jiri:

https://git.kernel.org/cgit/linux/kernel/git/acme/linux.git/commit/?h=perf/urgent&id=9754c4f9b23d5ce6756514acdf134ad61470734a

- Arnaldo