[GIT PULL] perf tools: Factorize common code

public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed

* [GIT PULL] perf tools: Factorize common code
@ 2009-08-12  9:25 Frederic Weisbecker
  2009-08-12  9:26 ` [PATCH 1/3] perf tools: Factorize high level dso helpers Frederic Weisbecker
                   ` (5 more replies)
  0 siblings, 6 replies; 17+ messages in thread
From: Frederic Weisbecker @ 2009-08-12  9:25 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: LKML, Frederic Weisbecker, Arnaldo Carvalho de Melo,
	Peter Zijlstra, Mike Galbraith, Brice Goglin

Ingo,

This set factorizes the common definition of structs, functions,
variables inside perf tools to make it easier for perf trace to
be created.

I guess this patchset only factorizes 10 % of what could be done,
the largest part is the intersection between perf report and perf
annotate. It's a lot of work.

I guess I will do the rest in separate patchsets while writing
perf trace.

Thanks,
Frederic.


The following changes since commit 9f8666971185b86615a074bcac67c90fdf8af8bc:
  Brice Goglin (1):
        perf report: Add raw displaying of per-thread counters

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing.git ..BRANCH.NOT.VERIFIED..

Frederic Weisbecker (3):
      perf tools: Factorize high level dso helpers
      perf tools: Factorize the event structure definitions in a single file
      perf tools: Factorize the map helpers

 tools/perf/Makefile           |    2 +
 tools/perf/builtin-annotate.c |  209 +-------------------------------
 tools/perf/builtin-record.c   |   20 +---
 tools/perf/builtin-report.c   |  274 +----------------------------------------
 tools/perf/builtin-stat.c     |    1 -
 tools/perf/builtin-top.c      |   24 ----
 tools/perf/builtin.h          |    1 +
 tools/perf/perf.h             |    1 +
 tools/perf/util/callchain.h   |    1 +
 tools/perf/util/event.h       |   84 +++++++++++++
 tools/perf/util/map.c         |   97 +++++++++++++++
 tools/perf/util/symbol.c      |   97 +++++++++++++++
 tools/perf/util/symbol.h      |   12 ++
 tools/perf/util/util.h        |    1 +
 14 files changed, 303 insertions(+), 521 deletions(-)
 create mode 100644 tools/perf/util/event.h
 create mode 100644 tools/perf/util/map.c

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 1/3] perf tools: Factorize high level dso helpers
  2009-08-12  9:25 [GIT PULL] perf tools: Factorize common code Frederic Weisbecker
@ 2009-08-12  9:26 ` Frederic Weisbecker
  2009-08-12  9:59   ` Frederic Weisbecker
  2009-08-12  9:26 ` [PATCH 2/3] perf tools: Factorize the event structure definitions in a single file Frederic Weisbecker
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 17+ messages in thread
From: Frederic Weisbecker @ 2009-08-12  9:26 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: LKML, Frederic Weisbecker, Arnaldo Carvalho de Melo,
	Peter Zijlstra, Mike Galbraith, Brice Goglin

Factorize multiple definitions of high level dso helpers into the
symbol source file.

The side effect is a general export of the verbose and eprintf
debugging helpers into a new file dedicated to debugging purposes.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Brice Goglin <Brice.Goglin@inria.fr>
---
 tools/perf/Makefile           |    1 +
 tools/perf/builtin-annotate.c |   96 ----------------------------------------
 tools/perf/builtin-record.c   |    1 -
 tools/perf/builtin-report.c   |   97 -----------------------------------------
 tools/perf/builtin-stat.c     |    1 -
 tools/perf/builtin-top.c      |    4 --
 tools/perf/builtin.h          |    1 +
 tools/perf/perf.h             |    1 +
 tools/perf/util/symbol.c      |   97 +++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/symbol.h      |   11 +++++
 10 files changed, 111 insertions(+), 199 deletions(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index de7beac..2aee21b 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -339,6 +339,7 @@ LIB_OBJS += util/pager.o
 LIB_OBJS += util/header.o
 LIB_OBJS += util/callchain.o
 LIB_OBJS += util/values.o
+LIB_OBJS += util/debug.o
 
 BUILTIN_OBJS += builtin-annotate.o
 BUILTIN_OBJS += builtin-help.o
diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 1dba568..1a79299 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -26,7 +26,6 @@
 #define SHOW_HV		4
 
 static char		const *input_name = "perf.data";
-static char		*vmlinux = "vmlinux";
 
 static char		default_sort_order[] = "comm,symbol";
 static char		*sort_order = default_sort_order;
@@ -37,9 +36,6 @@ static int		show_mask = SHOW_KERNEL | SHOW_USER | SHOW_HV;
 static int		dump_trace = 0;
 #define dprintf(x...)	do { if (dump_trace) printf(x); } while (0)
 
-static int		verbose;
-
-static int		modules;
 
 static int		full_paths;
 
@@ -89,98 +85,6 @@ struct sym_ext {
 	char		*path;
 };
 
-static LIST_HEAD(dsos);
-static struct dso *kernel_dso;
-static struct dso *vdso;
-
-
-static void dsos__add(struct dso *dso)
-{
-	list_add_tail(&dso->node, &dsos);
-}
-
-static struct dso *dsos__find(const char *name)
-{
-	struct dso *pos;
-
-	list_for_each_entry(pos, &dsos, node)
-		if (strcmp(pos->name, name) == 0)
-			return pos;
-	return NULL;
-}
-
-static struct dso *dsos__findnew(const char *name)
-{
-	struct dso *dso = dsos__find(name);
-	int nr;
-
-	if (dso)
-		return dso;
-
-	dso = dso__new(name, 0);
-	if (!dso)
-		goto out_delete_dso;
-
-	nr = dso__load(dso, NULL, verbose);
-	if (nr < 0) {
-		if (verbose)
-			fprintf(stderr, "Failed to open: %s\n", name);
-		goto out_delete_dso;
-	}
-	if (!nr && verbose) {
-		fprintf(stderr,
-		"No symbols found in: %s, maybe install a debug package?\n",
-				name);
-	}
-
-	dsos__add(dso);
-
-	return dso;
-
-out_delete_dso:
-	dso__delete(dso);
-	return NULL;
-}
-
-static void dsos__fprintf(FILE *fp)
-{
-	struct dso *pos;
-
-	list_for_each_entry(pos, &dsos, node)
-		dso__fprintf(pos, fp);
-}
-
-static struct symbol *vdso__find_symbol(struct dso *dso, u64 ip)
-{
-	return dso__find_symbol(dso, ip);
-}
-
-static int load_kernel(void)
-{
-	int err;
-
-	kernel_dso = dso__new("[kernel]", 0);
-	if (!kernel_dso)
-		return -1;
-
-	err = dso__load_kernel(kernel_dso, vmlinux, NULL, verbose, modules);
-	if (err <= 0) {
-		dso__delete(kernel_dso);
-		kernel_dso = NULL;
-	} else
-		dsos__add(kernel_dso);
-
-	vdso = dso__new("[vdso]", 0);
-	if (!vdso)
-		return -1;
-
-	vdso->find_symbol = vdso__find_symbol;
-
-	dsos__add(vdso);
-
-	return err;
-}
-
 struct map {
 	struct list_head node;
 	u64	 start;
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 0345aad..afae387 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -40,7 +40,6 @@ static int			inherit				= 1;
 static int			force				= 0;
 static int			append_file			= 0;
 static int			call_graph			= 0;
-static int			verbose				= 0;
 static int			inherit_stat			= 0;
 static int			no_samples			= 0;
 static int			sample_address			= 0;
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 2357c66..827eab2 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -30,7 +30,6 @@
 #define SHOW_HV		4
 
 static char		const *input_name = "perf.data";
-static char		*vmlinux = NULL;
 
 static char		default_sort_order[] = "comm,dso,symbol";
 static char		*sort_order = default_sort_order;
@@ -46,11 +45,6 @@ static int		dump_trace = 0;
 #define dprintf(x...)	do { if (dump_trace) printf(x); } while (0)
 #define cdprintf(x...)	do { if (dump_trace) color_fprintf(stdout, color, x); } while (0)
 
-static int		verbose;
-#define eprintf(x...)	do { if (verbose) fprintf(stderr, x); } while (0)
-
-static int		modules;
-
 static int		full_paths;
 static int		show_nr_samples;
 
@@ -161,98 +155,7 @@ static int repsep_fprintf(FILE *fp, const char *fmt, ...)
 	return n;
 }
 
-static LIST_HEAD(dsos);
-static struct dso *kernel_dso;
-static struct dso *vdso;
-static struct dso *hypervisor_dso;
-
-static void dsos__add(struct dso *dso)
-{
-	list_add_tail(&dso->node, &dsos);
-}
-
-static struct dso *dsos__find(const char *name)
-{
-	struct dso *pos;
-
-	list_for_each_entry(pos, &dsos, node)
-		if (strcmp(pos->name, name) == 0)
-			return pos;
-	return NULL;
-}
-
-static struct dso *dsos__findnew(const char *name)
-{
-	struct dso *dso = dsos__find(name);
-	int nr;
-
-	if (dso)
-		return dso;
-
-	dso = dso__new(name, 0);
-	if (!dso)
-		goto out_delete_dso;
-
-	nr = dso__load(dso, NULL, verbose);
-	if (nr < 0) {
-		eprintf("Failed to open: %s\n", name);
-		goto out_delete_dso;
-	}
-	if (!nr)
-		eprintf("No symbols found in: %s, maybe install a debug package?\n", name);
-
-	dsos__add(dso);
-
-	return dso;
-
-out_delete_dso:
-	dso__delete(dso);
-	return NULL;
-}
-
-static void dsos__fprintf(FILE *fp)
-{
-	struct dso *pos;
-
-	list_for_each_entry(pos, &dsos, node)
-		dso__fprintf(pos, fp);
-}
-
-static struct symbol *vdso__find_symbol(struct dso *dso, u64 ip)
-{
-	return dso__find_symbol(dso, ip);
-}
-
-static int load_kernel(void)
-{
-	int err;
-
-	kernel_dso = dso__new("[kernel]", 0);
-	if (!kernel_dso)
-		return -1;
-
-	err = dso__load_kernel(kernel_dso, vmlinux, NULL, verbose, modules);
-	if (err <= 0) {
-		dso__delete(kernel_dso);
-		kernel_dso = NULL;
-	} else
-		dsos__add(kernel_dso);
-
-	vdso = dso__new("[vdso]", 0);
-	if (!vdso)
-		return -1;
 
-	vdso->find_symbol = vdso__find_symbol;
-
-	dsos__add(vdso);
-
-	hypervisor_dso = dso__new("[hypervisor]", 0);
-	if (!hypervisor_dso)
-		return -1;
-	dsos__add(hypervisor_dso);
-
-	return err;
-}
 
 static char __cwd[PATH_MAX];
 static char *cwd = __cwd;
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index b4b06c7..4b9dd4a 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -63,7 +63,6 @@ static struct perf_counter_attr default_attrs[] = {
 #define MAX_RUN			100
 
 static int			system_wide			=  0;
-static int			verbose				=  0;
 static unsigned int		nr_cpus				=  0;
 static int			run_idx				=  0;
 
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 7de28ce..0aa5673 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -68,8 +68,6 @@ static int			group				=  0;
 static unsigned int		page_size;
 static unsigned int		mmap_pages			= 16;
 static int			freq				=  0;
-static int			verbose				=  0;
-static char			*vmlinux			=  NULL;
 
 static int			delay_secs			=  2;
 static int			zero;
@@ -338,8 +336,6 @@ static void show_details(struct sym_entry *syme)
 		printf("%d lines not displayed, maybe increase display entries [e]\n", more);
 }
 
-struct dso			*kernel_dso;
-
 /*
  * Symbols will be added here in record_ip and will get out
  * after decayed.
diff --git a/tools/perf/builtin.h b/tools/perf/builtin.h
index 51d1682..3a63e41 100644
--- a/tools/perf/builtin.h
+++ b/tools/perf/builtin.h
@@ -22,5 +22,6 @@ extern int cmd_stat(int argc, const char **argv, const char *prefix);
 extern int cmd_top(int argc, const char **argv, const char *prefix);
 extern int cmd_version(int argc, const char **argv, const char *prefix);
 extern int cmd_list(int argc, const char **argv, const char *prefix);
+extern int cmd_trace(int argc, const char **argv, const char *prefix);
 
 #endif
diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index e5148e2..f550921 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -48,6 +48,7 @@
 
 #include "../../include/linux/perf_counter.h"
 #include "util/types.h"
+#include "util/debug.h"
 
 /*
  * prctl(PR_TASK_PERF_COUNTERS_DISABLE) will (cheaply) disable all
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index f1dcede..e9b13b4 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -937,6 +937,103 @@ int dso__load_kernel(struct dso *self, const char *vmlinux,
 	return err;
 }
 
+LIST_HEAD(dsos);
+struct dso	*kernel_dso;
+struct dso	*vdso;
+struct dso	*hypervisor_dso;
+
+char		*vmlinux = "vmlinux";
+int		modules;
+
+static void dsos__add(struct dso *dso)
+{
+	list_add_tail(&dso->node, &dsos);
+}
+
+static struct dso *dsos__find(const char *name)
+{
+	struct dso *pos;
+
+	list_for_each_entry(pos, &dsos, node)
+		if (strcmp(pos->name, name) == 0)
+			return pos;
+	return NULL;
+}
+
+struct dso *dsos__findnew(const char *name)
+{
+	struct dso *dso = dsos__find(name);
+	int nr;
+
+	if (dso)
+		return dso;
+
+	dso = dso__new(name, 0);
+	if (!dso)
+		goto out_delete_dso;
+
+	nr = dso__load(dso, NULL, verbose);
+	if (nr < 0) {
+		eprintf("Failed to open: %s\n", name);
+		goto out_delete_dso;
+	}
+	if (!nr)
+		eprintf("No symbols found in: %s, maybe install a debug package?\n", name);
+
+	dsos__add(dso);
+
+	return dso;
+
+out_delete_dso:
+	dso__delete(dso);
+	return NULL;
+}
+
+void dsos__fprintf(FILE *fp)
+{
+	struct dso *pos;
+
+	list_for_each_entry(pos, &dsos, node)
+		dso__fprintf(pos, fp);
+}
+
+static struct symbol *vdso__find_symbol(struct dso *dso, u64 ip)
+{
+	return dso__find_symbol(dso, ip);
+}
+
+int load_kernel(void)
+{
+	int err;
+
+	kernel_dso = dso__new("[kernel]", 0);
+	if (!kernel_dso)
+		return -1;
+
+	err = dso__load_kernel(kernel_dso, vmlinux, NULL, verbose, modules);
+	if (err <= 0) {
+		dso__delete(kernel_dso);
+		kernel_dso = NULL;
+	} else
+		dsos__add(kernel_dso);
+
+	vdso = dso__new("[vdso]", 0);
+	if (!vdso)
+		return -1;
+
+	vdso->find_symbol = vdso__find_symbol;
+
+	dsos__add(vdso);
+
+	hypervisor_dso = dso__new("[hypervisor]", 0);
+	if (!hypervisor_dso)
+		return -1;
+	dsos__add(hypervisor_dso);
+
+	return err;
+}
+
+
 void symbol__init(void)
 {
 	elf_version(EV_CURRENT);
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 1e003ec..f3490fc 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -48,9 +48,20 @@ int dso__load_kernel(struct dso *self, const char *vmlinux,
 		     symbol_filter_t filter, int verbose, int modules);
 int dso__load_modules(struct dso *self, symbol_filter_t filter, int verbose);
 int dso__load(struct dso *self, symbol_filter_t filter, int verbose);
+struct dso *dsos__findnew(const char *name);
+void dsos__fprintf(FILE *fp);
 
 size_t dso__fprintf(struct dso *self, FILE *fp);
 char dso__symtab_origin(const struct dso *self);
 
+int load_kernel(void);
+
 void symbol__init(void);
+
+extern struct list_head dsos;
+extern struct dso *kernel_dso;
+extern struct dso *vdso;
+extern struct dso *hypervisor_dso;
+extern char *vmlinux;
+extern int   modules;
 #endif /* _PERF_SYMBOL_ */
-- 
1.6.2.3


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/3] perf tools: Factorize high level dso helpers
  2009-08-12  9:26 ` [PATCH 1/3] perf tools: Factorize high level dso helpers Frederic Weisbecker
@ 2009-08-12  9:59   ` Frederic Weisbecker
  2009-08-12 10:08     ` Ingo Molnar
  0 siblings, 1 reply; 17+ messages in thread
From: Frederic Weisbecker @ 2009-08-12  9:59 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: LKML, Arnaldo Carvalho de Melo, Peter Zijlstra, Mike Galbraith,
	Brice Goglin

On Wed, Aug 12, 2009 at 11:26:00AM +0200, Frederic Weisbecker wrote:
> Factorize multiple definitions of high level dso helpers into the
> symbol source file.
> 
> The side effect is a general export of the verbose and eprintf
> debugging helpers into a new file dedicated to debugging purposes.
> 
> Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Mike Galbraith <efault@gmx.de>
> Cc: Brice Goglin <Brice.Goglin@inria.fr>
> ---
>  tools/perf/Makefile           |    1 +
>  tools/perf/builtin-annotate.c |   96 ----------------------------------------
>  tools/perf/builtin-record.c   |    1 -
>  tools/perf/builtin-report.c   |   97 -----------------------------------------
>  tools/perf/builtin-stat.c     |    1 -
>  tools/perf/builtin-top.c      |    4 --
>  tools/perf/builtin.h          |    1 +
>  tools/perf/perf.h             |    1 +
>  tools/perf/util/symbol.c      |   97 +++++++++++++++++++++++++++++++++++++++++
>  tools/perf/util/symbol.h      |   11 +++++
>  10 files changed, 111 insertions(+), 199 deletions(-)
> 
> diff --git a/tools/perf/Makefile b/tools/perf/Makefile
> index de7beac..2aee21b 100644
> --- a/tools/perf/Makefile
> +++ b/tools/perf/Makefile
> @@ -339,6 +339,7 @@ LIB_OBJS += util/pager.o
>  LIB_OBJS += util/header.o
>  LIB_OBJS += util/callchain.o
>  LIB_OBJS += util/values.o
> +LIB_OBJS += util/debug.o


Damn, did I forgot to git-add util/debug.c ?

How could I fix this? Another pull request?

Frederic.


>  
>  BUILTIN_OBJS += builtin-annotate.o
>  BUILTIN_OBJS += builtin-help.o
> diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
> index 1dba568..1a79299 100644
> --- a/tools/perf/builtin-annotate.c
> +++ b/tools/perf/builtin-annotate.c
> @@ -26,7 +26,6 @@
>  #define SHOW_HV		4
>  
>  static char		const *input_name = "perf.data";
> -static char		*vmlinux = "vmlinux";
>  
>  static char		default_sort_order[] = "comm,symbol";
>  static char		*sort_order = default_sort_order;
> @@ -37,9 +36,6 @@ static int		show_mask = SHOW_KERNEL | SHOW_USER | SHOW_HV;
>  static int		dump_trace = 0;
>  #define dprintf(x...)	do { if (dump_trace) printf(x); } while (0)
>  
> -static int		verbose;
> -
> -static int		modules;
>  
>  static int		full_paths;
>  
> @@ -89,98 +85,6 @@ struct sym_ext {
>  	char		*path;
>  };
>  
> -static LIST_HEAD(dsos);
> -static struct dso *kernel_dso;
> -static struct dso *vdso;
> -
> -
> -static void dsos__add(struct dso *dso)
> -{
> -	list_add_tail(&dso->node, &dsos);
> -}
> -
> -static struct dso *dsos__find(const char *name)
> -{
> -	struct dso *pos;
> -
> -	list_for_each_entry(pos, &dsos, node)
> -		if (strcmp(pos->name, name) == 0)
> -			return pos;
> -	return NULL;
> -}
> -
> -static struct dso *dsos__findnew(const char *name)
> -{
> -	struct dso *dso = dsos__find(name);
> -	int nr;
> -
> -	if (dso)
> -		return dso;
> -
> -	dso = dso__new(name, 0);
> -	if (!dso)
> -		goto out_delete_dso;
> -
> -	nr = dso__load(dso, NULL, verbose);
> -	if (nr < 0) {
> -		if (verbose)
> -			fprintf(stderr, "Failed to open: %s\n", name);
> -		goto out_delete_dso;
> -	}
> -	if (!nr && verbose) {
> -		fprintf(stderr,
> -		"No symbols found in: %s, maybe install a debug package?\n",
> -				name);
> -	}
> -
> -	dsos__add(dso);
> -
> -	return dso;
> -
> -out_delete_dso:
> -	dso__delete(dso);
> -	return NULL;
> -}
> -
> -static void dsos__fprintf(FILE *fp)
> -{
> -	struct dso *pos;
> -
> -	list_for_each_entry(pos, &dsos, node)
> -		dso__fprintf(pos, fp);
> -}
> -
> -static struct symbol *vdso__find_symbol(struct dso *dso, u64 ip)
> -{
> -	return dso__find_symbol(dso, ip);
> -}
> -
> -static int load_kernel(void)
> -{
> -	int err;
> -
> -	kernel_dso = dso__new("[kernel]", 0);
> -	if (!kernel_dso)
> -		return -1;
> -
> -	err = dso__load_kernel(kernel_dso, vmlinux, NULL, verbose, modules);
> -	if (err <= 0) {
> -		dso__delete(kernel_dso);
> -		kernel_dso = NULL;
> -	} else
> -		dsos__add(kernel_dso);
> -
> -	vdso = dso__new("[vdso]", 0);
> -	if (!vdso)
> -		return -1;
> -
> -	vdso->find_symbol = vdso__find_symbol;
> -
> -	dsos__add(vdso);
> -
> -	return err;
> -}
> -
>  struct map {
>  	struct list_head node;
>  	u64	 start;
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index 0345aad..afae387 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -40,7 +40,6 @@ static int			inherit				= 1;
>  static int			force				= 0;
>  static int			append_file			= 0;
>  static int			call_graph			= 0;
> -static int			verbose				= 0;
>  static int			inherit_stat			= 0;
>  static int			no_samples			= 0;
>  static int			sample_address			= 0;
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index 2357c66..827eab2 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -30,7 +30,6 @@
>  #define SHOW_HV		4
>  
>  static char		const *input_name = "perf.data";
> -static char		*vmlinux = NULL;
>  
>  static char		default_sort_order[] = "comm,dso,symbol";
>  static char		*sort_order = default_sort_order;
> @@ -46,11 +45,6 @@ static int		dump_trace = 0;
>  #define dprintf(x...)	do { if (dump_trace) printf(x); } while (0)
>  #define cdprintf(x...)	do { if (dump_trace) color_fprintf(stdout, color, x); } while (0)
>  
> -static int		verbose;
> -#define eprintf(x...)	do { if (verbose) fprintf(stderr, x); } while (0)
> -
> -static int		modules;
> -
>  static int		full_paths;
>  static int		show_nr_samples;
>  
> @@ -161,98 +155,7 @@ static int repsep_fprintf(FILE *fp, const char *fmt, ...)
>  	return n;
>  }
>  
> -static LIST_HEAD(dsos);
> -static struct dso *kernel_dso;
> -static struct dso *vdso;
> -static struct dso *hypervisor_dso;
> -
> -static void dsos__add(struct dso *dso)
> -{
> -	list_add_tail(&dso->node, &dsos);
> -}
> -
> -static struct dso *dsos__find(const char *name)
> -{
> -	struct dso *pos;
> -
> -	list_for_each_entry(pos, &dsos, node)
> -		if (strcmp(pos->name, name) == 0)
> -			return pos;
> -	return NULL;
> -}
> -
> -static struct dso *dsos__findnew(const char *name)
> -{
> -	struct dso *dso = dsos__find(name);
> -	int nr;
> -
> -	if (dso)
> -		return dso;
> -
> -	dso = dso__new(name, 0);
> -	if (!dso)
> -		goto out_delete_dso;
> -
> -	nr = dso__load(dso, NULL, verbose);
> -	if (nr < 0) {
> -		eprintf("Failed to open: %s\n", name);
> -		goto out_delete_dso;
> -	}
> -	if (!nr)
> -		eprintf("No symbols found in: %s, maybe install a debug package?\n", name);
> -
> -	dsos__add(dso);
> -
> -	return dso;
> -
> -out_delete_dso:
> -	dso__delete(dso);
> -	return NULL;
> -}
> -
> -static void dsos__fprintf(FILE *fp)
> -{
> -	struct dso *pos;
> -
> -	list_for_each_entry(pos, &dsos, node)
> -		dso__fprintf(pos, fp);
> -}
> -
> -static struct symbol *vdso__find_symbol(struct dso *dso, u64 ip)
> -{
> -	return dso__find_symbol(dso, ip);
> -}
> -
> -static int load_kernel(void)
> -{
> -	int err;
> -
> -	kernel_dso = dso__new("[kernel]", 0);
> -	if (!kernel_dso)
> -		return -1;
> -
> -	err = dso__load_kernel(kernel_dso, vmlinux, NULL, verbose, modules);
> -	if (err <= 0) {
> -		dso__delete(kernel_dso);
> -		kernel_dso = NULL;
> -	} else
> -		dsos__add(kernel_dso);
> -
> -	vdso = dso__new("[vdso]", 0);
> -	if (!vdso)
> -		return -1;
>  
> -	vdso->find_symbol = vdso__find_symbol;
> -
> -	dsos__add(vdso);
> -
> -	hypervisor_dso = dso__new("[hypervisor]", 0);
> -	if (!hypervisor_dso)
> -		return -1;
> -	dsos__add(hypervisor_dso);
> -
> -	return err;
> -}
>  
>  static char __cwd[PATH_MAX];
>  static char *cwd = __cwd;
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index b4b06c7..4b9dd4a 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -63,7 +63,6 @@ static struct perf_counter_attr default_attrs[] = {
>  #define MAX_RUN			100
>  
>  static int			system_wide			=  0;
> -static int			verbose				=  0;
>  static unsigned int		nr_cpus				=  0;
>  static int			run_idx				=  0;
>  
> diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> index 7de28ce..0aa5673 100644
> --- a/tools/perf/builtin-top.c
> +++ b/tools/perf/builtin-top.c
> @@ -68,8 +68,6 @@ static int			group				=  0;
>  static unsigned int		page_size;
>  static unsigned int		mmap_pages			= 16;
>  static int			freq				=  0;
> -static int			verbose				=  0;
> -static char			*vmlinux			=  NULL;
>  
>  static int			delay_secs			=  2;
>  static int			zero;
> @@ -338,8 +336,6 @@ static void show_details(struct sym_entry *syme)
>  		printf("%d lines not displayed, maybe increase display entries [e]\n", more);
>  }
>  
> -struct dso			*kernel_dso;
> -
>  /*
>   * Symbols will be added here in record_ip and will get out
>   * after decayed.
> diff --git a/tools/perf/builtin.h b/tools/perf/builtin.h
> index 51d1682..3a63e41 100644
> --- a/tools/perf/builtin.h
> +++ b/tools/perf/builtin.h
> @@ -22,5 +22,6 @@ extern int cmd_stat(int argc, const char **argv, const char *prefix);
>  extern int cmd_top(int argc, const char **argv, const char *prefix);
>  extern int cmd_version(int argc, const char **argv, const char *prefix);
>  extern int cmd_list(int argc, const char **argv, const char *prefix);
> +extern int cmd_trace(int argc, const char **argv, const char *prefix);
>  
>  #endif
> diff --git a/tools/perf/perf.h b/tools/perf/perf.h
> index e5148e2..f550921 100644
> --- a/tools/perf/perf.h
> +++ b/tools/perf/perf.h
> @@ -48,6 +48,7 @@
>  
>  #include "../../include/linux/perf_counter.h"
>  #include "util/types.h"
> +#include "util/debug.h"
>  
>  /*
>   * prctl(PR_TASK_PERF_COUNTERS_DISABLE) will (cheaply) disable all
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index f1dcede..e9b13b4 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -937,6 +937,103 @@ int dso__load_kernel(struct dso *self, const char *vmlinux,
>  	return err;
>  }
>  
> +LIST_HEAD(dsos);
> +struct dso	*kernel_dso;
> +struct dso	*vdso;
> +struct dso	*hypervisor_dso;
> +
> +char		*vmlinux = "vmlinux";
> +int		modules;
> +
> +static void dsos__add(struct dso *dso)
> +{
> +	list_add_tail(&dso->node, &dsos);
> +}
> +
> +static struct dso *dsos__find(const char *name)
> +{
> +	struct dso *pos;
> +
> +	list_for_each_entry(pos, &dsos, node)
> +		if (strcmp(pos->name, name) == 0)
> +			return pos;
> +	return NULL;
> +}
> +
> +struct dso *dsos__findnew(const char *name)
> +{
> +	struct dso *dso = dsos__find(name);
> +	int nr;
> +
> +	if (dso)
> +		return dso;
> +
> +	dso = dso__new(name, 0);
> +	if (!dso)
> +		goto out_delete_dso;
> +
> +	nr = dso__load(dso, NULL, verbose);
> +	if (nr < 0) {
> +		eprintf("Failed to open: %s\n", name);
> +		goto out_delete_dso;
> +	}
> +	if (!nr)
> +		eprintf("No symbols found in: %s, maybe install a debug package?\n", name);
> +
> +	dsos__add(dso);
> +
> +	return dso;
> +
> +out_delete_dso:
> +	dso__delete(dso);
> +	return NULL;
> +}
> +
> +void dsos__fprintf(FILE *fp)
> +{
> +	struct dso *pos;
> +
> +	list_for_each_entry(pos, &dsos, node)
> +		dso__fprintf(pos, fp);
> +}
> +
> +static struct symbol *vdso__find_symbol(struct dso *dso, u64 ip)
> +{
> +	return dso__find_symbol(dso, ip);
> +}
> +
> +int load_kernel(void)
> +{
> +	int err;
> +
> +	kernel_dso = dso__new("[kernel]", 0);
> +	if (!kernel_dso)
> +		return -1;
> +
> +	err = dso__load_kernel(kernel_dso, vmlinux, NULL, verbose, modules);
> +	if (err <= 0) {
> +		dso__delete(kernel_dso);
> +		kernel_dso = NULL;
> +	} else
> +		dsos__add(kernel_dso);
> +
> +	vdso = dso__new("[vdso]", 0);
> +	if (!vdso)
> +		return -1;
> +
> +	vdso->find_symbol = vdso__find_symbol;
> +
> +	dsos__add(vdso);
> +
> +	hypervisor_dso = dso__new("[hypervisor]", 0);
> +	if (!hypervisor_dso)
> +		return -1;
> +	dsos__add(hypervisor_dso);
> +
> +	return err;
> +}
> +
> +
>  void symbol__init(void)
>  {
>  	elf_version(EV_CURRENT);
> diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
> index 1e003ec..f3490fc 100644
> --- a/tools/perf/util/symbol.h
> +++ b/tools/perf/util/symbol.h
> @@ -48,9 +48,20 @@ int dso__load_kernel(struct dso *self, const char *vmlinux,
>  		     symbol_filter_t filter, int verbose, int modules);
>  int dso__load_modules(struct dso *self, symbol_filter_t filter, int verbose);
>  int dso__load(struct dso *self, symbol_filter_t filter, int verbose);
> +struct dso *dsos__findnew(const char *name);
> +void dsos__fprintf(FILE *fp);
>  
>  size_t dso__fprintf(struct dso *self, FILE *fp);
>  char dso__symtab_origin(const struct dso *self);
>  
> +int load_kernel(void);
> +
>  void symbol__init(void);
> +
> +extern struct list_head dsos;
> +extern struct dso *kernel_dso;
> +extern struct dso *vdso;
> +extern struct dso *hypervisor_dso;
> +extern char *vmlinux;
> +extern int   modules;
>  #endif /* _PERF_SYMBOL_ */
> -- 
> 1.6.2.3
> 


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/3] perf tools: Factorize high level dso helpers
  2009-08-12  9:59   ` Frederic Weisbecker
@ 2009-08-12 10:08     ` Ingo Molnar
  2009-08-12 10:48       ` [GIT PULL v2] perf tools: Factorize common code Frederic Weisbecker
                         ` (3 more replies)
  0 siblings, 4 replies; 17+ messages in thread
From: Ingo Molnar @ 2009-08-12 10:08 UTC (permalink / raw)
  To: Frederic Weisbecker
  Cc: LKML, Arnaldo Carvalho de Melo, Peter Zijlstra, Mike Galbraith,
	Brice Goglin


* Frederic Weisbecker <fweisbec@gmail.com> wrote:

> On Wed, Aug 12, 2009 at 11:26:00AM +0200, Frederic Weisbecker wrote:
> > Factorize multiple definitions of high level dso helpers into the
> > symbol source file.
> > 
> > The side effect is a general export of the verbose and eprintf
> > debugging helpers into a new file dedicated to debugging purposes.
> > 
> > Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
> > Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Mike Galbraith <efault@gmx.de>
> > Cc: Brice Goglin <Brice.Goglin@inria.fr>
> > ---
> >  tools/perf/Makefile           |    1 +
> >  tools/perf/builtin-annotate.c |   96 ----------------------------------------
> >  tools/perf/builtin-record.c   |    1 -
> >  tools/perf/builtin-report.c   |   97 -----------------------------------------
> >  tools/perf/builtin-stat.c     |    1 -
> >  tools/perf/builtin-top.c      |    4 --
> >  tools/perf/builtin.h          |    1 +
> >  tools/perf/perf.h             |    1 +
> >  tools/perf/util/symbol.c      |   97 +++++++++++++++++++++++++++++++++++++++++
> >  tools/perf/util/symbol.h      |   11 +++++
> >  10 files changed, 111 insertions(+), 199 deletions(-)
> > 
> > diff --git a/tools/perf/Makefile b/tools/perf/Makefile
> > index de7beac..2aee21b 100644
> > --- a/tools/perf/Makefile
> > +++ b/tools/perf/Makefile
> > @@ -339,6 +339,7 @@ LIB_OBJS += util/pager.o
> >  LIB_OBJS += util/header.o
> >  LIB_OBJS += util/callchain.o
> >  LIB_OBJS += util/values.o
> > +LIB_OBJS += util/debug.o
> 
> 
> Damn, did I forgot to git-add util/debug.c ?
> 
> How could I fix this? Another pull request?

yeah. I've zapped those commits from perfcounters/core.

	Ingo

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [GIT PULL v2] perf tools: Factorize common code
  2009-08-12 10:08     ` Ingo Molnar
@ 2009-08-12 10:48       ` Frederic Weisbecker
  2009-08-12 10:54         ` Ingo Molnar
  2009-08-12 13:54         ` Arnaldo Carvalho de Melo
  2009-08-12 10:48       ` [PATCH 1/3 v2] perf tools: Factorize high level dso helpers Frederic Weisbecker
                         ` (2 subsequent siblings)
  3 siblings, 2 replies; 17+ messages in thread
From: Frederic Weisbecker @ 2009-08-12 10:48 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: LKML, Frederic Weisbecker, Arnaldo Carvalho de Melo,
	Peter Zijlstra, Mike Galbraith, Brice Goglin

On Wed, Aug 12, 2009 at 12:08:51PM +0200, Ingo Molnar wrote:
> yeah. I've zapped those commits from perfcounters/core.
>
>       Ingo

Ok, sorry about that.
I've rebuilt a clean tree. It should be fine now.

Thanks.

The following changes since commit 9f8666971185b86615a074bcac67c90fdf8af8bc:
  Brice Goglin (1):
        perf report: Add raw displaying of per-thread counters

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing.git \
	perfcounters/core

Frederic Weisbecker (3):
      perf tools: Factorize high level dso helpers
      perf tools: Factorize the event structure definitions in a single file
      perf tools: Factorize the map helpers

 tools/perf/Makefile           |    2 +
 tools/perf/builtin-annotate.c |  209 +-------------------------------
 tools/perf/builtin-record.c   |   20 +---
 tools/perf/builtin-report.c   |  274 +----------------------------------------
 tools/perf/builtin-stat.c     |    1 -
 tools/perf/builtin-top.c      |   24 ----
 tools/perf/builtin.h          |    1 +
 tools/perf/perf.h             |    1 +
 tools/perf/util/callchain.h   |    1 +
 tools/perf/util/debug.c       |   22 ++++
 tools/perf/util/debug.h       |    5 +
 tools/perf/util/event.h       |   84 +++++++++++++
 tools/perf/util/map.c         |   97 +++++++++++++++
 tools/perf/util/symbol.c      |   97 +++++++++++++++
 tools/perf/util/symbol.h      |   12 ++
 tools/perf/util/util.h        |    1 +
 16 files changed, 330 insertions(+), 521 deletions(-)
 create mode 100644 tools/perf/util/debug.c
 create mode 100644 tools/perf/util/debug.h
 create mode 100644 tools/perf/util/event.h
 create mode 100644 tools/perf/util/map.c

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [GIT PULL v2] perf tools: Factorize common code
  2009-08-12 10:48       ` [GIT PULL v2] perf tools: Factorize common code Frederic Weisbecker
@ 2009-08-12 10:54         ` Ingo Molnar
  2009-08-12 11:00           ` Frederic Weisbecker
  2009-08-12 13:54         ` Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 17+ messages in thread
From: Ingo Molnar @ 2009-08-12 10:54 UTC (permalink / raw)
  To: Frederic Weisbecker
  Cc: LKML, Arnaldo Carvalho de Melo, Peter Zijlstra, Mike Galbraith,
	Brice Goglin


* Frederic Weisbecker <fweisbec@gmail.com> wrote:

> On Wed, Aug 12, 2009 at 12:08:51PM +0200, Ingo Molnar wrote:
> > yeah. I've zapped those commits from perfcounters/core.
> >
> >       Ingo
> 
> Ok, sorry about that.
> I've rebuilt a clean tree. It should be fine now.

btw., you can always check whether there's any missing 
git-add, via:

  git ls-files --others

And the safest way to commit is to use 'git commit -a' - 
that will auto-include new files. (but the tree has to be 
clean for that)

> 
> Thanks.
> 
> The following changes since commit 9f8666971185b86615a074bcac67c90fdf8af8bc:
>   Brice Goglin (1):
>         perf report: Add raw displaying of per-thread counters
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing.git \
> 	perfcounters/core
> 
> Frederic Weisbecker (3):
>       perf tools: Factorize high level dso helpers
>       perf tools: Factorize the event structure definitions in a single file
>       perf tools: Factorize the map helpers
> 
>  tools/perf/Makefile           |    2 +
>  tools/perf/builtin-annotate.c |  209 +-------------------------------
>  tools/perf/builtin-record.c   |   20 +---
>  tools/perf/builtin-report.c   |  274 +----------------------------------------
>  tools/perf/builtin-stat.c     |    1 -
>  tools/perf/builtin-top.c      |   24 ----
>  tools/perf/builtin.h          |    1 +
>  tools/perf/perf.h             |    1 +
>  tools/perf/util/callchain.h   |    1 +
>  tools/perf/util/debug.c       |   22 ++++
>  tools/perf/util/debug.h       |    5 +
>  tools/perf/util/event.h       |   84 +++++++++++++
>  tools/perf/util/map.c         |   97 +++++++++++++++
>  tools/perf/util/symbol.c      |   97 +++++++++++++++
>  tools/perf/util/symbol.h      |   12 ++
>  tools/perf/util/util.h        |    1 +
>  16 files changed, 330 insertions(+), 521 deletions(-)
>  create mode 100644 tools/perf/util/debug.c
>  create mode 100644 tools/perf/util/debug.h
>  create mode 100644 tools/perf/util/event.h
>  create mode 100644 tools/perf/util/map.c

Re-pulled it - it works fine here too - thanks!

	Ingo

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [GIT PULL v2] perf tools: Factorize common code
  2009-08-12 10:54         ` Ingo Molnar
@ 2009-08-12 11:00           ` Frederic Weisbecker
  0 siblings, 0 replies; 17+ messages in thread
From: Frederic Weisbecker @ 2009-08-12 11:00 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: LKML, Arnaldo Carvalho de Melo, Peter Zijlstra, Mike Galbraith,
	Brice Goglin

On Wed, Aug 12, 2009 at 12:54:33PM +0200, Ingo Molnar wrote:
> 
> * Frederic Weisbecker <fweisbec@gmail.com> wrote:
> 
> > On Wed, Aug 12, 2009 at 12:08:51PM +0200, Ingo Molnar wrote:
> > > yeah. I've zapped those commits from perfcounters/core.
> > >
> > >       Ingo
> > 
> > Ok, sorry about that.
> > I've rebuilt a clean tree. It should be fine now.
> 
> btw., you can always check whether there's any missing 
> git-add, via:
> 
>   git ls-files --others


Ah nice tip.


 
> And the safest way to commit is to use 'git commit -a' - 
> that will auto-include new files. (but the tree has to be 
> clean for that)


Well, I always have junks in my directories. .rej/.orig files,
or more especially scripts and tests in the case of the tools/perf
directory. So it's hard to check everything this way.

Note that most of the time I use git-commit -a, but it doesn't include
untracked files AFAICS.


> > 
> > Thanks.
> > 
> > The following changes since commit 9f8666971185b86615a074bcac67c90fdf8af8bc:
> >   Brice Goglin (1):
> >         perf report: Add raw displaying of per-thread counters
> > 
> > are available in the git repository at:
> > 
> >   git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing.git \
> > 	perfcounters/core
> > 
> > Frederic Weisbecker (3):
> >       perf tools: Factorize high level dso helpers
> >       perf tools: Factorize the event structure definitions in a single file
> >       perf tools: Factorize the map helpers
> > 
> >  tools/perf/Makefile           |    2 +
> >  tools/perf/builtin-annotate.c |  209 +-------------------------------
> >  tools/perf/builtin-record.c   |   20 +---
> >  tools/perf/builtin-report.c   |  274 +----------------------------------------
> >  tools/perf/builtin-stat.c     |    1 -
> >  tools/perf/builtin-top.c      |   24 ----
> >  tools/perf/builtin.h          |    1 +
> >  tools/perf/perf.h             |    1 +
> >  tools/perf/util/callchain.h   |    1 +
> >  tools/perf/util/debug.c       |   22 ++++
> >  tools/perf/util/debug.h       |    5 +
> >  tools/perf/util/event.h       |   84 +++++++++++++
> >  tools/perf/util/map.c         |   97 +++++++++++++++
> >  tools/perf/util/symbol.c      |   97 +++++++++++++++
> >  tools/perf/util/symbol.h      |   12 ++
> >  tools/perf/util/util.h        |    1 +
> >  16 files changed, 330 insertions(+), 521 deletions(-)
> >  create mode 100644 tools/perf/util/debug.c
> >  create mode 100644 tools/perf/util/debug.h
> >  create mode 100644 tools/perf/util/event.h
> >  create mode 100644 tools/perf/util/map.c
> 
> Re-pulled it - it works fine here too - thanks!
> 
> 	Ingo


Cool, thanks.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [GIT PULL v2] perf tools: Factorize common code
  2009-08-12 10:48       ` [GIT PULL v2] perf tools: Factorize common code Frederic Weisbecker
  2009-08-12 10:54         ` Ingo Molnar
@ 2009-08-12 13:54         ` Arnaldo Carvalho de Melo
  2009-08-12 14:31           ` Ingo Molnar
  1 sibling, 1 reply; 17+ messages in thread
From: Arnaldo Carvalho de Melo @ 2009-08-12 13:54 UTC (permalink / raw)
  To: Frederic Weisbecker
  Cc: Ingo Molnar, LKML, Peter Zijlstra, Mike Galbraith, Brice Goglin

Em Wed, Aug 12, 2009 at 12:48:29PM +0200, Frederic Weisbecker escreveu:
> On Wed, Aug 12, 2009 at 12:08:51PM +0200, Ingo Molnar wrote:
> > yeah. I've zapped those commits from perfcounters/core.
> >
> >       Ingo
> 
> Ok, sorry about that.
> I've rebuilt a clean tree. It should be fine now.
> 
> Thanks.

Thanks a lot for doing this work, I intended to do that but got
sidetracked with symbol resolving neverending corner cases 8)

- Arnaldo

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [GIT PULL v2] perf tools: Factorize common code
  2009-08-12 13:54         ` Arnaldo Carvalho de Melo
@ 2009-08-12 14:31           ` Ingo Molnar
  0 siblings, 0 replies; 17+ messages in thread
From: Ingo Molnar @ 2009-08-12 14:31 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Frederic Weisbecker, LKML, Peter Zijlstra, Mike Galbraith,
	Brice Goglin


* Arnaldo Carvalho de Melo <acme@redhat.com> wrote:

> Em Wed, Aug 12, 2009 at 12:48:29PM +0200, Frederic Weisbecker escreveu:
> > On Wed, Aug 12, 2009 at 12:08:51PM +0200, Ingo Molnar wrote:
> > > yeah. I've zapped those commits from perfcounters/core.
> > >
> > >       Ingo
> > 
> > Ok, sorry about that.
> > I've rebuilt a clean tree. It should be fine now.
> > 
> > Thanks.
> 
> Thanks a lot for doing this work, I intended to do that but got 
> sidetracked with symbol resolving neverending corner cases 8)

:)

Also, there's still quite a bit of code left to be factorized out of 
builtin-annotate.c and builtin-report.c.

	Ingo

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 1/3 v2] perf tools: Factorize high level dso helpers
  2009-08-12 10:08     ` Ingo Molnar
  2009-08-12 10:48       ` [GIT PULL v2] perf tools: Factorize common code Frederic Weisbecker
@ 2009-08-12 10:48       ` Frederic Weisbecker
  2009-08-12 10:48       ` [PATCH 2/3 v2] perf tools: Factorize the event structure definitions in a single file Frederic Weisbecker
  2009-08-12 10:48       ` [PATCH 3/3 v2] perf tools: Factorize the map helpers Frederic Weisbecker
  3 siblings, 0 replies; 17+ messages in thread
From: Frederic Weisbecker @ 2009-08-12 10:48 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: LKML, Frederic Weisbecker, Arnaldo Carvalho de Melo,
	Peter Zijlstra, Mike Galbraith, Brice Goglin

Factorize multiple definitions of high level dso helpers into the
symbol source file.

The side effect is a general export of the verbose and eprintf
debugging helpers into a new file dedicated to debugging purposes.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Brice Goglin <Brice.Goglin@inria.fr>
---
 tools/perf/Makefile           |    1 +
 tools/perf/builtin-annotate.c |   96 ----------------------------------------
 tools/perf/builtin-record.c   |    1 -
 tools/perf/builtin-report.c   |   97 -----------------------------------------
 tools/perf/builtin-stat.c     |    1 -
 tools/perf/builtin-top.c      |    4 --
 tools/perf/builtin.h          |    1 +
 tools/perf/perf.h             |    1 +
 tools/perf/util/debug.c       |   22 +++++++++
 tools/perf/util/debug.h       |    5 ++
 tools/perf/util/symbol.c      |   97 +++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/symbol.h      |   11 +++++
 12 files changed, 138 insertions(+), 199 deletions(-)
 create mode 100644 tools/perf/util/debug.c
 create mode 100644 tools/perf/util/debug.h

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index de7beac..2aee21b 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -339,6 +339,7 @@ LIB_OBJS += util/pager.o
 LIB_OBJS += util/header.o
 LIB_OBJS += util/callchain.o
 LIB_OBJS += util/values.o
+LIB_OBJS += util/debug.o
 
 BUILTIN_OBJS += builtin-annotate.o
 BUILTIN_OBJS += builtin-help.o
diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 1dba568..1a79299 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -26,7 +26,6 @@
 #define SHOW_HV		4
 
 static char		const *input_name = "perf.data";
-static char		*vmlinux = "vmlinux";
 
 static char		default_sort_order[] = "comm,symbol";
 static char		*sort_order = default_sort_order;
@@ -37,9 +36,6 @@ static int		show_mask = SHOW_KERNEL | SHOW_USER | SHOW_HV;
 static int		dump_trace = 0;
 #define dprintf(x...)	do { if (dump_trace) printf(x); } while (0)
 
-static int		verbose;
-
-static int		modules;
 
 static int		full_paths;
 
@@ -89,98 +85,6 @@ struct sym_ext {
 	char		*path;
 };
 
-static LIST_HEAD(dsos);
-static struct dso *kernel_dso;
-static struct dso *vdso;
-
-
-static void dsos__add(struct dso *dso)
-{
-	list_add_tail(&dso->node, &dsos);
-}
-
-static struct dso *dsos__find(const char *name)
-{
-	struct dso *pos;
-
-	list_for_each_entry(pos, &dsos, node)
-		if (strcmp(pos->name, name) == 0)
-			return pos;
-	return NULL;
-}
-
-static struct dso *dsos__findnew(const char *name)
-{
-	struct dso *dso = dsos__find(name);
-	int nr;
-
-	if (dso)
-		return dso;
-
-	dso = dso__new(name, 0);
-	if (!dso)
-		goto out_delete_dso;
-
-	nr = dso__load(dso, NULL, verbose);
-	if (nr < 0) {
-		if (verbose)
-			fprintf(stderr, "Failed to open: %s\n", name);
-		goto out_delete_dso;
-	}
-	if (!nr && verbose) {
-		fprintf(stderr,
-		"No symbols found in: %s, maybe install a debug package?\n",
-				name);
-	}
-
-	dsos__add(dso);
-
-	return dso;
-
-out_delete_dso:
-	dso__delete(dso);
-	return NULL;
-}
-
-static void dsos__fprintf(FILE *fp)
-{
-	struct dso *pos;
-
-	list_for_each_entry(pos, &dsos, node)
-		dso__fprintf(pos, fp);
-}
-
-static struct symbol *vdso__find_symbol(struct dso *dso, u64 ip)
-{
-	return dso__find_symbol(dso, ip);
-}
-
-static int load_kernel(void)
-{
-	int err;
-
-	kernel_dso = dso__new("[kernel]", 0);
-	if (!kernel_dso)
-		return -1;
-
-	err = dso__load_kernel(kernel_dso, vmlinux, NULL, verbose, modules);
-	if (err <= 0) {
-		dso__delete(kernel_dso);
-		kernel_dso = NULL;
-	} else
-		dsos__add(kernel_dso);
-
-	vdso = dso__new("[vdso]", 0);
-	if (!vdso)
-		return -1;
-
-	vdso->find_symbol = vdso__find_symbol;
-
-	dsos__add(vdso);
-
-	return err;
-}
-
 struct map {
 	struct list_head node;
 	u64	 start;
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 0345aad..afae387 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -40,7 +40,6 @@ static int			inherit				= 1;
 static int			force				= 0;
 static int			append_file			= 0;
 static int			call_graph			= 0;
-static int			verbose				= 0;
 static int			inherit_stat			= 0;
 static int			no_samples			= 0;
 static int			sample_address			= 0;
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 2357c66..827eab2 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -30,7 +30,6 @@
 #define SHOW_HV		4
 
 static char		const *input_name = "perf.data";
-static char		*vmlinux = NULL;
 
 static char		default_sort_order[] = "comm,dso,symbol";
 static char		*sort_order = default_sort_order;
@@ -46,11 +45,6 @@ static int		dump_trace = 0;
 #define dprintf(x...)	do { if (dump_trace) printf(x); } while (0)
 #define cdprintf(x...)	do { if (dump_trace) color_fprintf(stdout, color, x); } while (0)
 
-static int		verbose;
-#define eprintf(x...)	do { if (verbose) fprintf(stderr, x); } while (0)
-
-static int		modules;
-
 static int		full_paths;
 static int		show_nr_samples;
 
@@ -161,98 +155,7 @@ static int repsep_fprintf(FILE *fp, const char *fmt, ...)
 	return n;
 }
 
-static LIST_HEAD(dsos);
-static struct dso *kernel_dso;
-static struct dso *vdso;
-static struct dso *hypervisor_dso;
-
-static void dsos__add(struct dso *dso)
-{
-	list_add_tail(&dso->node, &dsos);
-}
-
-static struct dso *dsos__find(const char *name)
-{
-	struct dso *pos;
-
-	list_for_each_entry(pos, &dsos, node)
-		if (strcmp(pos->name, name) == 0)
-			return pos;
-	return NULL;
-}
-
-static struct dso *dsos__findnew(const char *name)
-{
-	struct dso *dso = dsos__find(name);
-	int nr;
-
-	if (dso)
-		return dso;
-
-	dso = dso__new(name, 0);
-	if (!dso)
-		goto out_delete_dso;
-
-	nr = dso__load(dso, NULL, verbose);
-	if (nr < 0) {
-		eprintf("Failed to open: %s\n", name);
-		goto out_delete_dso;
-	}
-	if (!nr)
-		eprintf("No symbols found in: %s, maybe install a debug package?\n", name);
-
-	dsos__add(dso);
-
-	return dso;
-
-out_delete_dso:
-	dso__delete(dso);
-	return NULL;
-}
-
-static void dsos__fprintf(FILE *fp)
-{
-	struct dso *pos;
-
-	list_for_each_entry(pos, &dsos, node)
-		dso__fprintf(pos, fp);
-}
-
-static struct symbol *vdso__find_symbol(struct dso *dso, u64 ip)
-{
-	return dso__find_symbol(dso, ip);
-}
-
-static int load_kernel(void)
-{
-	int err;
-
-	kernel_dso = dso__new("[kernel]", 0);
-	if (!kernel_dso)
-		return -1;
-
-	err = dso__load_kernel(kernel_dso, vmlinux, NULL, verbose, modules);
-	if (err <= 0) {
-		dso__delete(kernel_dso);
-		kernel_dso = NULL;
-	} else
-		dsos__add(kernel_dso);
-
-	vdso = dso__new("[vdso]", 0);
-	if (!vdso)
-		return -1;
 
-	vdso->find_symbol = vdso__find_symbol;
-
-	dsos__add(vdso);
-
-	hypervisor_dso = dso__new("[hypervisor]", 0);
-	if (!hypervisor_dso)
-		return -1;
-	dsos__add(hypervisor_dso);
-
-	return err;
-}
 
 static char __cwd[PATH_MAX];
 static char *cwd = __cwd;
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index b4b06c7..4b9dd4a 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -63,7 +63,6 @@ static struct perf_counter_attr default_attrs[] = {
 #define MAX_RUN			100
 
 static int			system_wide			=  0;
-static int			verbose				=  0;
 static unsigned int		nr_cpus				=  0;
 static int			run_idx				=  0;
 
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 7de28ce..0aa5673 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -68,8 +68,6 @@ static int			group				=  0;
 static unsigned int		page_size;
 static unsigned int		mmap_pages			= 16;
 static int			freq				=  0;
-static int			verbose				=  0;
-static char			*vmlinux			=  NULL;
 
 static int			delay_secs			=  2;
 static int			zero;
@@ -338,8 +336,6 @@ static void show_details(struct sym_entry *syme)
 		printf("%d lines not displayed, maybe increase display entries [e]\n", more);
 }
 
-struct dso			*kernel_dso;
-
 /*
  * Symbols will be added here in record_ip and will get out
  * after decayed.
diff --git a/tools/perf/builtin.h b/tools/perf/builtin.h
index 51d1682..3a63e41 100644
--- a/tools/perf/builtin.h
+++ b/tools/perf/builtin.h
@@ -22,5 +22,6 @@ extern int cmd_stat(int argc, const char **argv, const char *prefix);
 extern int cmd_top(int argc, const char **argv, const char *prefix);
 extern int cmd_version(int argc, const char **argv, const char *prefix);
 extern int cmd_list(int argc, const char **argv, const char *prefix);
+extern int cmd_trace(int argc, const char **argv, const char *prefix);
 
 #endif
diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index e5148e2..f550921 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -48,6 +48,7 @@
 
 #include "../../include/linux/perf_counter.h"
 #include "util/types.h"
+#include "util/debug.h"
 
 /*
  * prctl(PR_TASK_PERF_COUNTERS_DISABLE) will (cheaply) disable all
diff --git a/tools/perf/util/debug.c b/tools/perf/util/debug.c
new file mode 100644
index 0000000..7cb8464
--- /dev/null
+++ b/tools/perf/util/debug.c
@@ -0,0 +1,22 @@
+/* For general debugging purposes */
+
+#include "../perf.h"
+#include <string.h>
+#include <stdarg.h>
+#include <stdio.h>
+
+int verbose = 0;
+
+int eprintf(const char *fmt, ...)
+{
+	va_list args;
+	int ret = 0;
+
+	if (verbose) {
+		va_start(args, fmt);
+		ret = vfprintf(stderr, fmt, args);
+		va_end(args);
+	}
+
+	return ret;
+}
diff --git a/tools/perf/util/debug.h b/tools/perf/util/debug.h
new file mode 100644
index 0000000..2ae9090
--- /dev/null
+++ b/tools/perf/util/debug.h
@@ -0,0 +1,5 @@
+/* For debugging general purposes */
+
+extern int verbose;
+
+int eprintf(const char *fmt, ...) __attribute__((format(printf, 1, 2)));
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index f1dcede..e9b13b4 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -937,6 +937,103 @@ int dso__load_kernel(struct dso *self, const char *vmlinux,
 	return err;
 }
 
+LIST_HEAD(dsos);
+struct dso	*kernel_dso;
+struct dso	*vdso;
+struct dso	*hypervisor_dso;
+
+char		*vmlinux = "vmlinux";
+int		modules;
+
+static void dsos__add(struct dso *dso)
+{
+	list_add_tail(&dso->node, &dsos);
+}
+
+static struct dso *dsos__find(const char *name)
+{
+	struct dso *pos;
+
+	list_for_each_entry(pos, &dsos, node)
+		if (strcmp(pos->name, name) == 0)
+			return pos;
+	return NULL;
+}
+
+struct dso *dsos__findnew(const char *name)
+{
+	struct dso *dso = dsos__find(name);
+	int nr;
+
+	if (dso)
+		return dso;
+
+	dso = dso__new(name, 0);
+	if (!dso)
+		goto out_delete_dso;
+
+	nr = dso__load(dso, NULL, verbose);
+	if (nr < 0) {
+		eprintf("Failed to open: %s\n", name);
+		goto out_delete_dso;
+	}
+	if (!nr)
+		eprintf("No symbols found in: %s, maybe install a debug package?\n", name);
+
+	dsos__add(dso);
+
+	return dso;
+
+out_delete_dso:
+	dso__delete(dso);
+	return NULL;
+}
+
+void dsos__fprintf(FILE *fp)
+{
+	struct dso *pos;
+
+	list_for_each_entry(pos, &dsos, node)
+		dso__fprintf(pos, fp);
+}
+
+static struct symbol *vdso__find_symbol(struct dso *dso, u64 ip)
+{
+	return dso__find_symbol(dso, ip);
+}
+
+int load_kernel(void)
+{
+	int err;
+
+	kernel_dso = dso__new("[kernel]", 0);
+	if (!kernel_dso)
+		return -1;
+
+	err = dso__load_kernel(kernel_dso, vmlinux, NULL, verbose, modules);
+	if (err <= 0) {
+		dso__delete(kernel_dso);
+		kernel_dso = NULL;
+	} else
+		dsos__add(kernel_dso);
+
+	vdso = dso__new("[vdso]", 0);
+	if (!vdso)
+		return -1;
+
+	vdso->find_symbol = vdso__find_symbol;
+
+	dsos__add(vdso);
+
+	hypervisor_dso = dso__new("[hypervisor]", 0);
+	if (!hypervisor_dso)
+		return -1;
+	dsos__add(hypervisor_dso);
+
+	return err;
+}
+
+
 void symbol__init(void)
 {
 	elf_version(EV_CURRENT);
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 1e003ec..f3490fc 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -48,9 +48,20 @@ int dso__load_kernel(struct dso *self, const char *vmlinux,
 		     symbol_filter_t filter, int verbose, int modules);
 int dso__load_modules(struct dso *self, symbol_filter_t filter, int verbose);
 int dso__load(struct dso *self, symbol_filter_t filter, int verbose);
+struct dso *dsos__findnew(const char *name);
+void dsos__fprintf(FILE *fp);
 
 size_t dso__fprintf(struct dso *self, FILE *fp);
 char dso__symtab_origin(const struct dso *self);
 
+int load_kernel(void);
+
 void symbol__init(void);
+
+extern struct list_head dsos;
+extern struct dso *kernel_dso;
+extern struct dso *vdso;
+extern struct dso *hypervisor_dso;
+extern char *vmlinux;
+extern int   modules;
 #endif /* _PERF_SYMBOL_ */
-- 
1.6.2.3


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 2/3 v2] perf tools: Factorize the event structure definitions in a single file
  2009-08-12 10:08     ` Ingo Molnar
  2009-08-12 10:48       ` [GIT PULL v2] perf tools: Factorize common code Frederic Weisbecker
  2009-08-12 10:48       ` [PATCH 1/3 v2] perf tools: Factorize high level dso helpers Frederic Weisbecker
@ 2009-08-12 10:48       ` Frederic Weisbecker
  2009-08-12 10:48       ` [PATCH 3/3 v2] perf tools: Factorize the map helpers Frederic Weisbecker
  3 siblings, 0 replies; 17+ messages in thread
From: Frederic Weisbecker @ 2009-08-12 10:48 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: LKML, Frederic Weisbecker, Arnaldo Carvalho de Melo,
	Peter Zijlstra, Mike Galbraith, Brice Goglin

Factorize the multiple definition of the events structures into a
single util/event.h file.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Brice Goglin <Brice.Goglin@inria.fr>
---
 tools/perf/builtin-annotate.c |   34 -------------------------
 tools/perf/builtin-record.c   |   18 -------------
 tools/perf/builtin-report.c   |   53 ----------------------------------------
 tools/perf/builtin-top.c      |   20 ---------------
 tools/perf/util/event.h       |   54 +++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/util.h        |    2 +
 6 files changed, 56 insertions(+), 125 deletions(-)
 create mode 100644 tools/perf/util/event.h

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 1a79299..fee663a 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -44,40 +44,6 @@ static int		print_line;
 static unsigned long	page_size;
 static unsigned long	mmap_window = 32;
 
-struct ip_event {
-	struct perf_event_header header;
-	u64 ip;
-	u32 pid, tid;
-};
-
-struct mmap_event {
-	struct perf_event_header header;
-	u32 pid, tid;
-	u64 start;
-	u64 len;
-	u64 pgoff;
-	char filename[PATH_MAX];
-};
-
-struct comm_event {
-	struct perf_event_header header;
-	u32 pid, tid;
-	char comm[16];
-};
-
-struct fork_event {
-	struct perf_event_header header;
-	u32 pid, ppid;
-};
-
-typedef union event_union {
-	struct perf_event_header	header;
-	struct ip_event			ip;
-	struct mmap_event		mmap;
-	struct comm_event		comm;
-	struct fork_event		fork;
-} event_t;
-
 
 struct sym_ext {
 	struct rb_node	node;
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index afae387..718b8f7 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -59,24 +59,6 @@ static int			file_new = 1;
 
 struct perf_header		*header;
 
-struct mmap_event {
-	struct perf_event_header	header;
-	u32				pid;
-	u32				tid;
-	u64				start;
-	u64				len;
-	u64				pgoff;
-	char				filename[PATH_MAX];
-};
-
-struct comm_event {
-	struct perf_event_header	header;
-	u32				pid;
-	u32				tid;
-	char				comm[16];
-};
-
-
 struct mmap_data {
 	int			counter;
 	void			*base;
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 827eab2..1efefcc 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -75,59 +75,6 @@ struct callchain_param	callchain_param = {
 
 static u64		sample_type;
 
-struct ip_event {
-	struct perf_event_header header;
-	u64 ip;
-	u32 pid, tid;
-	unsigned char __more_data[];
-};
-
-struct mmap_event {
-	struct perf_event_header header;
-	u32 pid, tid;
-	u64 start;
-	u64 len;
-	u64 pgoff;
-	char filename[PATH_MAX];
-};
-
-struct comm_event {
-	struct perf_event_header header;
-	u32 pid, tid;
-	char comm[16];
-};
-
-struct fork_event {
-	struct perf_event_header header;
-	u32 pid, ppid;
-	u32 tid, ptid;
-};
-
-struct lost_event {
-	struct perf_event_header header;
-	u64 id;
-	u64 lost;
-};
-
-struct read_event {
-	struct perf_event_header header;
-	u32 pid,tid;
-	u64 value;
-	u64 time_enabled;
-	u64 time_running;
-	u64 id;
-};
-
-typedef union event_union {
-	struct perf_event_header	header;
-	struct ip_event			ip;
-	struct mmap_event		mmap;
-	struct comm_event		comm;
-	struct fork_event		fork;
-	struct lost_event		lost;
-	struct read_event		read;
-} event_t;
-
 static int repsep_fprintf(FILE *fp, const char *fmt, ...)
 {
 	int n;
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 0aa5673..9a6dbbf 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -933,26 +933,6 @@ static void mmap_read_counter(struct mmap_data *md)
 	last_read = this_read;
 
 	for (; old != head;) {
-		struct ip_event {
-			struct perf_event_header header;
-			u64 ip;
-			u32 pid, target_pid;
-		};
-		struct mmap_event {
-			struct perf_event_header header;
-			u32 pid, target_pid;
-			u64 start;
-			u64 len;
-			u64 pgoff;
-			char filename[PATH_MAX];
-		};
-
-		typedef union event_union {
-			struct perf_event_header header;
-			struct ip_event ip;
-			struct mmap_event mmap;
-		} event_t;
-
 		event_t *event = (event_t *)&data[old & md->mask];
 
 		event_t event_copy;
diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
new file mode 100644
index 0000000..91e2fe5
--- /dev/null
+++ b/tools/perf/util/event.h
@@ -0,0 +1,54 @@
+#include "../perf.h"
+
+struct ip_event {
+	struct perf_event_header header;
+	u64 ip;
+	u32 pid, tid;
+	unsigned char __more_data[];
+};
+
+struct mmap_event {
+	struct perf_event_header header;
+	u32 pid, tid;
+	u64 start;
+	u64 len;
+	u64 pgoff;
+	char filename[PATH_MAX];
+};
+
+struct comm_event {
+	struct perf_event_header header;
+	u32 pid, tid;
+	char comm[16];
+};
+
+struct fork_event {
+	struct perf_event_header header;
+	u32 pid, ppid;
+	u32 tid, ptid;
+};
+
+struct lost_event {
+	struct perf_event_header header;
+	u64 id;
+	u64 lost;
+};
+
+struct read_event {
+	struct perf_event_header header;
+	u32 pid,tid;
+	u64 value;
+	u64 time_enabled;
+	u64 time_running;
+	u64 id;
+};
+
+typedef union event_union {
+	struct perf_event_header	header;
+	struct ip_event			ip;
+	struct mmap_event		mmap;
+	struct comm_event		comm;
+	struct fork_event		fork;
+	struct lost_event		lost;
+	struct read_event		read;
+} event_t;
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index 68fe157..be4b52c 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -83,6 +83,8 @@
 #include <inttypes.h>
 #include "../../../include/linux/magic.h"
 
+#include "event.h"
+
 #ifndef NO_ICONV
 #include <iconv.h>
 #endif
-- 
1.6.2.3


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 3/3 v2] perf tools: Factorize the map helpers
  2009-08-12 10:08     ` Ingo Molnar
                         ` (2 preceding siblings ...)
  2009-08-12 10:48       ` [PATCH 2/3 v2] perf tools: Factorize the event structure definitions in a single file Frederic Weisbecker
@ 2009-08-12 10:48       ` Frederic Weisbecker
  3 siblings, 0 replies; 17+ messages in thread
From: Frederic Weisbecker @ 2009-08-12 10:48 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: LKML, Frederic Weisbecker, Arnaldo Carvalho de Melo,
	Peter Zijlstra, Mike Galbraith, Brice Goglin

Factorize the dso mapping helpers into a single purpose common file
"util/map.c"

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Brice Goglin <Brice.Goglin@inria.fr>
---
 tools/perf/Makefile           |    1 +
 tools/perf/builtin-annotate.c |   79 +--------------------------
 tools/perf/builtin-record.c   |    1 +
 tools/perf/builtin-report.c   |  124 ++---------------------------------------
 tools/perf/util/callchain.h   |    1 +
 tools/perf/util/event.h       |   30 ++++++++++
 tools/perf/util/map.c         |   97 ++++++++++++++++++++++++++++++++
 tools/perf/util/symbol.h      |    1 +
 tools/perf/util/util.h        |    1 -
 9 files changed, 137 insertions(+), 198 deletions(-)
 create mode 100644 tools/perf/util/map.c

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 2aee21b..cb9033d 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -340,6 +340,7 @@ LIB_OBJS += util/header.o
 LIB_OBJS += util/callchain.o
 LIB_OBJS += util/values.o
 LIB_OBJS += util/debug.o
+LIB_OBJS += util/map.o
 
 BUILTIN_OBJS += builtin-annotate.o
 BUILTIN_OBJS += builtin-help.o
diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index fee663a..543c452 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -51,83 +51,6 @@ struct sym_ext {
 	char		*path;
 };
 
-struct map {
-	struct list_head node;
-	u64	 start;
-	u64	 end;
-	u64	 pgoff;
-	u64	 (*map_ip)(struct map *, u64);
-	struct dso	 *dso;
-};
-
-static u64 map__map_ip(struct map *map, u64 ip)
-{
-	return ip - map->start + map->pgoff;
-}
-
-static u64 vdso__map_ip(struct map *map __used, u64 ip)
-{
-	return ip;
-}
-
-static struct map *map__new(struct mmap_event *event)
-{
-	struct map *self = malloc(sizeof(*self));
-
-	if (self != NULL) {
-		const char *filename = event->filename;
-
-		self->start = event->start;
-		self->end   = event->start + event->len;
-		self->pgoff = event->pgoff;
-
-		self->dso = dsos__findnew(filename);
-		if (self->dso == NULL)
-			goto out_delete;
-
-		if (self->dso == vdso)
-			self->map_ip = vdso__map_ip;
-		else
-			self->map_ip = map__map_ip;
-	}
-	return self;
-out_delete:
-	free(self);
-	return NULL;
-}
-
-static struct map *map__clone(struct map *self)
-{
-	struct map *map = malloc(sizeof(*self));
-
-	if (!map)
-		return NULL;
-
-	memcpy(map, self, sizeof(*self));
-
-	return map;
-}
-
-static int map__overlap(struct map *l, struct map *r)
-{
-	if (l->start > r->start) {
-		struct map *t = l;
-		l = r;
-		r = t;
-	}
-
-	if (l->end > r->start)
-		return 1;
-
-	return 0;
-}
-
-static size_t map__fprintf(struct map *self, FILE *fp)
-{
-	return fprintf(fp, " %Lx-%Lx %Lx %s\n",
-		       self->start, self->end, self->pgoff, self->dso->name);
-}
-
 
 struct thread {
 	struct rb_node	 rb_node;
@@ -797,7 +720,7 @@ static int
 process_mmap_event(event_t *event, unsigned long offset, unsigned long head)
 {
 	struct thread *thread = threads__findnew(event->mmap.pid);
-	struct map *map = map__new(&event->mmap);
+	struct map *map = map__new(&event->mmap, NULL, 0);
 
 	dprintf("%p [%p]: PERF_EVENT_MMAP %d: [%p(%p) @ %p]: %s\n",
 		(void *)(offset + head),
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 718b8f7..106c6ab 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -15,6 +15,7 @@
 #include "util/string.h"
 
 #include "util/header.h"
+#include "util/event.h"
 
 #include <unistd.h>
 #include <sched.h>
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 1efefcc..93945ec 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -67,6 +67,10 @@ static char		callchain_default_opt[] = "fractal,0.5";
 
 static int		callchain;
 
+static char		__cwd[PATH_MAX];
+static char		*cwd = __cwd;
+static int		cwdlen;
+
 static
 struct callchain_param	callchain_param = {
 	.mode	= CHAIN_GRAPH_REL,
@@ -102,124 +106,6 @@ static int repsep_fprintf(FILE *fp, const char *fmt, ...)
 	return n;
 }
 
-
-
-static char __cwd[PATH_MAX];
-static char *cwd = __cwd;
-static int cwdlen;
-
-static int strcommon(const char *pathname)
-{
-	int n = 0;
-
-	while (n < cwdlen && pathname[n] == cwd[n])
-		++n;
-
-	return n;
-}
-
-struct map {
-	struct list_head node;
-	u64	 start;
-	u64	 end;
-	u64	 pgoff;
-	u64	 (*map_ip)(struct map *, u64);
-	struct dso	 *dso;
-};
-
-static u64 map__map_ip(struct map *map, u64 ip)
-{
-	return ip - map->start + map->pgoff;
-}
-
-static u64 vdso__map_ip(struct map *map __used, u64 ip)
-{
-	return ip;
-}
-
-static inline int is_anon_memory(const char *filename)
-{
-	return strcmp(filename, "//anon") == 0;
-}
-
-static struct map *map__new(struct mmap_event *event)
-{
-	struct map *self = malloc(sizeof(*self));
-
-	if (self != NULL) {
-		const char *filename = event->filename;
-		char newfilename[PATH_MAX];
-		int anon;
-
-		if (cwd) {
-			int n = strcommon(filename);
-
-			if (n == cwdlen) {
-				snprintf(newfilename, sizeof(newfilename),
-					 ".%s", filename + n);
-				filename = newfilename;
-			}
-		}
-
-		anon = is_anon_memory(filename);
-
-		if (anon) {
-			snprintf(newfilename, sizeof(newfilename), "/tmp/perf-%d.map", event->pid);
-			filename = newfilename;
-		}
-
-		self->start = event->start;
-		self->end   = event->start + event->len;
-		self->pgoff = event->pgoff;
-
-		self->dso = dsos__findnew(filename);
-		if (self->dso == NULL)
-			goto out_delete;
-
-		if (self->dso == vdso || anon)
-			self->map_ip = vdso__map_ip;
-		else
-			self->map_ip = map__map_ip;
-	}
-	return self;
-out_delete:
-	free(self);
-	return NULL;
-}
-
-static struct map *map__clone(struct map *self)
-{
-	struct map *map = malloc(sizeof(*self));
-
-	if (!map)
-		return NULL;
-
-	memcpy(map, self, sizeof(*self));
-
-	return map;
-}
-
-static int map__overlap(struct map *l, struct map *r)
-{
-	if (l->start > r->start) {
-		struct map *t = l;
-		l = r;
-		r = t;
-	}
-
-	if (l->end > r->start)
-		return 1;
-
-	return 0;
-}
-
-static size_t map__fprintf(struct map *self, FILE *fp)
-{
-	return fprintf(fp, " %Lx-%Lx %Lx %s\n",
-		       self->start, self->end, self->pgoff, self->dso->name);
-}
-
-
 struct thread {
 	struct rb_node	 rb_node;
 	struct list_head maps;
@@ -1474,7 +1360,7 @@ static int
 process_mmap_event(event_t *event, unsigned long offset, unsigned long head)
 {
 	struct thread *thread = threads__findnew(event->mmap.pid);
-	struct map *map = map__new(&event->mmap);
+	struct map *map = map__new(&event->mmap, cwd, cwdlen);
 
 	dprintf("%p [%p]: PERF_EVENT_MMAP %d: [%p(%p) @ %p]: %s\n",
 		(void *)(offset + head),
diff --git a/tools/perf/util/callchain.h b/tools/perf/util/callchain.h
index a926ae4..43cf3ea 100644
--- a/tools/perf/util/callchain.h
+++ b/tools/perf/util/callchain.h
@@ -4,6 +4,7 @@
 #include "../perf.h"
 #include <linux/list.h>
 #include <linux/rbtree.h>
+#include "util.h"
 #include "symbol.h"
 
 enum chain_mode {
diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
index 91e2fe5..d26dc88 100644
--- a/tools/perf/util/event.h
+++ b/tools/perf/util/event.h
@@ -1,4 +1,8 @@
+#ifndef __PERF_EVENT_H
+#define __PERF_EVENT_H
 #include "../perf.h"
+#include "util.h"
+#include <linux/list.h>
 
 struct ip_event {
 	struct perf_event_header header;
@@ -52,3 +56,29 @@ typedef union event_union {
 	struct lost_event		lost;
 	struct read_event		read;
 } event_t;
+
+struct map {
+	struct list_head	node;
+	u64			start;
+	u64			end;
+	u64			pgoff;
+	u64			(*map_ip)(struct map *, u64);
+	struct dso		*dso;
+};
+
+static inline u64 map__map_ip(struct map *map, u64 ip)
+{
+	return ip - map->start + map->pgoff;
+}
+
+static inline u64 vdso__map_ip(struct map *map __used, u64 ip)
+{
+	return ip;
+}
+
+struct map *map__new(struct mmap_event *event, char *cwd, int cwdlen);
+struct map *map__clone(struct map *self);
+int map__overlap(struct map *l, struct map *r);
+size_t map__fprintf(struct map *self, FILE *fp);
+
+#endif
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
new file mode 100644
index 0000000..804e023
--- /dev/null
+++ b/tools/perf/util/map.c
@@ -0,0 +1,97 @@
+#include "event.h"
+#include "symbol.h"
+#include <stdlib.h>
+#include <string.h>
+#include <stdio.h>
+
+static inline int is_anon_memory(const char *filename)
+{
+	return strcmp(filename, "//anon") == 0;
+}
+
+static int strcommon(const char *pathname, char *cwd, int cwdlen)
+{
+	int n = 0;
+
+	while (n < cwdlen && pathname[n] == cwd[n])
+		++n;
+
+	return n;
+}
+
+ struct map *map__new(struct mmap_event *event, char *cwd, int cwdlen)
+{
+	struct map *self = malloc(sizeof(*self));
+
+	if (self != NULL) {
+		const char *filename = event->filename;
+		char newfilename[PATH_MAX];
+		int anon;
+
+		if (cwd) {
+			int n = strcommon(filename, cwd, cwdlen);
+
+			if (n == cwdlen) {
+				snprintf(newfilename, sizeof(newfilename),
+					 ".%s", filename + n);
+				filename = newfilename;
+			}
+		}
+
+		anon = is_anon_memory(filename);
+
+		if (anon) {
+			snprintf(newfilename, sizeof(newfilename), "/tmp/perf-%d.map", event->pid);
+			filename = newfilename;
+		}
+
+		self->start = event->start;
+		self->end   = event->start + event->len;
+		self->pgoff = event->pgoff;
+
+		self->dso = dsos__findnew(filename);
+		if (self->dso == NULL)
+			goto out_delete;
+
+		if (self->dso == vdso || anon)
+			self->map_ip = vdso__map_ip;
+		else
+			self->map_ip = map__map_ip;
+	}
+	return self;
+out_delete:
+	free(self);
+	return NULL;
+}
+
+struct map *map__clone(struct map *self)
+{
+	struct map *map = malloc(sizeof(*self));
+
+	if (!map)
+		return NULL;
+
+	memcpy(map, self, sizeof(*self));
+
+	return map;
+}
+
+int map__overlap(struct map *l, struct map *r)
+{
+	if (l->start > r->start) {
+		struct map *t = l;
+		l = r;
+		r = t;
+	}
+
+	if (l->end > r->start)
+		return 1;
+
+	return 0;
+}
+
+size_t map__fprintf(struct map *self, FILE *fp)
+{
+	return fprintf(fp, " %Lx-%Lx %Lx %s\n",
+		       self->start, self->end, self->pgoff, self->dso->name);
+}
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index f3490fc..50f7235 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -6,6 +6,7 @@
 #include <linux/list.h>
 #include <linux/rbtree.h>
 #include "module.h"
+#include "event.h"
 
 struct symbol {
 	struct rb_node	rb_node;
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index be4b52c..d61a6f0 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -83,7 +83,6 @@
 #include <inttypes.h>
 #include "../../../include/linux/magic.h"
 
-#include "event.h"
 
 #ifndef NO_ICONV
 #include <iconv.h>
-- 
1.6.2.3


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 2/3] perf tools: Factorize the event structure definitions in a single file
  2009-08-12  9:25 [GIT PULL] perf tools: Factorize common code Frederic Weisbecker
  2009-08-12  9:26 ` [PATCH 1/3] perf tools: Factorize high level dso helpers Frederic Weisbecker
@ 2009-08-12  9:26 ` Frederic Weisbecker
  2009-08-12  9:26 ` [PATCH 3/3] perf tools: Factorize the map helpers Frederic Weisbecker
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 17+ messages in thread
From: Frederic Weisbecker @ 2009-08-12  9:26 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: LKML, Frederic Weisbecker, Arnaldo Carvalho de Melo,
	Peter Zijlstra, Mike Galbraith, Brice Goglin

Factorize the multiple definition of the events structures into a
single util/event.h file.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Brice Goglin <Brice.Goglin@inria.fr>
---
 tools/perf/builtin-annotate.c |   34 -------------------------
 tools/perf/builtin-record.c   |   18 -------------
 tools/perf/builtin-report.c   |   53 ----------------------------------------
 tools/perf/builtin-top.c      |   20 ---------------
 tools/perf/util/event.h       |   54 +++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/util.h        |    2 +
 6 files changed, 56 insertions(+), 125 deletions(-)
 create mode 100644 tools/perf/util/event.h

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 1a79299..fee663a 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -44,40 +44,6 @@ static int		print_line;
 static unsigned long	page_size;
 static unsigned long	mmap_window = 32;
 
-struct ip_event {
-	struct perf_event_header header;
-	u64 ip;
-	u32 pid, tid;
-};
-
-struct mmap_event {
-	struct perf_event_header header;
-	u32 pid, tid;
-	u64 start;
-	u64 len;
-	u64 pgoff;
-	char filename[PATH_MAX];
-};
-
-struct comm_event {
-	struct perf_event_header header;
-	u32 pid, tid;
-	char comm[16];
-};
-
-struct fork_event {
-	struct perf_event_header header;
-	u32 pid, ppid;
-};
-
-typedef union event_union {
-	struct perf_event_header	header;
-	struct ip_event			ip;
-	struct mmap_event		mmap;
-	struct comm_event		comm;
-	struct fork_event		fork;
-} event_t;
-
 
 struct sym_ext {
 	struct rb_node	node;
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index afae387..718b8f7 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -59,24 +59,6 @@ static int			file_new = 1;
 
 struct perf_header		*header;
 
-struct mmap_event {
-	struct perf_event_header	header;
-	u32				pid;
-	u32				tid;
-	u64				start;
-	u64				len;
-	u64				pgoff;
-	char				filename[PATH_MAX];
-};
-
-struct comm_event {
-	struct perf_event_header	header;
-	u32				pid;
-	u32				tid;
-	char				comm[16];
-};
-
-
 struct mmap_data {
 	int			counter;
 	void			*base;
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 827eab2..1efefcc 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -75,59 +75,6 @@ struct callchain_param	callchain_param = {
 
 static u64		sample_type;
 
-struct ip_event {
-	struct perf_event_header header;
-	u64 ip;
-	u32 pid, tid;
-	unsigned char __more_data[];
-};
-
-struct mmap_event {
-	struct perf_event_header header;
-	u32 pid, tid;
-	u64 start;
-	u64 len;
-	u64 pgoff;
-	char filename[PATH_MAX];
-};
-
-struct comm_event {
-	struct perf_event_header header;
-	u32 pid, tid;
-	char comm[16];
-};
-
-struct fork_event {
-	struct perf_event_header header;
-	u32 pid, ppid;
-	u32 tid, ptid;
-};
-
-struct lost_event {
-	struct perf_event_header header;
-	u64 id;
-	u64 lost;
-};
-
-struct read_event {
-	struct perf_event_header header;
-	u32 pid,tid;
-	u64 value;
-	u64 time_enabled;
-	u64 time_running;
-	u64 id;
-};
-
-typedef union event_union {
-	struct perf_event_header	header;
-	struct ip_event			ip;
-	struct mmap_event		mmap;
-	struct comm_event		comm;
-	struct fork_event		fork;
-	struct lost_event		lost;
-	struct read_event		read;
-} event_t;
-
 static int repsep_fprintf(FILE *fp, const char *fmt, ...)
 {
 	int n;
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 0aa5673..9a6dbbf 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -933,26 +933,6 @@ static void mmap_read_counter(struct mmap_data *md)
 	last_read = this_read;
 
 	for (; old != head;) {
-		struct ip_event {
-			struct perf_event_header header;
-			u64 ip;
-			u32 pid, target_pid;
-		};
-		struct mmap_event {
-			struct perf_event_header header;
-			u32 pid, target_pid;
-			u64 start;
-			u64 len;
-			u64 pgoff;
-			char filename[PATH_MAX];
-		};
-
-		typedef union event_union {
-			struct perf_event_header header;
-			struct ip_event ip;
-			struct mmap_event mmap;
-		} event_t;
-
 		event_t *event = (event_t *)&data[old & md->mask];
 
 		event_t event_copy;
diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
new file mode 100644
index 0000000..91e2fe5
--- /dev/null
+++ b/tools/perf/util/event.h
@@ -0,0 +1,54 @@
+#include "../perf.h"
+
+struct ip_event {
+	struct perf_event_header header;
+	u64 ip;
+	u32 pid, tid;
+	unsigned char __more_data[];
+};
+
+struct mmap_event {
+	struct perf_event_header header;
+	u32 pid, tid;
+	u64 start;
+	u64 len;
+	u64 pgoff;
+	char filename[PATH_MAX];
+};
+
+struct comm_event {
+	struct perf_event_header header;
+	u32 pid, tid;
+	char comm[16];
+};
+
+struct fork_event {
+	struct perf_event_header header;
+	u32 pid, ppid;
+	u32 tid, ptid;
+};
+
+struct lost_event {
+	struct perf_event_header header;
+	u64 id;
+	u64 lost;
+};
+
+struct read_event {
+	struct perf_event_header header;
+	u32 pid,tid;
+	u64 value;
+	u64 time_enabled;
+	u64 time_running;
+	u64 id;
+};
+
+typedef union event_union {
+	struct perf_event_header	header;
+	struct ip_event			ip;
+	struct mmap_event		mmap;
+	struct comm_event		comm;
+	struct fork_event		fork;
+	struct lost_event		lost;
+	struct read_event		read;
+} event_t;
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index 68fe157..be4b52c 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -83,6 +83,8 @@
 #include <inttypes.h>
 #include "../../../include/linux/magic.h"
 
+#include "event.h"
+
 #ifndef NO_ICONV
 #include <iconv.h>
 #endif
-- 
1.6.2.3


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 3/3] perf tools: Factorize the map helpers
  2009-08-12  9:25 [GIT PULL] perf tools: Factorize common code Frederic Weisbecker
  2009-08-12  9:26 ` [PATCH 1/3] perf tools: Factorize high level dso helpers Frederic Weisbecker
  2009-08-12  9:26 ` [PATCH 2/3] perf tools: Factorize the event structure definitions in a single file Frederic Weisbecker
@ 2009-08-12  9:26 ` Frederic Weisbecker
  2009-08-12  9:27 ` [GIT PULL] perf tools: Factorize common code Frederic Weisbecker
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 17+ messages in thread
From: Frederic Weisbecker @ 2009-08-12  9:26 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: LKML, Frederic Weisbecker, Arnaldo Carvalho de Melo,
	Peter Zijlstra, Mike Galbraith, Brice Goglin

Factorize the dso mapping helpers into a single purpose common file
"util/map.c"

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Brice Goglin <Brice.Goglin@inria.fr>
---
 tools/perf/Makefile           |    1 +
 tools/perf/builtin-annotate.c |   79 +--------------------------
 tools/perf/builtin-record.c   |    1 +
 tools/perf/builtin-report.c   |  124 ++---------------------------------------
 tools/perf/util/callchain.h   |    1 +
 tools/perf/util/event.h       |   30 ++++++++++
 tools/perf/util/map.c         |   97 ++++++++++++++++++++++++++++++++
 tools/perf/util/symbol.h      |    1 +
 tools/perf/util/util.h        |    1 -
 9 files changed, 137 insertions(+), 198 deletions(-)
 create mode 100644 tools/perf/util/map.c

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 2aee21b..f4d868c 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -340,6 +340,7 @@ LIB_OBJS += util/header.o
 LIB_OBJS += util/callchain.o
 LIB_OBJS += util/values.o
 LIB_OBJS += util/debug.o
+LIB_OBJS += util/event.o
 
 BUILTIN_OBJS += builtin-annotate.o
 BUILTIN_OBJS += builtin-help.o
diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index fee663a..543c452 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -51,83 +51,6 @@ struct sym_ext {
 	char		*path;
 };
 
-struct map {
-	struct list_head node;
-	u64	 start;
-	u64	 end;
-	u64	 pgoff;
-	u64	 (*map_ip)(struct map *, u64);
-	struct dso	 *dso;
-};
-
-static u64 map__map_ip(struct map *map, u64 ip)
-{
-	return ip - map->start + map->pgoff;
-}
-
-static u64 vdso__map_ip(struct map *map __used, u64 ip)
-{
-	return ip;
-}
-
-static struct map *map__new(struct mmap_event *event)
-{
-	struct map *self = malloc(sizeof(*self));
-
-	if (self != NULL) {
-		const char *filename = event->filename;
-
-		self->start = event->start;
-		self->end   = event->start + event->len;
-		self->pgoff = event->pgoff;
-
-		self->dso = dsos__findnew(filename);
-		if (self->dso == NULL)
-			goto out_delete;
-
-		if (self->dso == vdso)
-			self->map_ip = vdso__map_ip;
-		else
-			self->map_ip = map__map_ip;
-	}
-	return self;
-out_delete:
-	free(self);
-	return NULL;
-}
-
-static struct map *map__clone(struct map *self)
-{
-	struct map *map = malloc(sizeof(*self));
-
-	if (!map)
-		return NULL;
-
-	memcpy(map, self, sizeof(*self));
-
-	return map;
-}
-
-static int map__overlap(struct map *l, struct map *r)
-{
-	if (l->start > r->start) {
-		struct map *t = l;
-		l = r;
-		r = t;
-	}
-
-	if (l->end > r->start)
-		return 1;
-
-	return 0;
-}
-
-static size_t map__fprintf(struct map *self, FILE *fp)
-{
-	return fprintf(fp, " %Lx-%Lx %Lx %s\n",
-		       self->start, self->end, self->pgoff, self->dso->name);
-}
-
 
 struct thread {
 	struct rb_node	 rb_node;
@@ -797,7 +720,7 @@ static int
 process_mmap_event(event_t *event, unsigned long offset, unsigned long head)
 {
 	struct thread *thread = threads__findnew(event->mmap.pid);
-	struct map *map = map__new(&event->mmap);
+	struct map *map = map__new(&event->mmap, NULL, 0);
 
 	dprintf("%p [%p]: PERF_EVENT_MMAP %d: [%p(%p) @ %p]: %s\n",
 		(void *)(offset + head),
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 718b8f7..106c6ab 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -15,6 +15,7 @@
 #include "util/string.h"
 
 #include "util/header.h"
+#include "util/event.h"
 
 #include <unistd.h>
 #include <sched.h>
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 1efefcc..93945ec 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -67,6 +67,10 @@ static char		callchain_default_opt[] = "fractal,0.5";
 
 static int		callchain;
 
+static char		__cwd[PATH_MAX];
+static char		*cwd = __cwd;
+static int		cwdlen;
+
 static
 struct callchain_param	callchain_param = {
 	.mode	= CHAIN_GRAPH_REL,
@@ -102,124 +106,6 @@ static int repsep_fprintf(FILE *fp, const char *fmt, ...)
 	return n;
 }
 
-
-
-static char __cwd[PATH_MAX];
-static char *cwd = __cwd;
-static int cwdlen;
-
-static int strcommon(const char *pathname)
-{
-	int n = 0;
-
-	while (n < cwdlen && pathname[n] == cwd[n])
-		++n;
-
-	return n;
-}
-
-struct map {
-	struct list_head node;
-	u64	 start;
-	u64	 end;
-	u64	 pgoff;
-	u64	 (*map_ip)(struct map *, u64);
-	struct dso	 *dso;
-};
-
-static u64 map__map_ip(struct map *map, u64 ip)
-{
-	return ip - map->start + map->pgoff;
-}
-
-static u64 vdso__map_ip(struct map *map __used, u64 ip)
-{
-	return ip;
-}
-
-static inline int is_anon_memory(const char *filename)
-{
-	return strcmp(filename, "//anon") == 0;
-}
-
-static struct map *map__new(struct mmap_event *event)
-{
-	struct map *self = malloc(sizeof(*self));
-
-	if (self != NULL) {
-		const char *filename = event->filename;
-		char newfilename[PATH_MAX];
-		int anon;
-
-		if (cwd) {
-			int n = strcommon(filename);
-
-			if (n == cwdlen) {
-				snprintf(newfilename, sizeof(newfilename),
-					 ".%s", filename + n);
-				filename = newfilename;
-			}
-		}
-
-		anon = is_anon_memory(filename);
-
-		if (anon) {
-			snprintf(newfilename, sizeof(newfilename), "/tmp/perf-%d.map", event->pid);
-			filename = newfilename;
-		}
-
-		self->start = event->start;
-		self->end   = event->start + event->len;
-		self->pgoff = event->pgoff;
-
-		self->dso = dsos__findnew(filename);
-		if (self->dso == NULL)
-			goto out_delete;
-
-		if (self->dso == vdso || anon)
-			self->map_ip = vdso__map_ip;
-		else
-			self->map_ip = map__map_ip;
-	}
-	return self;
-out_delete:
-	free(self);
-	return NULL;
-}
-
-static struct map *map__clone(struct map *self)
-{
-	struct map *map = malloc(sizeof(*self));
-
-	if (!map)
-		return NULL;
-
-	memcpy(map, self, sizeof(*self));
-
-	return map;
-}
-
-static int map__overlap(struct map *l, struct map *r)
-{
-	if (l->start > r->start) {
-		struct map *t = l;
-		l = r;
-		r = t;
-	}
-
-	if (l->end > r->start)
-		return 1;
-
-	return 0;
-}
-
-static size_t map__fprintf(struct map *self, FILE *fp)
-{
-	return fprintf(fp, " %Lx-%Lx %Lx %s\n",
-		       self->start, self->end, self->pgoff, self->dso->name);
-}
-
-
 struct thread {
 	struct rb_node	 rb_node;
 	struct list_head maps;
@@ -1474,7 +1360,7 @@ static int
 process_mmap_event(event_t *event, unsigned long offset, unsigned long head)
 {
 	struct thread *thread = threads__findnew(event->mmap.pid);
-	struct map *map = map__new(&event->mmap);
+	struct map *map = map__new(&event->mmap, cwd, cwdlen);
 
 	dprintf("%p [%p]: PERF_EVENT_MMAP %d: [%p(%p) @ %p]: %s\n",
 		(void *)(offset + head),
diff --git a/tools/perf/util/callchain.h b/tools/perf/util/callchain.h
index a926ae4..43cf3ea 100644
--- a/tools/perf/util/callchain.h
+++ b/tools/perf/util/callchain.h
@@ -4,6 +4,7 @@
 #include "../perf.h"
 #include <linux/list.h>
 #include <linux/rbtree.h>
+#include "util.h"
 #include "symbol.h"
 
 enum chain_mode {
diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
index 91e2fe5..d26dc88 100644
--- a/tools/perf/util/event.h
+++ b/tools/perf/util/event.h
@@ -1,4 +1,8 @@
+#ifndef __PERF_EVENT_H
+#define __PERF_EVENT_H
 #include "../perf.h"
+#include "util.h"
+#include <linux/list.h>
 
 struct ip_event {
 	struct perf_event_header header;
@@ -52,3 +56,29 @@ typedef union event_union {
 	struct lost_event		lost;
 	struct read_event		read;
 } event_t;
+
+struct map {
+	struct list_head	node;
+	u64			start;
+	u64			end;
+	u64			pgoff;
+	u64			(*map_ip)(struct map *, u64);
+	struct dso		*dso;
+};
+
+static inline u64 map__map_ip(struct map *map, u64 ip)
+{
+	return ip - map->start + map->pgoff;
+}
+
+static inline u64 vdso__map_ip(struct map *map __used, u64 ip)
+{
+	return ip;
+}
+
+struct map *map__new(struct mmap_event *event, char *cwd, int cwdlen);
+struct map *map__clone(struct map *self);
+int map__overlap(struct map *l, struct map *r);
+size_t map__fprintf(struct map *self, FILE *fp);
+
+#endif
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
new file mode 100644
index 0000000..804e023
--- /dev/null
+++ b/tools/perf/util/map.c
@@ -0,0 +1,97 @@
+#include "event.h"
+#include "symbol.h"
+#include <stdlib.h>
+#include <string.h>
+#include <stdio.h>
+
+static inline int is_anon_memory(const char *filename)
+{
+	return strcmp(filename, "//anon") == 0;
+}
+
+static int strcommon(const char *pathname, char *cwd, int cwdlen)
+{
+	int n = 0;
+
+	while (n < cwdlen && pathname[n] == cwd[n])
+		++n;
+
+	return n;
+}
+
+ struct map *map__new(struct mmap_event *event, char *cwd, int cwdlen)
+{
+	struct map *self = malloc(sizeof(*self));
+
+	if (self != NULL) {
+		const char *filename = event->filename;
+		char newfilename[PATH_MAX];
+		int anon;
+
+		if (cwd) {
+			int n = strcommon(filename, cwd, cwdlen);
+
+			if (n == cwdlen) {
+				snprintf(newfilename, sizeof(newfilename),
+					 ".%s", filename + n);
+				filename = newfilename;
+			}
+		}
+
+		anon = is_anon_memory(filename);
+
+		if (anon) {
+			snprintf(newfilename, sizeof(newfilename), "/tmp/perf-%d.map", event->pid);
+			filename = newfilename;
+		}
+
+		self->start = event->start;
+		self->end   = event->start + event->len;
+		self->pgoff = event->pgoff;
+
+		self->dso = dsos__findnew(filename);
+		if (self->dso == NULL)
+			goto out_delete;
+
+		if (self->dso == vdso || anon)
+			self->map_ip = vdso__map_ip;
+		else
+			self->map_ip = map__map_ip;
+	}
+	return self;
+out_delete:
+	free(self);
+	return NULL;
+}
+
+struct map *map__clone(struct map *self)
+{
+	struct map *map = malloc(sizeof(*self));
+
+	if (!map)
+		return NULL;
+
+	memcpy(map, self, sizeof(*self));
+
+	return map;
+}
+
+int map__overlap(struct map *l, struct map *r)
+{
+	if (l->start > r->start) {
+		struct map *t = l;
+		l = r;
+		r = t;
+	}
+
+	if (l->end > r->start)
+		return 1;
+
+	return 0;
+}
+
+size_t map__fprintf(struct map *self, FILE *fp)
+{
+	return fprintf(fp, " %Lx-%Lx %Lx %s\n",
+		       self->start, self->end, self->pgoff, self->dso->name);
+}
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index f3490fc..50f7235 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -6,6 +6,7 @@
 #include <linux/list.h>
 #include <linux/rbtree.h>
 #include "module.h"
+#include "event.h"
 
 struct symbol {
 	struct rb_node	rb_node;
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index be4b52c..d61a6f0 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -83,7 +83,6 @@
 #include <inttypes.h>
 #include "../../../include/linux/magic.h"
 
-#include "event.h"
 
 #ifndef NO_ICONV
 #include <iconv.h>
-- 
1.6.2.3


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [GIT PULL] perf tools: Factorize common code
  2009-08-12  9:25 [GIT PULL] perf tools: Factorize common code Frederic Weisbecker
                   ` (2 preceding siblings ...)
  2009-08-12  9:26 ` [PATCH 3/3] perf tools: Factorize the map helpers Frederic Weisbecker
@ 2009-08-12  9:27 ` Frederic Weisbecker
  2009-08-12  9:32 ` Ingo Molnar
  2009-08-12  9:52 ` Peter Zijlstra
  5 siblings, 0 replies; 17+ messages in thread
From: Frederic Weisbecker @ 2009-08-12  9:27 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: LKML, Arnaldo Carvalho de Melo, Peter Zijlstra, Mike Galbraith,
	Brice Goglin

On Wed, Aug 12, 2009 at 11:25:59AM +0200, Frederic Weisbecker wrote:
> Ingo,
> 
> This set factorizes the common definition of structs, functions,
> variables inside perf tools to make it easier for perf trace to
> be created.
> 
> I guess this patchset only factorizes 10 % of what could be done,
> the largest part is the intersection between perf report and perf
> annotate. It's a lot of work.
> 
> I guess I will do the rest in separate patchsets while writing
> perf trace.
> 
> Thanks,
> Frederic.
> 
> 
> The following changes since commit 9f8666971185b86615a074bcac67c90fdf8af8bc:
>   Brice Goglin (1):
>         perf report: Add raw displaying of per-thread counters
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing.git ..BRANCH.NOT.VERIFIED..


s/BRANCH.NOT.VERIFIED/"perfcounters/core"


> 
> Frederic Weisbecker (3):
>       perf tools: Factorize high level dso helpers
>       perf tools: Factorize the event structure definitions in a single file
>       perf tools: Factorize the map helpers
> 
>  tools/perf/Makefile           |    2 +
>  tools/perf/builtin-annotate.c |  209 +-------------------------------
>  tools/perf/builtin-record.c   |   20 +---
>  tools/perf/builtin-report.c   |  274 +----------------------------------------
>  tools/perf/builtin-stat.c     |    1 -
>  tools/perf/builtin-top.c      |   24 ----
>  tools/perf/builtin.h          |    1 +
>  tools/perf/perf.h             |    1 +
>  tools/perf/util/callchain.h   |    1 +
>  tools/perf/util/event.h       |   84 +++++++++++++
>  tools/perf/util/map.c         |   97 +++++++++++++++
>  tools/perf/util/symbol.c      |   97 +++++++++++++++
>  tools/perf/util/symbol.h      |   12 ++
>  tools/perf/util/util.h        |    1 +
>  14 files changed, 303 insertions(+), 521 deletions(-)
>  create mode 100644 tools/perf/util/event.h
>  create mode 100644 tools/perf/util/map.c


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [GIT PULL] perf tools: Factorize common code
  2009-08-12  9:25 [GIT PULL] perf tools: Factorize common code Frederic Weisbecker
                   ` (3 preceding siblings ...)
  2009-08-12  9:27 ` [GIT PULL] perf tools: Factorize common code Frederic Weisbecker
@ 2009-08-12  9:32 ` Ingo Molnar
  2009-08-12  9:52 ` Peter Zijlstra
  5 siblings, 0 replies; 17+ messages in thread
From: Ingo Molnar @ 2009-08-12  9:32 UTC (permalink / raw)
  To: Frederic Weisbecker
  Cc: LKML, Arnaldo Carvalho de Melo, Peter Zijlstra, Mike Galbraith,
	Brice Goglin


* Frederic Weisbecker <fweisbec@gmail.com> wrote:

> Ingo,
> 
> This set factorizes the common definition of structs, functions, 
> variables inside perf tools to make it easier for perf trace to be 
> created.
> 
> I guess this patchset only factorizes 10 % of what could be done, 
> the largest part is the intersection between perf report and perf 
> annotate. It's a lot of work.
> 
> I guess I will do the rest in separate patchsets while writing 
> perf trace.

Pulled, thanks Frederic!

	Ingo

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [GIT PULL] perf tools: Factorize common code
  2009-08-12  9:25 [GIT PULL] perf tools: Factorize common code Frederic Weisbecker
                   ` (4 preceding siblings ...)
  2009-08-12  9:32 ` Ingo Molnar
@ 2009-08-12  9:52 ` Peter Zijlstra
  5 siblings, 0 replies; 17+ messages in thread
From: Peter Zijlstra @ 2009-08-12  9:52 UTC (permalink / raw)
  To: Frederic Weisbecker
  Cc: Ingo Molnar, LKML, Arnaldo Carvalho de Melo, Mike Galbraith,
	Brice Goglin

On Wed, 2009-08-12 at 11:25 +0200, Frederic Weisbecker wrote:
> Ingo,
> 
> This set factorizes the common definition of structs, functions,
> variables inside perf tools to make it easier for perf trace to
> be created.
> 
> I guess this patchset only factorizes 10 % of what could be done,
> the largest part is the intersection between perf report and perf
> annotate. It's a lot of work.
> 
> I guess I will do the rest in separate patchsets while writing
> perf trace.

Awesome, much needed cleanups.

Thanks

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2009-08-12 14:31 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-08-12  9:25 [GIT PULL] perf tools: Factorize common code Frederic Weisbecker
2009-08-12  9:26 ` [PATCH 1/3] perf tools: Factorize high level dso helpers Frederic Weisbecker
2009-08-12  9:59   ` Frederic Weisbecker
2009-08-12 10:08     ` Ingo Molnar
2009-08-12 10:48       ` [GIT PULL v2] perf tools: Factorize common code Frederic Weisbecker
2009-08-12 10:54         ` Ingo Molnar
2009-08-12 11:00           ` Frederic Weisbecker
2009-08-12 13:54         ` Arnaldo Carvalho de Melo
2009-08-12 14:31           ` Ingo Molnar
2009-08-12 10:48       ` [PATCH 1/3 v2] perf tools: Factorize high level dso helpers Frederic Weisbecker
2009-08-12 10:48       ` [PATCH 2/3 v2] perf tools: Factorize the event structure definitions in a single file Frederic Weisbecker
2009-08-12 10:48       ` [PATCH 3/3 v2] perf tools: Factorize the map helpers Frederic Weisbecker
2009-08-12  9:26 ` [PATCH 2/3] perf tools: Factorize the event structure definitions in a single file Frederic Weisbecker
2009-08-12  9:26 ` [PATCH 3/3] perf tools: Factorize the map helpers Frederic Weisbecker
2009-08-12  9:27 ` [GIT PULL] perf tools: Factorize common code Frederic Weisbecker
2009-08-12  9:32 ` Ingo Molnar
2009-08-12  9:52 ` Peter Zijlstra

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox