Re: [PATCH 2/6] perf tools: refractor code to move call path handling out of thread-stack

[Adrian Hunter <adrian.hunter@intel.com>]

On 28/04/16 11:19, Chris Phlipot wrote:
> Move the call path handling code out of thread-stack.c and thread-stack.h
> to allow other components that are not part of thread-stack to create call
> paths.
> 
> Summary:
> -Create call-path.c and call-path.h and add them to the build.
> -Move all call path related code out of thread-stack.c and thread-stack.h
> 	and into call-path.c and call-path.h.
> -A small subset of structures and functions are now visible through
> 	call-path.h, which is required for thread-stack.c to continue to
> 	compile.
> 
> This change is a prerequisite for subsequent patches in this change set
> and by itself contains no user-visible changes.
> 
> Signed-off-by: Chris Phlipot <cphlipot0@gmail.com>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  tools/perf/util/Build                              |   1 +
>  tools/perf/util/call-path.c                        | 122 ++++++++++++++++++++
>  tools/perf/util/call-path.h                        |  77 +++++++++++++
>  tools/perf/util/db-export.c                        |   1 +
>  .../util/scripting-engines/trace-event-python.c    |   1 +
>  tools/perf/util/thread-stack.c                     | 126 +--------------------
>  tools/perf/util/thread-stack.h                     |  25 +---
>  7 files changed, 204 insertions(+), 149 deletions(-)
>  create mode 100644 tools/perf/util/call-path.c
>  create mode 100644 tools/perf/util/call-path.h
> 
> diff --git a/tools/perf/util/Build b/tools/perf/util/Build
> index 90229a8..027bb2b 100644
> --- a/tools/perf/util/Build
> +++ b/tools/perf/util/Build
> @@ -74,6 +74,7 @@ libperf-y += srcline.o
>  libperf-y += data.o
>  libperf-y += tsc.o
>  libperf-y += cloexec.o
> +libperf-y += call-path.o
>  libperf-y += thread-stack.o
>  libperf-$(CONFIG_AUXTRACE) += auxtrace.o
>  libperf-$(CONFIG_AUXTRACE) += intel-pt-decoder/
> diff --git a/tools/perf/util/call-path.c b/tools/perf/util/call-path.c
> new file mode 100644
> index 0000000..904a170
> --- /dev/null
> +++ b/tools/perf/util/call-path.c
> @@ -0,0 +1,122 @@
> +/*
> + * call-path.h: Manipulate a tree data structure containing function call paths
> + * Copyright (c) 2014, Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + */
> +
> +#include <linux/rbtree.h>
> +#include <linux/list.h>
> +
> +#include "util.h"
> +#include "call-path.h"
> +
> +static void call_path__init(struct call_path *cp, struct call_path *parent,
> +			    struct symbol *sym, u64 ip, bool in_kernel)
> +{
> +	cp->parent = parent;
> +	cp->sym = sym;
> +	cp->ip = sym ? 0 : ip;
> +	cp->db_id = 0;
> +	cp->in_kernel = in_kernel;
> +	RB_CLEAR_NODE(&cp->rb_node);
> +	cp->children = RB_ROOT;
> +}
> +
> +struct call_path_root *call_path_root__new(void)
> +{
> +	struct call_path_root *cpr;
> +
> +	cpr = zalloc(sizeof(struct call_path_root));
> +	if (!cpr)
> +		return NULL;
> +	call_path__init(&cpr->call_path, NULL, NULL, 0, false);
> +	INIT_LIST_HEAD(&cpr->blocks);
> +	return cpr;
> +}
> +
> +void call_path_root__free(struct call_path_root *cpr)
> +{
> +	struct call_path_block *pos, *n;
> +
> +	list_for_each_entry_safe(pos, n, &cpr->blocks, node) {
> +		list_del(&pos->node);
> +		free(pos);
> +	}
> +	free(cpr);
> +}
> +
> +static struct call_path *call_path__new(struct call_path_root *cpr,
> +					struct call_path *parent,
> +					struct symbol *sym, u64 ip,
> +					bool in_kernel)
> +{
> +	struct call_path_block *cpb;
> +	struct call_path *cp;
> +	size_t n;
> +
> +	if (cpr->next < cpr->sz) {
> +		cpb = list_last_entry(&cpr->blocks, struct call_path_block,
> +				      node);
> +	} else {
> +		cpb = zalloc(sizeof(struct call_path_block));
> +		if (!cpb)
> +			return NULL;
> +		list_add_tail(&cpb->node, &cpr->blocks);
> +		cpr->sz += CALL_PATH_BLOCK_SIZE;
> +	}
> +
> +	n = cpr->next++ & CALL_PATH_BLOCK_MASK;
> +	cp = &cpb->cp[n];
> +
> +	call_path__init(cp, parent, sym, ip, in_kernel);
> +
> +	return cp;
> +}
> +
> +struct call_path *call_path__findnew(struct call_path_root *cpr,
> +				     struct call_path *parent,
> +				     struct symbol *sym, u64 ip, u64 ks)
> +{
> +	struct rb_node **p;
> +	struct rb_node *node_parent = NULL;
> +	struct call_path *cp;
> +	bool in_kernel = ip >= ks;
> +
> +	if (sym)
> +		ip = 0;
> +
> +	if (!parent)
> +		return call_path__new(cpr, parent, sym, ip, in_kernel);
> +
> +	p = &parent->children.rb_node;
> +	while (*p != NULL) {
> +		node_parent = *p;
> +		cp = rb_entry(node_parent, struct call_path, rb_node);
> +
> +		if (cp->sym == sym && cp->ip == ip)
> +			return cp;
> +
> +		if (sym < cp->sym || (sym == cp->sym && ip < cp->ip))
> +			p = &(*p)->rb_left;
> +		else
> +			p = &(*p)->rb_right;
> +	}
> +
> +	cp = call_path__new(cpr, parent, sym, ip, in_kernel);
> +	if (!cp)
> +		return NULL;
> +
> +	rb_link_node(&cp->rb_node, node_parent, p);
> +	rb_insert_color(&cp->rb_node, &parent->children);
> +
> +	return cp;
> +}
> diff --git a/tools/perf/util/call-path.h b/tools/perf/util/call-path.h
> new file mode 100644
> index 0000000..477f6d0
> --- /dev/null
> +++ b/tools/perf/util/call-path.h
> @@ -0,0 +1,77 @@
> +/*
> + * call-path.h: Manipulate a tree data structure containing function call paths
> + * Copyright (c) 2014, Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + */
> +
> +#ifndef __PERF_CALL_PATH_H
> +#define __PERF_CALL_PATH_H
> +
> +#include <sys/types.h>
> +
> +#include <linux/types.h>
> +#include <linux/rbtree.h>
> +
> +/**
> + * struct call_path - node in list of calls leading to a function call.
> + * @parent: call path to the parent function call
> + * @sym: symbol of function called
> + * @ip: only if sym is null, the ip of the function
> + * @db_id: id used for db-export
> + * @in_kernel: whether function is a in the kernel
> + * @rb_node: node in parent's tree of called functions
> + * @children: tree of call paths of functions called
> + *
> + * In combination with the call_return structure, the call_path structure
> + * defines a context-sensitve call-graph.
> + */
> +struct call_path {
> +	struct call_path *parent;
> +	struct symbol *sym;
> +	u64 ip;
> +	u64 db_id;
> +	bool in_kernel;
> +	struct rb_node rb_node;
> +	struct rb_root children;
> +};
> +
> +#define CALL_PATH_BLOCK_SHIFT 8
> +#define CALL_PATH_BLOCK_SIZE (1 << CALL_PATH_BLOCK_SHIFT)
> +#define CALL_PATH_BLOCK_MASK (CALL_PATH_BLOCK_SIZE - 1)
> +
> +struct call_path_block {
> +	struct call_path cp[CALL_PATH_BLOCK_SIZE];
> +	struct list_head node;
> +};
> +
> +/**
> + * struct call_path_root - root of all call paths.
> + * @call_path: root call path
> + * @blocks: list of blocks to store call paths
> + * @next: next free space
> + * @sz: number of spaces
> + */
> +struct call_path_root {
> +	struct call_path call_path;
> +	struct list_head blocks;
> +	size_t next;
> +	size_t sz;
> +};
> +
> +struct call_path_root *call_path_root__new(void);
> +void call_path_root__free(struct call_path_root *cpr);
> +
> +struct call_path *call_path__findnew(struct call_path_root *cpr,
> +				     struct call_path *parent,
> +				     struct symbol *sym, u64 ip, u64 ks);
> +
> +#endif
> diff --git a/tools/perf/util/db-export.c b/tools/perf/util/db-export.c
> index 049438d..4fc607c 100644
> --- a/tools/perf/util/db-export.c
> +++ b/tools/perf/util/db-export.c
> @@ -23,6 +23,7 @@
>  #include "event.h"
>  #include "util.h"
>  #include "thread-stack.h"
> +#include "call-path.h"
>  #include "db-export.h"
>  
>  struct deferred_export {
> diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
> index 525eb49..7bb8592 100644
> --- a/tools/perf/util/scripting-engines/trace-event-python.c
> +++ b/tools/perf/util/scripting-engines/trace-event-python.c
> @@ -41,6 +41,7 @@
>  #include "../thread-stack.h"
>  #include "../trace-event.h"
>  #include "../machine.h"
> +#include "../call-path.h"
>  #include "thread_map.h"
>  #include "cpumap.h"
>  #include "stat.h"
> diff --git a/tools/perf/util/thread-stack.c b/tools/perf/util/thread-stack.c
> index 679688e..fc419a5 100644
> --- a/tools/perf/util/thread-stack.c
> +++ b/tools/perf/util/thread-stack.c
> @@ -22,31 +22,9 @@
>  #include "debug.h"
>  #include "symbol.h"
>  #include "comm.h"
> +#include "call-path.h"
>  #include "thread-stack.h"
>  
> -#define CALL_PATH_BLOCK_SHIFT 8
> -#define CALL_PATH_BLOCK_SIZE (1 << CALL_PATH_BLOCK_SHIFT)
> -#define CALL_PATH_BLOCK_MASK (CALL_PATH_BLOCK_SIZE - 1)
> -
> -struct call_path_block {
> -	struct call_path cp[CALL_PATH_BLOCK_SIZE];
> -	struct list_head node;
> -};
> -
> -/**
> - * struct call_path_root - root of all call paths.
> - * @call_path: root call path
> - * @blocks: list of blocks to store call paths
> - * @next: next free space
> - * @sz: number of spaces
> - */
> -struct call_path_root {
> -	struct call_path call_path;
> -	struct list_head blocks;
> -	size_t next;
> -	size_t sz;
> -};
> -
>  /**
>   * struct call_return_processor - provides a call-back to consume call-return
>   *                                information.
> @@ -335,108 +313,6 @@ void thread_stack__sample(struct thread *thread, struct ip_callchain *chain,
>  		chain->ips[i] = thread->ts->stack[thread->ts->cnt - i].ret_addr;
>  }
>  
> -static void call_path__init(struct call_path *cp, struct call_path *parent,
> -			    struct symbol *sym, u64 ip, bool in_kernel)
> -{
> -	cp->parent = parent;
> -	cp->sym = sym;
> -	cp->ip = sym ? 0 : ip;
> -	cp->db_id = 0;
> -	cp->in_kernel = in_kernel;
> -	RB_CLEAR_NODE(&cp->rb_node);
> -	cp->children = RB_ROOT;
> -}
> -
> -static struct call_path_root *call_path_root__new(void)
> -{
> -	struct call_path_root *cpr;
> -
> -	cpr = zalloc(sizeof(struct call_path_root));
> -	if (!cpr)
> -		return NULL;
> -	call_path__init(&cpr->call_path, NULL, NULL, 0, false);
> -	INIT_LIST_HEAD(&cpr->blocks);
> -	return cpr;
> -}
> -
> -static void call_path_root__free(struct call_path_root *cpr)
> -{
> -	struct call_path_block *pos, *n;
> -
> -	list_for_each_entry_safe(pos, n, &cpr->blocks, node) {
> -		list_del(&pos->node);
> -		free(pos);
> -	}
> -	free(cpr);
> -}
> -
> -static struct call_path *call_path__new(struct call_path_root *cpr,
> -					struct call_path *parent,
> -					struct symbol *sym, u64 ip,
> -					bool in_kernel)
> -{
> -	struct call_path_block *cpb;
> -	struct call_path *cp;
> -	size_t n;
> -
> -	if (cpr->next < cpr->sz) {
> -		cpb = list_last_entry(&cpr->blocks, struct call_path_block,
> -				      node);
> -	} else {
> -		cpb = zalloc(sizeof(struct call_path_block));
> -		if (!cpb)
> -			return NULL;
> -		list_add_tail(&cpb->node, &cpr->blocks);
> -		cpr->sz += CALL_PATH_BLOCK_SIZE;
> -	}
> -
> -	n = cpr->next++ & CALL_PATH_BLOCK_MASK;
> -	cp = &cpb->cp[n];
> -
> -	call_path__init(cp, parent, sym, ip, in_kernel);
> -
> -	return cp;
> -}
> -
> -static struct call_path *call_path__findnew(struct call_path_root *cpr,
> -					    struct call_path *parent,
> -					    struct symbol *sym, u64 ip, u64 ks)
> -{
> -	struct rb_node **p;
> -	struct rb_node *node_parent = NULL;
> -	struct call_path *cp;
> -	bool in_kernel = ip >= ks;
> -
> -	if (sym)
> -		ip = 0;
> -
> -	if (!parent)
> -		return call_path__new(cpr, parent, sym, ip, in_kernel);
> -
> -	p = &parent->children.rb_node;
> -	while (*p != NULL) {
> -		node_parent = *p;
> -		cp = rb_entry(node_parent, struct call_path, rb_node);
> -
> -		if (cp->sym == sym && cp->ip == ip)
> -			return cp;
> -
> -		if (sym < cp->sym || (sym == cp->sym && ip < cp->ip))
> -			p = &(*p)->rb_left;
> -		else
> -			p = &(*p)->rb_right;
> -	}
> -
> -	cp = call_path__new(cpr, parent, sym, ip, in_kernel);
> -	if (!cp)
> -		return NULL;
> -
> -	rb_link_node(&cp->rb_node, node_parent, p);
> -	rb_insert_color(&cp->rb_node, &parent->children);
> -
> -	return cp;
> -}
> -
>  struct call_return_processor *
>  call_return_processor__new(int (*process)(struct call_return *cr, void *data),
>  			   void *data)
> diff --git a/tools/perf/util/thread-stack.h b/tools/perf/util/thread-stack.h
> index e1528f1..ec9bedd 100644
> --- a/tools/perf/util/thread-stack.h
> +++ b/tools/perf/util/thread-stack.h
> @@ -19,7 +19,6 @@
>  #include <sys/types.h>
>  
>  #include <linux/types.h>
> -#include <linux/rbtree.h>
>  
>  struct thread;
>  struct comm;
> @@ -30,6 +29,7 @@ struct call_return_processor;
>  struct comm;
>  struct perf_sample;
>  struct addr_location;
> +struct call_path;
>  
>  /*
>   * Call/Return flags.
> @@ -68,29 +68,6 @@ struct call_return {
>  	u32 flags;
>  };
>  
> -/**
> - * struct call_path - node in list of calls leading to a function call.
> - * @parent: call path to the parent function call
> - * @sym: symbol of function called
> - * @ip: only if sym is null, the ip of the function
> - * @db_id: id used for db-export
> - * @in_kernel: whether function is a in the kernel
> - * @rb_node: node in parent's tree of called functions
> - * @children: tree of call paths of functions called
> - *
> - * In combination with the call_return structure, the call_path structure
> - * defines a context-sensitve call-graph.
> - */
> -struct call_path {
> -	struct call_path *parent;
> -	struct symbol *sym;
> -	u64 ip;
> -	u64 db_id;
> -	bool in_kernel;
> -	struct rb_node rb_node;
> -	struct rb_root children;
> -};
> -
>  int thread_stack__event(struct thread *thread, u32 flags, u64 from_ip,
>  			u64 to_ip, u16 insn_len, u64 trace_nr);
>  void thread_stack__set_trace_nr(struct thread *thread, u64 trace_nr);
>