linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@infradead.org>
To: Ingo Molnar <mingo@kernel.org>
Cc: linux-kernel@vger.kernel.org,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Jiri Olsa <jolsa@redhat.com>, David Ahern <dsahern@gmail.com>,
	Ingo Molnar <mingo@elte.hu>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Arnaldo Carvalho de Melo <acme@redhat.com>,
	Stephane Eranian <eranian@google.com>,
	Namhyung Kim <namhyung@kernel.org>
Subject: [PATCH 05/29] perf tools: Add new COMM infrastructure
Date: Mon,  4 Nov 2013 14:58:43 -0300	[thread overview]
Message-ID: <1383587947-17419-6-git-send-email-acme@infradead.org> (raw)
In-Reply-To: <1383587947-17419-1-git-send-email-acme@infradead.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>
Tested-by: Jiri Olsa <jolsa@redhat.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>
[ Use __ as separator for class__method for private comm_str methods ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Makefile.perf   |   2 +
 tools/perf/builtin-trace.c |   4 +-
 tools/perf/util/comm.c     | 106 +++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/comm.h     |  20 +++++++++
 tools/perf/util/thread.c   |  92 +++++++++++++++++++++++++++++----------
 tools/perf/util/thread.h   |   3 +-
 6 files changed, 200 insertions(+), 27 deletions(-)
 create mode 100644 tools/perf/util/comm.c
 create mode 100644 tools/perf/util/comm.h

diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index bc7cfa18a1e3..cb52bdb755c7 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -273,6 +273,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
@@ -341,6 +342,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/builtin-trace.c b/tools/perf/builtin-trace.c
index 95d639212d98..b3e57dc64546 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -1114,7 +1114,7 @@ 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_str(thread));
 		printed += fprintf(fp, "%d ", thread->tid);
 	}
 
@@ -1986,7 +1986,7 @@ static int trace__fprintf_one_thread(struct thread *thread, void *priv)
 	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_str(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/util/comm.c b/tools/perf/util/comm.c
new file mode 100644
index 000000000000..8b3ac9f0207f
--- /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 0ea73fe383f5..15c53c2e109e 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 *thread = zalloc(sizeof(*thread));
 
 	if (thread != NULL) {
@@ -16,47 +19,88 @@ struct thread *thread__new(pid_t pid, pid_t tid)
 		thread->pid_ = pid;
 		thread->tid = tid;
 		thread->ppid = -1;
-		thread->comm = malloc(32);
-		if (thread->comm)
-			snprintf(thread->comm, 32, ":%d", thread->tid);
+		INIT_LIST_HEAD(&thread->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, &thread->comm_list);
 	}
 
 	return thread;
+
+err_thread:
+	free(thread);
+	return NULL;
 }
 
 void thread__delete(struct thread *thread)
 {
+	struct comm *comm, *tmp;
+
 	map_groups__exit(&thread->mg);
-	free(thread->comm);
+	list_for_each_entry_safe(comm, tmp, &thread->comm_list, list) {
+		list_del(&comm->list);
+		comm__free(comm);
+	}
+
 	free(thread);
 }
 
-int thread__set_comm(struct thread *thread, const char *comm,
-		     u64 timestamp __maybe_unused)
+static struct comm *thread__comm(const struct thread *thread)
 {
-	int err;
+	if (list_empty(&thread->comm_list))
+		return NULL;
 
-	if (thread->comm)
-		free(thread->comm);
-	thread->comm = strdup(comm);
-	err = thread->comm == NULL ? -ENOMEM : 0;
-	if (!err) {
-		thread->comm_set = true;
+	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 *thread)
 {
 	if (!thread->comm_len) {
-		if (!thread->comm)
+		const char *comm = thread__comm_str(thread);
+		if (!comm)
 			return 0;
-		thread->comm_len = strlen(thread->comm);
+		thread->comm_len = strlen(comm);
 	}
 
 	return thread->comm_len;
@@ -74,17 +118,17 @@ void thread__insert_map(struct thread *thread, struct map *map)
 	map_groups__insert(&thread->mg, map);
 }
 
-int thread__fork(struct thread *thread, 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 (thread->comm)
-			free(thread->comm);
-		thread->comm = strdup(parent->comm);
-		if (!thread->comm)
+		const char *comm = thread__comm_str(parent);
+		if (!comm)
 			return -ENOMEM;
+		err = thread__set_comm(thread, comm, timestamp);
+		if (!err)
+			return err;
 		thread->comm_set = true;
 	}
 
diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
index 4e9724270a64..8702c6b4163a 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.8.1.4


  parent reply	other threads:[~2013-11-04 18:07 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-04 17:58 [GIT PULL 00/29] perf/core improvements and fixes Arnaldo Carvalho de Melo
2013-11-04 17:58 ` [PATCH 01/29] perf probe: Fix typo Arnaldo Carvalho de Melo
2013-11-04 17:58 ` [PATCH 02/29] perf tools: Add missing data.h into LIB_H headers Arnaldo Carvalho de Melo
2013-11-04 17:58 ` [PATCH 03/29] perf tools: Use an accessor to read thread comm Arnaldo Carvalho de Melo
2013-11-04 17:58 ` [PATCH 04/29] perf tools: Add time argument on COMM setting Arnaldo Carvalho de Melo
2013-11-04 17:58 ` Arnaldo Carvalho de Melo [this message]
2013-11-04 17:58 ` [PATCH 06/29] perf tools: Compare hists comm by addresses Arnaldo Carvalho de Melo
2013-11-04 17:58 ` [PATCH 07/29] perf tools: Get current comm instead of last one Arnaldo Carvalho de Melo
2013-11-04 17:58 ` [PATCH 08/29] perf evsel: Add a debug print if perf_event_open fails Arnaldo Carvalho de Melo
2013-11-04 17:58 ` [PATCH 09/29] perf script: Set up output options for in-stream attributes Arnaldo Carvalho de Melo
2013-11-04 17:58 ` [PATCH 10/29] perf tools: Fix 32-bit cross build Arnaldo Carvalho de Melo
2013-11-04 17:58 ` [PATCH 11/29] perf tools: Fix libunwind build and feature detection for 32-bit build Arnaldo Carvalho de Melo
2013-11-04 17:58 ` [PATCH 12/29] perf evlist: Add a debug print if event buffer mmap fails Arnaldo Carvalho de Melo
2013-11-04 17:58 ` [PATCH 13/29] perf evsel: Always use perf_evsel__set_sample_bit() Arnaldo Carvalho de Melo
2013-11-04 17:58 ` [PATCH 14/29] perf evsel: Add missing overflow check for TRANSACTION Arnaldo Carvalho de Melo
2013-11-04 17:58 ` [PATCH 15/29] perf test: Update "sample parsing" test for PERF_SAMPLE_TRANSACTION Arnaldo Carvalho de Melo
2013-11-04 17:58 ` [PATCH 16/29] perf evsel: Synthesize PERF_SAMPLE_TRANSACTION Arnaldo Carvalho de Melo
2013-11-04 17:58 ` [PATCH 17/29] perf tools: Show single option when failed to parse Arnaldo Carvalho de Melo
2013-11-04 17:58 ` [PATCH 18/29] perf report: Postpone setting up browser after parsing options Arnaldo Carvalho de Melo
2013-11-04 17:58 ` [PATCH 19/29] perf report: Use parse_options_usage() for -s option failure Arnaldo Carvalho de Melo
2013-11-04 17:58 ` [PATCH 20/29] perf top: " Arnaldo Carvalho de Melo
2013-11-04 17:58 ` [PATCH 21/29] perf stat: Enhance option parse error message Arnaldo Carvalho de Melo
2013-11-04 17:59 ` [PATCH 22/29] tools lib traceevent: Add support for extracting trace_clock in report Arnaldo Carvalho de Melo
2013-11-04 17:59 ` [PATCH 23/29] tools lib traceevent: Update printk formats when entered Arnaldo Carvalho de Melo
2013-11-04 17:59 ` [PATCH 24/29] tools lib traceevent: If %s is a pointer, check printk formats Arnaldo Carvalho de Melo
2013-11-04 17:59 ` [PATCH 25/29] tools lib traceevent: Handle __print_hex(__get_dynamic_array(fieldname), len) Arnaldo Carvalho de Melo
2013-11-04 17:59 ` [PATCH 26/29] tools lib traceevent: Have bprintk output the same as the kernel does Arnaldo Carvalho de Melo
2013-11-04 17:59 ` [PATCH 27/29] tools lib traceevent: Check for spaces in character array Arnaldo Carvalho de Melo
2013-11-04 17:59 ` [PATCH 28/29] tools lib traceevent: Add flags NOHANDLE and PRINTRAW to individual events Arnaldo Carvalho de Melo
2013-11-04 17:59 ` [PATCH 29/29] tools lib traceevent: Add pevent_print_func_field() helper function Arnaldo Carvalho de Melo
2013-11-04 20:16 ` [GIT PULL 00/29] perf/core improvements and fixes Ingo Molnar
2013-11-05  7:52   ` Ingo Molnar
2013-11-05  8:05     ` Ingo Molnar

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=1383587947-17419-6-git-send-email-acme@infradead.org \
    --to=acme@infradead.org \
    --cc=a.p.zijlstra@chello.nl \
    --cc=acme@redhat.com \
    --cc=dsahern@gmail.com \
    --cc=eranian@google.com \
    --cc=fweisbec@gmail.com \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=mingo@kernel.org \
    --cc=namhyung@kernel.org \
    /path/to/YOUR_REPLY

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

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