[PATCH 0/8] perf thread-stack: Fix thread stack processing for the idle task

[PATCH 0/8] perf thread-stack: Fix thread stack processing for the idle task

[Adrian Hunter]

Hi

Here are some patches that fix thread stack processing for the idle task.
Patches 1-6 are preparation.  The fix is patch 7.  Patch 8 is a comment to
explain the issue in perf_session__register_idle_thread().

perf creates a single 'struct thread' to represent the idle task. That is
because threads are identified by pid and tid, and the idle task always has
pid == tid == 0.  However, there are actually separate idle tasks for each
cpu. That creates a problem for thread stack processing which assumes that
each thread has a single stack, not one stack per cpu. Fix that by passing
through the cpu number, and in the case of the idle "thread", pick the
thread stack from an array based on the cpu number.


Adrian Hunter (8):
      perf thread-stack: Simplify some code in thread_stack__process()
      perf thread-stack: Tidy thread_stack__bottom() usage
      perf thread-stack: Avoid direct reference to the thread's stack
      perf thread-stack: Allow for a thread stack array
      perf thread-stack: Factor out thread_stack__init()
      perf thread-stack: Allocate an array of thread stacks
      perf thread-stack: Fix thread stack processing for the idle task
      perf session: Add comment for perf_session__register_idle_thread()

 tools/perf/builtin-script.c    |   4 +-
 tools/perf/util/intel-bts.c    |   4 +-
 tools/perf/util/intel-pt.c     |   6 +-
 tools/perf/util/session.c      |   7 ++
 tools/perf/util/thread-stack.c | 228 +++++++++++++++++++++++++++++------------
 tools/perf/util/thread-stack.h |   8 +-
 6 files changed, 183 insertions(+), 74 deletions(-)


Regards
Adrian

[PATCH 1/8] perf thread-stack: Simplify some code in thread_stack__process()

[Adrian Hunter]

In preparation for fixing thread stack processing for the idle task,
simplify some code in thread_stack__process().

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/thread-stack.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/tools/perf/util/thread-stack.c b/tools/perf/util/thread-stack.c
index 61a4286a74dc..115dc4b27a1b 100644
--- a/tools/perf/util/thread-stack.c
+++ b/tools/perf/util/thread-stack.c
@@ -593,17 +593,13 @@ int thread_stack__process(struct thread *thread, struct comm *comm,
 	struct thread_stack *ts = thread->ts;
 	int err = 0;
 
-	if (ts) {
-		if (!ts->crp) {
-			/* Supersede thread_stack__event() */
-			thread_stack__free(thread);
-			thread->ts = thread_stack__new(thread, crp);
-			if (!thread->ts)
-				return -ENOMEM;
-			ts = thread->ts;
-			ts->comm = comm;
-		}
-	} else {
+	if (ts && !ts->crp) {
+		/* Supersede thread_stack__event() */
+		thread_stack__free(thread);
+		ts = NULL;
+	}
+
+	if (!ts) {
 		thread->ts = thread_stack__new(thread, crp);
 		if (!thread->ts)
 			return -ENOMEM;
-- 
2.17.1

[PATCH 2/8] perf thread-stack: Tidy thread_stack__bottom() usage

[Adrian Hunter]

In preparation for fixing thread stack processing for the idle task,
tidy thread_stack__bottom() usage. Specifically, the parameter 'thread' is
not needed.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/thread-stack.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/thread-stack.c b/tools/perf/util/thread-stack.c
index 115dc4b27a1b..068c7c8db4be 100644
--- a/tools/perf/util/thread-stack.c
+++ b/tools/perf/util/thread-stack.c
@@ -449,7 +449,7 @@ static int thread_stack__pop_cp(struct thread *thread, struct thread_stack *ts,
 	return 1;
 }
 
-static int thread_stack__bottom(struct thread *thread, struct thread_stack *ts,
+static int thread_stack__bottom(struct thread_stack *ts,
 				struct perf_sample *sample,
 				struct addr_location *from_al,
 				struct addr_location *to_al, u64 ref)
@@ -474,7 +474,7 @@ static int thread_stack__bottom(struct thread *thread, struct thread_stack *ts,
 	if (!cp)
 		return -ENOMEM;
 
-	return thread_stack__push_cp(thread->ts, ip, sample->time, ref, cp,
+	return thread_stack__push_cp(ts, ip, sample->time, ref, cp,
 				     true, false);
 }
 
@@ -617,8 +617,7 @@ int thread_stack__process(struct thread *thread, struct comm *comm,
 
 	/* If the stack is empty, put the current symbol on the stack */
 	if (!ts->cnt) {
-		err = thread_stack__bottom(thread, ts, sample, from_al, to_al,
-					   ref);
+		err = thread_stack__bottom(ts, sample, from_al, to_al, ref);
 		if (err)
 			return err;
 	}
-- 
2.17.1

[PATCH 3/8] perf thread-stack: Avoid direct reference to the thread's stack

[Adrian Hunter]

In preparation for fixing thread stack processing for the idle task,
avoid direct reference to the thread's stack. The thread stack will change
to an array of thread stacks, at which point the meaning of the direct
reference will change.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/thread-stack.c | 81 ++++++++++++++++++++--------------
 1 file changed, 49 insertions(+), 32 deletions(-)

diff --git a/tools/perf/util/thread-stack.c b/tools/perf/util/thread-stack.c
index 068c7c8db4be..e13127755293 100644
--- a/tools/perf/util/thread-stack.c
+++ b/tools/perf/util/thread-stack.c
@@ -111,9 +111,16 @@ static struct thread_stack *thread_stack__new(struct thread *thread,
 		ts->kernel_start = 1ULL << 63;
 	ts->crp = crp;
 
+	thread->ts = ts;
+
 	return ts;
 }
 
+static inline struct thread_stack *thread_stack__ts(struct thread *thread)
+{
+	return thread ? thread->ts : NULL;
+}
+
 static int thread_stack__push(struct thread_stack *ts, u64 ret_addr,
 			      bool trace_end)
 {
@@ -226,8 +233,10 @@ static int __thread_stack__flush(struct thread *thread, struct thread_stack *ts)
 
 int thread_stack__flush(struct thread *thread)
 {
-	if (thread->ts)
-		return __thread_stack__flush(thread, thread->ts);
+	struct thread_stack *ts = thread->ts;
+
+	if (ts)
+		return __thread_stack__flush(thread, ts);
 
 	return 0;
 }
@@ -235,16 +244,18 @@ int thread_stack__flush(struct thread *thread)
 int thread_stack__event(struct thread *thread, u32 flags, u64 from_ip,
 			u64 to_ip, u16 insn_len, u64 trace_nr)
 {
+	struct thread_stack *ts = thread_stack__ts(thread);
+
 	if (!thread)
 		return -EINVAL;
 
-	if (!thread->ts) {
-		thread->ts = thread_stack__new(thread, NULL);
-		if (!thread->ts) {
+	if (!ts) {
+		ts = thread_stack__new(thread, NULL);
+		if (!ts) {
 			pr_warning("Out of memory: no thread stack\n");
 			return -ENOMEM;
 		}
-		thread->ts->trace_nr = trace_nr;
+		ts->trace_nr = trace_nr;
 	}
 
 	/*
@@ -252,14 +263,14 @@ int thread_stack__event(struct thread *thread, u32 flags, u64 from_ip,
 	 * the stack might be completely invalid.  Better to report nothing than
 	 * to report something misleading, so flush the stack.
 	 */
-	if (trace_nr != thread->ts->trace_nr) {
-		if (thread->ts->trace_nr)
-			__thread_stack__flush(thread, thread->ts);
-		thread->ts->trace_nr = trace_nr;
+	if (trace_nr != ts->trace_nr) {
+		if (ts->trace_nr)
+			__thread_stack__flush(thread, ts);
+		ts->trace_nr = trace_nr;
 	}
 
 	/* Stop here if thread_stack__process() is in use */
-	if (thread->ts->crp)
+	if (ts->crp)
 		return 0;
 
 	if (flags & PERF_IP_FLAG_CALL) {
@@ -270,7 +281,7 @@ int thread_stack__event(struct thread *thread, u32 flags, u64 from_ip,
 		ret_addr = from_ip + insn_len;
 		if (ret_addr == to_ip)
 			return 0; /* Zero-length calls are excluded */
-		return thread_stack__push(thread->ts, ret_addr,
+		return thread_stack__push(ts, ret_addr,
 					  flags & PERF_IP_FLAG_TRACE_END);
 	} else if (flags & PERF_IP_FLAG_TRACE_BEGIN) {
 		/*
@@ -280,10 +291,10 @@ int thread_stack__event(struct thread *thread, u32 flags, u64 from_ip,
 		 * address, so try to pop that. Also, do not expect a call made
 		 * when the trace ended, to return, so pop that.
 		 */
-		thread_stack__pop(thread->ts, to_ip);
-		thread_stack__pop_trace_end(thread->ts);
+		thread_stack__pop(ts, to_ip);
+		thread_stack__pop_trace_end(ts);
 	} else if ((flags & PERF_IP_FLAG_RETURN) && from_ip) {
-		thread_stack__pop(thread->ts, to_ip);
+		thread_stack__pop(ts, to_ip);
 	}
 
 	return 0;
@@ -291,21 +302,25 @@ int thread_stack__event(struct thread *thread, u32 flags, u64 from_ip,
 
 void thread_stack__set_trace_nr(struct thread *thread, u64 trace_nr)
 {
-	if (!thread || !thread->ts)
+	struct thread_stack *ts = thread_stack__ts(thread);
+
+	if (!ts)
 		return;
 
-	if (trace_nr != thread->ts->trace_nr) {
-		if (thread->ts->trace_nr)
-			__thread_stack__flush(thread, thread->ts);
-		thread->ts->trace_nr = trace_nr;
+	if (trace_nr != ts->trace_nr) {
+		if (ts->trace_nr)
+			__thread_stack__flush(thread, ts);
+		ts->trace_nr = trace_nr;
 	}
 }
 
 void thread_stack__free(struct thread *thread)
 {
-	if (thread->ts) {
-		__thread_stack__flush(thread, thread->ts);
-		zfree(&thread->ts->stack);
+	struct thread_stack *ts = thread->ts;
+
+	if (ts) {
+		__thread_stack__flush(thread, ts);
+		zfree(&ts->stack);
 		zfree(&thread->ts);
 	}
 }
@@ -318,6 +333,7 @@ static inline u64 callchain_context(u64 ip, u64 kernel_start)
 void thread_stack__sample(struct thread *thread, struct ip_callchain *chain,
 			  size_t sz, u64 ip, u64 kernel_start)
 {
+	struct thread_stack *ts = thread_stack__ts(thread);
 	u64 context = callchain_context(ip, kernel_start);
 	u64 last_context;
 	size_t i, j;
@@ -330,15 +346,15 @@ void thread_stack__sample(struct thread *thread, struct ip_callchain *chain,
 	chain->ips[0] = context;
 	chain->ips[1] = ip;
 
-	if (!thread || !thread->ts) {
+	if (!ts) {
 		chain->nr = 2;
 		return;
 	}
 
 	last_context = context;
 
-	for (i = 2, j = 1; i < sz && j <= thread->ts->cnt; i++, j++) {
-		ip = thread->ts->stack[thread->ts->cnt - j].ret_addr;
+	for (i = 2, j = 1; i < sz && j <= ts->cnt; i++, j++) {
+		ip = ts->stack[ts->cnt - j].ret_addr;
 		context = callchain_context(ip, kernel_start);
 		if (context != last_context) {
 			if (i >= sz - 1)
@@ -590,7 +606,7 @@ int thread_stack__process(struct thread *thread, struct comm *comm,
 			  struct addr_location *to_al, u64 ref,
 			  struct call_return_processor *crp)
 {
-	struct thread_stack *ts = thread->ts;
+	struct thread_stack *ts = thread_stack__ts(thread);
 	int err = 0;
 
 	if (ts && !ts->crp) {
@@ -600,10 +616,9 @@ int thread_stack__process(struct thread *thread, struct comm *comm,
 	}
 
 	if (!ts) {
-		thread->ts = thread_stack__new(thread, crp);
-		if (!thread->ts)
+		ts = thread_stack__new(thread, crp);
+		if (!ts)
 			return -ENOMEM;
-		ts = thread->ts;
 		ts->comm = comm;
 	}
 
@@ -668,7 +683,9 @@ int thread_stack__process(struct thread *thread, struct comm *comm,
 
 size_t thread_stack__depth(struct thread *thread)
 {
-	if (!thread->ts)
+	struct thread_stack *ts = thread_stack__ts(thread);
+
+	if (!ts)
 		return 0;
-	return thread->ts->cnt;
+	return ts->cnt;
 }
-- 
2.17.1

[PATCH 4/8] perf thread-stack: Allow for a thread stack array

[Adrian Hunter]

In preparation for fixing thread stack processing for the idle task,
allow for a thread stack array.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/thread-stack.c | 40 +++++++++++++++++++++++++++++-----
 1 file changed, 34 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/thread-stack.c b/tools/perf/util/thread-stack.c
index e13127755293..e59f6fa4c67d 100644
--- a/tools/perf/util/thread-stack.c
+++ b/tools/perf/util/thread-stack.c
@@ -60,6 +60,7 @@ struct thread_stack_entry {
  * @last_time: last timestamp
  * @crp: call/return processor
  * @comm: current comm
+ * @arr_sz: size of array if this is the first element of an array
  */
 struct thread_stack {
 	struct thread_stack_entry *stack;
@@ -71,6 +72,7 @@ struct thread_stack {
 	u64 last_time;
 	struct call_return_processor *crp;
 	struct comm *comm;
+	unsigned int arr_sz;
 };
 
 static int thread_stack__grow(struct thread_stack *ts)
@@ -100,6 +102,8 @@ static struct thread_stack *thread_stack__new(struct thread *thread,
 	if (!ts)
 		return NULL;
 
+	ts->arr_sz = 1;
+
 	if (thread_stack__grow(ts)) {
 		free(ts);
 		return NULL;
@@ -234,11 +238,19 @@ static int __thread_stack__flush(struct thread *thread, struct thread_stack *ts)
 int thread_stack__flush(struct thread *thread)
 {
 	struct thread_stack *ts = thread->ts;
+	unsigned int pos;
+	int err = 0;
 
-	if (ts)
-		return __thread_stack__flush(thread, ts);
+	if (ts) {
+		for (pos = 0; pos < ts->arr_sz; pos++) {
+			int ret = __thread_stack__flush(thread, ts + pos);
 
-	return 0;
+			if (ret)
+				err = ret;
+		}
+	}
+
+	return err;
 }
 
 int thread_stack__event(struct thread *thread, u32 flags, u64 from_ip,
@@ -314,13 +326,29 @@ void thread_stack__set_trace_nr(struct thread *thread, u64 trace_nr)
 	}
 }
 
+static void __thread_stack__free(struct thread *thread, struct thread_stack *ts)
+{
+	__thread_stack__flush(thread, ts);
+	zfree(&ts->stack);
+}
+
+static void thread_stack__reset(struct thread *thread, struct thread_stack *ts)
+{
+	unsigned int arr_sz = ts->arr_sz;
+
+	__thread_stack__free(thread, ts);
+	memset(ts, 0, sizeof(*ts));
+	ts->arr_sz = arr_sz;
+}
+
 void thread_stack__free(struct thread *thread)
 {
 	struct thread_stack *ts = thread->ts;
+	unsigned int pos;
 
 	if (ts) {
-		__thread_stack__flush(thread, ts);
-		zfree(&ts->stack);
+		for (pos = 0; pos < ts->arr_sz; pos++)
+			__thread_stack__free(thread, ts + pos);
 		zfree(&thread->ts);
 	}
 }
@@ -611,7 +639,7 @@ int thread_stack__process(struct thread *thread, struct comm *comm,
 
 	if (ts && !ts->crp) {
 		/* Supersede thread_stack__event() */
-		thread_stack__free(thread);
+		thread_stack__reset(thread, ts);
 		ts = NULL;
 	}
 
-- 
2.17.1

[PATCH 5/8] perf thread-stack: Factor out thread_stack__init()

[Adrian Hunter]

In preparation for fixing thread stack processing for the idle task,
factor out thread_stack__init().

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/thread-stack.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/tools/perf/util/thread-stack.c b/tools/perf/util/thread-stack.c
index e59f6fa4c67d..4340381d54c2 100644
--- a/tools/perf/util/thread-stack.c
+++ b/tools/perf/util/thread-stack.c
@@ -93,6 +93,24 @@ static int thread_stack__grow(struct thread_stack *ts)
 	return 0;
 }
 
+static int thread_stack__init(struct thread_stack *ts, struct thread *thread,
+			      struct call_return_processor *crp)
+{
+	int err;
+
+	err = thread_stack__grow(ts);
+	if (err)
+		return err;
+
+	if (thread->mg && thread->mg->machine)
+		ts->kernel_start = machine__kernel_start(thread->mg->machine);
+	else
+		ts->kernel_start = 1ULL << 63;
+	ts->crp = crp;
+
+	return 0;
+}
+
 static struct thread_stack *thread_stack__new(struct thread *thread,
 					      struct call_return_processor *crp)
 {
@@ -104,17 +122,11 @@ static struct thread_stack *thread_stack__new(struct thread *thread,
 
 	ts->arr_sz = 1;
 
-	if (thread_stack__grow(ts)) {
+	if (thread_stack__init(ts, thread, crp)) {
 		free(ts);
 		return NULL;
 	}
 
-	if (thread->mg && thread->mg->machine)
-		ts->kernel_start = machine__kernel_start(thread->mg->machine);
-	else
-		ts->kernel_start = 1ULL << 63;
-	ts->crp = crp;
-
 	thread->ts = ts;
 
 	return ts;
-- 
2.17.1

[PATCH 6/8] perf thread-stack: Allocate an array of thread stacks

[Adrian Hunter]

In preparation for fixing thread stack processing for the idle task,
allocate an array of thread stacks.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/thread-stack.c | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/tools/perf/util/thread-stack.c b/tools/perf/util/thread-stack.c
index 4340381d54c2..a896d89fe5f7 100644
--- a/tools/perf/util/thread-stack.c
+++ b/tools/perf/util/thread-stack.c
@@ -114,20 +114,26 @@ static int thread_stack__init(struct thread_stack *ts, struct thread *thread,
 static struct thread_stack *thread_stack__new(struct thread *thread,
 					      struct call_return_processor *crp)
 {
-	struct thread_stack *ts;
-
-	ts = zalloc(sizeof(struct thread_stack));
-	if (!ts)
-		return NULL;
-
-	ts->arr_sz = 1;
-
-	if (thread_stack__init(ts, thread, crp)) {
-		free(ts);
-		return NULL;
+	struct thread_stack *ts = thread->ts, *new_ts;
+	unsigned int old_sz = ts ? ts->arr_sz : 0;
+	unsigned int new_sz = 1;
+
+	if (!ts || new_sz > old_sz) {
+		new_ts = calloc(new_sz, sizeof(*ts));
+		if (!new_ts)
+			return NULL;
+		if (ts)
+			memcpy(new_ts, ts, old_sz * sizeof(*ts));
+		new_ts->arr_sz = new_sz;
+		if (thread->ts)
+			zfree(&thread->ts);
+		thread->ts = new_ts;
+		ts = new_ts;
 	}
 
-	thread->ts = ts;
+	if (!ts->stack &&
+	    thread_stack__init(ts, thread, crp))
+		return NULL;
 
 	return ts;
 }
-- 
2.17.1

[PATCH 8/8] perf session: Add comment for perf_session__register_idle_thread()

[Adrian Hunter]

Add a comment to perf_session__register_idle_thread() to bring attention to
a pitfall with the idle task thread structure. The pitfall is that there
should really be a 'struct thread' for the idle task of each cpu, but there
is only one that can have pid == tid == 0.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/session.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 78a067777144..5456c84c7dd1 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1527,6 +1527,13 @@ struct thread *perf_session__findnew(struct perf_session *session, pid_t pid)
 	return machine__findnew_thread(&session->machines.host, -1, pid);
 }
 
+/*
+ * Threads are identified by pid and tid, and the idle task has pid == tid == 0.
+ * So here a single thread is created for that, but actually there is a separate
+ * idle task per cpu, so there should be one 'struct thread' per cpu, but there
+ * is only 1. That causes problems for some tools, requiring workarounds. For
+ * example get_idle_thread() in builtin-sched.c, or thread_stack__per_cpu().
+ */
 int perf_session__register_idle_thread(struct perf_session *session)
 {
 	struct thread *thread;
-- 
2.17.1

Re: [PATCH 6/8] perf thread-stack: Allocate an array of thread stacks

[Jiri Olsa]

On Fri, Dec 21, 2018 at 02:06:18PM +0200, Adrian Hunter wrote:
> In preparation for fixing thread stack processing for the idle task,
> allocate an array of thread stacks.
> 
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>  tools/perf/util/thread-stack.c | 30 ++++++++++++++++++------------
>  1 file changed, 18 insertions(+), 12 deletions(-)
> 
> diff --git a/tools/perf/util/thread-stack.c b/tools/perf/util/thread-stack.c
> index 4340381d54c2..a896d89fe5f7 100644
> --- a/tools/perf/util/thread-stack.c
> +++ b/tools/perf/util/thread-stack.c
> @@ -114,20 +114,26 @@ static int thread_stack__init(struct thread_stack *ts, struct thread *thread,
>  static struct thread_stack *thread_stack__new(struct thread *thread,
>  					      struct call_return_processor *crp)
>  {
> -	struct thread_stack *ts;
> -
> -	ts = zalloc(sizeof(struct thread_stack));
> -	if (!ts)
> -		return NULL;
> -
> -	ts->arr_sz = 1;
> -
> -	if (thread_stack__init(ts, thread, crp)) {
> -		free(ts);
> -		return NULL;
> +	struct thread_stack *ts = thread->ts, *new_ts;
> +	unsigned int old_sz = ts ? ts->arr_sz : 0;
> +	unsigned int new_sz = 1;
> +
> +	if (!ts || new_sz > old_sz) {
> +		new_ts = calloc(new_sz, sizeof(*ts));
> +		if (!new_ts)
> +			return NULL;
> +		if (ts)
> +			memcpy(new_ts, ts, old_sz * sizeof(*ts));
> +		new_ts->arr_sz = new_sz;
> +		if (thread->ts)
> +			zfree(&thread->ts);

you don't need to check for thread->ts,
anyway it looks all good, for the patchset:

Acked-by: Jiri Olsa <jolsa@kernel.org>

jirka

> +		thread->ts = new_ts;
> +		ts = new_ts;
>  	}
>  
> -	thread->ts = ts;
> +	if (!ts->stack &&
> +	    thread_stack__init(ts, thread, crp))
> +		return NULL;
>  
>  	return ts;
>  }
> -- 
> 2.17.1
> 

Re: [PATCH 6/8] perf thread-stack: Allocate an array of thread stacks

[Arnaldo Carvalho de Melo]

Em Tue, Jan 01, 2019 at 07:28:52PM +0100, Jiri Olsa escreveu:
> On Fri, Dec 21, 2018 at 02:06:18PM +0200, Adrian Hunter wrote:
> > In preparation for fixing thread stack processing for the idle task,
> > allocate an array of thread stacks.
> > 
> > Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> > ---
> >  tools/perf/util/thread-stack.c | 30 ++++++++++++++++++------------
> >  1 file changed, 18 insertions(+), 12 deletions(-)
> > 
> > diff --git a/tools/perf/util/thread-stack.c b/tools/perf/util/thread-stack.c
> > index 4340381d54c2..a896d89fe5f7 100644
> > --- a/tools/perf/util/thread-stack.c
> > +++ b/tools/perf/util/thread-stack.c
> > @@ -114,20 +114,26 @@ static int thread_stack__init(struct thread_stack *ts, struct thread *thread,
> >  static struct thread_stack *thread_stack__new(struct thread *thread,
> >  					      struct call_return_processor *crp)
> >  {
> > -	struct thread_stack *ts;
> > -
> > -	ts = zalloc(sizeof(struct thread_stack));
> > -	if (!ts)
> > -		return NULL;
> > -
> > -	ts->arr_sz = 1;
> > -
> > -	if (thread_stack__init(ts, thread, crp)) {
> > -		free(ts);
> > -		return NULL;
> > +	struct thread_stack *ts = thread->ts, *new_ts;
> > +	unsigned int old_sz = ts ? ts->arr_sz : 0;
> > +	unsigned int new_sz = 1;
> > +
> > +	if (!ts || new_sz > old_sz) {
> > +		new_ts = calloc(new_sz, sizeof(*ts));
> > +		if (!new_ts)
> > +			return NULL;
> > +		if (ts)
> > +			memcpy(new_ts, ts, old_sz * sizeof(*ts));
> > +		new_ts->arr_sz = new_sz;
> > +		if (thread->ts)
> > +			zfree(&thread->ts);
> 
> you don't need to check for thread->ts,
> anyway it looks all good, for the patchset:
> 
> Acked-by: Jiri Olsa <jolsa@kernel.org>

I'll fix that when merging, which I'm now doing.

Thanks,

- Arnaldo

Re: [PATCH 3/8] perf thread-stack: Avoid direct reference to the thread's stack

[Arnaldo Carvalho de Melo]

Em Fri, Dec 21, 2018 at 02:06:15PM +0200, Adrian Hunter escreveu:
> In preparation for fixing thread stack processing for the idle task,
> avoid direct reference to the thread's stack. The thread stack will change
> to an array of thread stacks, at which point the meaning of the direct
> reference will change.
> 
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>  tools/perf/util/thread-stack.c | 81 ++++++++++++++++++++--------------
>  1 file changed, 49 insertions(+), 32 deletions(-)
> 
> diff --git a/tools/perf/util/thread-stack.c b/tools/perf/util/thread-stack.c
> index 068c7c8db4be..e13127755293 100644
> --- a/tools/perf/util/thread-stack.c
> +++ b/tools/perf/util/thread-stack.c
> @@ -111,9 +111,16 @@ static struct thread_stack *thread_stack__new(struct thread *thread,
>  		ts->kernel_start = 1ULL << 63;
>  	ts->crp = crp;
>  
> +	thread->ts = ts;
> +
>  	return ts;
>  }
>  
> +static inline struct thread_stack *thread_stack__ts(struct thread *thread)
> +{
> +	return thread ? thread->ts : NULL;
> +}

So, this is a 'thread' method, so it should be instead:

static inline struct thread_stack *thread__stack(struct thread *thread)

perhaps?

Or thread__ts() or the longer thread__thread_stack(), I think
thread__stack() is ok, I'm doing it tentatively in my local branch,
holler if you disagree.

- Arnaldo
  

> +
>  static int thread_stack__push(struct thread_stack *ts, u64 ret_addr,
>  			      bool trace_end)
>  {
> @@ -226,8 +233,10 @@ static int __thread_stack__flush(struct thread *thread, struct thread_stack *ts)
>  
>  int thread_stack__flush(struct thread *thread)
>  {
> -	if (thread->ts)
> -		return __thread_stack__flush(thread, thread->ts);
> +	struct thread_stack *ts = thread->ts;
> +
> +	if (ts)
> +		return __thread_stack__flush(thread, ts);
>  
>  	return 0;
>  }
> @@ -235,16 +244,18 @@ int thread_stack__flush(struct thread *thread)
>  int thread_stack__event(struct thread *thread, u32 flags, u64 from_ip,
>  			u64 to_ip, u16 insn_len, u64 trace_nr)
>  {
> +	struct thread_stack *ts = thread_stack__ts(thread);
> +
>  	if (!thread)
>  		return -EINVAL;
>  
> -	if (!thread->ts) {
> -		thread->ts = thread_stack__new(thread, NULL);
> -		if (!thread->ts) {
> +	if (!ts) {
> +		ts = thread_stack__new(thread, NULL);
> +		if (!ts) {
>  			pr_warning("Out of memory: no thread stack\n");
>  			return -ENOMEM;
>  		}
> -		thread->ts->trace_nr = trace_nr;
> +		ts->trace_nr = trace_nr;
>  	}
>  
>  	/*
> @@ -252,14 +263,14 @@ int thread_stack__event(struct thread *thread, u32 flags, u64 from_ip,
>  	 * the stack might be completely invalid.  Better to report nothing than
>  	 * to report something misleading, so flush the stack.
>  	 */
> -	if (trace_nr != thread->ts->trace_nr) {
> -		if (thread->ts->trace_nr)
> -			__thread_stack__flush(thread, thread->ts);
> -		thread->ts->trace_nr = trace_nr;
> +	if (trace_nr != ts->trace_nr) {
> +		if (ts->trace_nr)
> +			__thread_stack__flush(thread, ts);
> +		ts->trace_nr = trace_nr;
>  	}
>  
>  	/* Stop here if thread_stack__process() is in use */
> -	if (thread->ts->crp)
> +	if (ts->crp)
>  		return 0;
>  
>  	if (flags & PERF_IP_FLAG_CALL) {
> @@ -270,7 +281,7 @@ int thread_stack__event(struct thread *thread, u32 flags, u64 from_ip,
>  		ret_addr = from_ip + insn_len;
>  		if (ret_addr == to_ip)
>  			return 0; /* Zero-length calls are excluded */
> -		return thread_stack__push(thread->ts, ret_addr,
> +		return thread_stack__push(ts, ret_addr,
>  					  flags & PERF_IP_FLAG_TRACE_END);
>  	} else if (flags & PERF_IP_FLAG_TRACE_BEGIN) {
>  		/*
> @@ -280,10 +291,10 @@ int thread_stack__event(struct thread *thread, u32 flags, u64 from_ip,
>  		 * address, so try to pop that. Also, do not expect a call made
>  		 * when the trace ended, to return, so pop that.
>  		 */
> -		thread_stack__pop(thread->ts, to_ip);
> -		thread_stack__pop_trace_end(thread->ts);
> +		thread_stack__pop(ts, to_ip);
> +		thread_stack__pop_trace_end(ts);
>  	} else if ((flags & PERF_IP_FLAG_RETURN) && from_ip) {
> -		thread_stack__pop(thread->ts, to_ip);
> +		thread_stack__pop(ts, to_ip);
>  	}
>  
>  	return 0;
> @@ -291,21 +302,25 @@ int thread_stack__event(struct thread *thread, u32 flags, u64 from_ip,
>  
>  void thread_stack__set_trace_nr(struct thread *thread, u64 trace_nr)
>  {
> -	if (!thread || !thread->ts)
> +	struct thread_stack *ts = thread_stack__ts(thread);
> +
> +	if (!ts)
>  		return;
>  
> -	if (trace_nr != thread->ts->trace_nr) {
> -		if (thread->ts->trace_nr)
> -			__thread_stack__flush(thread, thread->ts);
> -		thread->ts->trace_nr = trace_nr;
> +	if (trace_nr != ts->trace_nr) {
> +		if (ts->trace_nr)
> +			__thread_stack__flush(thread, ts);
> +		ts->trace_nr = trace_nr;
>  	}
>  }
>  
>  void thread_stack__free(struct thread *thread)
>  {
> -	if (thread->ts) {
> -		__thread_stack__flush(thread, thread->ts);
> -		zfree(&thread->ts->stack);
> +	struct thread_stack *ts = thread->ts;
> +
> +	if (ts) {
> +		__thread_stack__flush(thread, ts);
> +		zfree(&ts->stack);
>  		zfree(&thread->ts);
>  	}
>  }
> @@ -318,6 +333,7 @@ static inline u64 callchain_context(u64 ip, u64 kernel_start)
>  void thread_stack__sample(struct thread *thread, struct ip_callchain *chain,
>  			  size_t sz, u64 ip, u64 kernel_start)
>  {
> +	struct thread_stack *ts = thread_stack__ts(thread);
>  	u64 context = callchain_context(ip, kernel_start);
>  	u64 last_context;
>  	size_t i, j;
> @@ -330,15 +346,15 @@ void thread_stack__sample(struct thread *thread, struct ip_callchain *chain,
>  	chain->ips[0] = context;
>  	chain->ips[1] = ip;
>  
> -	if (!thread || !thread->ts) {
> +	if (!ts) {
>  		chain->nr = 2;
>  		return;
>  	}
>  
>  	last_context = context;
>  
> -	for (i = 2, j = 1; i < sz && j <= thread->ts->cnt; i++, j++) {
> -		ip = thread->ts->stack[thread->ts->cnt - j].ret_addr;
> +	for (i = 2, j = 1; i < sz && j <= ts->cnt; i++, j++) {
> +		ip = ts->stack[ts->cnt - j].ret_addr;
>  		context = callchain_context(ip, kernel_start);
>  		if (context != last_context) {
>  			if (i >= sz - 1)
> @@ -590,7 +606,7 @@ int thread_stack__process(struct thread *thread, struct comm *comm,
>  			  struct addr_location *to_al, u64 ref,
>  			  struct call_return_processor *crp)
>  {
> -	struct thread_stack *ts = thread->ts;
> +	struct thread_stack *ts = thread_stack__ts(thread);
>  	int err = 0;
>  
>  	if (ts && !ts->crp) {
> @@ -600,10 +616,9 @@ int thread_stack__process(struct thread *thread, struct comm *comm,
>  	}
>  
>  	if (!ts) {
> -		thread->ts = thread_stack__new(thread, crp);
> -		if (!thread->ts)
> +		ts = thread_stack__new(thread, crp);
> +		if (!ts)
>  			return -ENOMEM;
> -		ts = thread->ts;
>  		ts->comm = comm;
>  	}
>  
> @@ -668,7 +683,9 @@ int thread_stack__process(struct thread *thread, struct comm *comm,
>  
>  size_t thread_stack__depth(struct thread *thread)
>  {
> -	if (!thread->ts)
> +	struct thread_stack *ts = thread_stack__ts(thread);
> +
> +	if (!ts)
>  		return 0;
> -	return thread->ts->cnt;
> +	return ts->cnt;
>  }
> -- 
> 2.17.1

-- 

- Arnaldo

[tip:perf/urgent] perf thread-stack: Simplify some code in thread_stack__process()

[tip-bot for Adrian Hunter]

Commit-ID:  03b32cb2810814756095dbd91fce0c77617d096b
Gitweb:     https://git.kernel.org/tip/03b32cb2810814756095dbd91fce0c77617d096b
Author:     Adrian Hunter <adrian.hunter@intel.com>
AuthorDate: Fri, 21 Dec 2018 14:06:13 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 2 Jan 2019 10:42:45 -0300

perf thread-stack: Simplify some code in thread_stack__process()

In preparation for fixing thread stack processing for the idle task,
simplify some code in thread_stack__process().

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Link: http://lkml.kernel.org/r/20181221120620.9659-2-adrian.hunter@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/thread-stack.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/tools/perf/util/thread-stack.c b/tools/perf/util/thread-stack.c
index 61a4286a74dc..115dc4b27a1b 100644
--- a/tools/perf/util/thread-stack.c
+++ b/tools/perf/util/thread-stack.c
@@ -593,17 +593,13 @@ int thread_stack__process(struct thread *thread, struct comm *comm,
 	struct thread_stack *ts = thread->ts;
 	int err = 0;
 
-	if (ts) {
-		if (!ts->crp) {
-			/* Supersede thread_stack__event() */
-			thread_stack__free(thread);
-			thread->ts = thread_stack__new(thread, crp);
-			if (!thread->ts)
-				return -ENOMEM;
-			ts = thread->ts;
-			ts->comm = comm;
-		}
-	} else {
+	if (ts && !ts->crp) {
+		/* Supersede thread_stack__event() */
+		thread_stack__free(thread);
+		ts = NULL;
+	}
+
+	if (!ts) {
 		thread->ts = thread_stack__new(thread, crp);
 		if (!thread->ts)
 			return -ENOMEM;

[tip:perf/urgent] perf thread-stack: Tidy thread_stack__bottom() usage

[tip-bot for Adrian Hunter]

Commit-ID:  e0b8951190c11797971864c845e0909561525621
Gitweb:     https://git.kernel.org/tip/e0b8951190c11797971864c845e0909561525621
Author:     Adrian Hunter <adrian.hunter@intel.com>
AuthorDate: Fri, 21 Dec 2018 14:06:14 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 2 Jan 2019 10:45:26 -0300

perf thread-stack: Tidy thread_stack__bottom() usage

In preparation for fixing thread stack processing for the idle task,
tidy thread_stack__bottom() usage. Specifically, the parameter 'thread'
is not needed.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Link: http://lkml.kernel.org/r/20181221120620.9659-3-adrian.hunter@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/thread-stack.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/thread-stack.c b/tools/perf/util/thread-stack.c
index 115dc4b27a1b..068c7c8db4be 100644
--- a/tools/perf/util/thread-stack.c
+++ b/tools/perf/util/thread-stack.c
@@ -449,7 +449,7 @@ static int thread_stack__pop_cp(struct thread *thread, struct thread_stack *ts,
 	return 1;
 }
 
-static int thread_stack__bottom(struct thread *thread, struct thread_stack *ts,
+static int thread_stack__bottom(struct thread_stack *ts,
 				struct perf_sample *sample,
 				struct addr_location *from_al,
 				struct addr_location *to_al, u64 ref)
@@ -474,7 +474,7 @@ static int thread_stack__bottom(struct thread *thread, struct thread_stack *ts,
 	if (!cp)
 		return -ENOMEM;
 
-	return thread_stack__push_cp(thread->ts, ip, sample->time, ref, cp,
+	return thread_stack__push_cp(ts, ip, sample->time, ref, cp,
 				     true, false);
 }
 
@@ -617,8 +617,7 @@ int thread_stack__process(struct thread *thread, struct comm *comm,
 
 	/* If the stack is empty, put the current symbol on the stack */
 	if (!ts->cnt) {
-		err = thread_stack__bottom(thread, ts, sample, from_al, to_al,
-					   ref);
+		err = thread_stack__bottom(ts, sample, from_al, to_al, ref);
 		if (err)
 			return err;
 	}

[tip:perf/urgent] perf thread-stack: Avoid direct reference to the thread's stack

[tip-bot for Adrian Hunter]

Commit-ID:  bd8e68ace110941f375f5d566b0cd99fe80634b8
Gitweb:     https://git.kernel.org/tip/bd8e68ace110941f375f5d566b0cd99fe80634b8
Author:     Adrian Hunter <adrian.hunter@intel.com>
AuthorDate: Fri, 21 Dec 2018 14:06:15 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 2 Jan 2019 10:48:18 -0300

perf thread-stack: Avoid direct reference to the thread's stack

In preparation for fixing thread stack processing for the idle task,
avoid direct reference to the thread's stack. The thread stack will
change to an array of thread stacks, at which point the meaning of the
direct reference will change.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Link: http://lkml.kernel.org/r/20181221120620.9659-4-adrian.hunter@intel.com
[ Rename thread_stack__ts() to thread__stack() since this operates on a 'thread' struct ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/thread-stack.c | 81 +++++++++++++++++++++++++-----------------
 1 file changed, 49 insertions(+), 32 deletions(-)

diff --git a/tools/perf/util/thread-stack.c b/tools/perf/util/thread-stack.c
index 068c7c8db4be..d93cd286b048 100644
--- a/tools/perf/util/thread-stack.c
+++ b/tools/perf/util/thread-stack.c
@@ -111,9 +111,16 @@ static struct thread_stack *thread_stack__new(struct thread *thread,
 		ts->kernel_start = 1ULL << 63;
 	ts->crp = crp;
 
+	thread->ts = ts;
+
 	return ts;
 }
 
+static inline struct thread_stack *thread__stack(struct thread *thread)
+{
+	return thread ? thread->ts : NULL;
+}
+
 static int thread_stack__push(struct thread_stack *ts, u64 ret_addr,
 			      bool trace_end)
 {
@@ -226,8 +233,10 @@ static int __thread_stack__flush(struct thread *thread, struct thread_stack *ts)
 
 int thread_stack__flush(struct thread *thread)
 {
-	if (thread->ts)
-		return __thread_stack__flush(thread, thread->ts);
+	struct thread_stack *ts = thread->ts;
+
+	if (ts)
+		return __thread_stack__flush(thread, ts);
 
 	return 0;
 }
@@ -235,16 +244,18 @@ int thread_stack__flush(struct thread *thread)
 int thread_stack__event(struct thread *thread, u32 flags, u64 from_ip,
 			u64 to_ip, u16 insn_len, u64 trace_nr)
 {
+	struct thread_stack *ts = thread__stack(thread);
+
 	if (!thread)
 		return -EINVAL;
 
-	if (!thread->ts) {
-		thread->ts = thread_stack__new(thread, NULL);
-		if (!thread->ts) {
+	if (!ts) {
+		ts = thread_stack__new(thread, NULL);
+		if (!ts) {
 			pr_warning("Out of memory: no thread stack\n");
 			return -ENOMEM;
 		}
-		thread->ts->trace_nr = trace_nr;
+		ts->trace_nr = trace_nr;
 	}
 
 	/*
@@ -252,14 +263,14 @@ int thread_stack__event(struct thread *thread, u32 flags, u64 from_ip,
 	 * the stack might be completely invalid.  Better to report nothing than
 	 * to report something misleading, so flush the stack.
 	 */
-	if (trace_nr != thread->ts->trace_nr) {
-		if (thread->ts->trace_nr)
-			__thread_stack__flush(thread, thread->ts);
-		thread->ts->trace_nr = trace_nr;
+	if (trace_nr != ts->trace_nr) {
+		if (ts->trace_nr)
+			__thread_stack__flush(thread, ts);
+		ts->trace_nr = trace_nr;
 	}
 
 	/* Stop here if thread_stack__process() is in use */
-	if (thread->ts->crp)
+	if (ts->crp)
 		return 0;
 
 	if (flags & PERF_IP_FLAG_CALL) {
@@ -270,7 +281,7 @@ int thread_stack__event(struct thread *thread, u32 flags, u64 from_ip,
 		ret_addr = from_ip + insn_len;
 		if (ret_addr == to_ip)
 			return 0; /* Zero-length calls are excluded */
-		return thread_stack__push(thread->ts, ret_addr,
+		return thread_stack__push(ts, ret_addr,
 					  flags & PERF_IP_FLAG_TRACE_END);
 	} else if (flags & PERF_IP_FLAG_TRACE_BEGIN) {
 		/*
@@ -280,10 +291,10 @@ int thread_stack__event(struct thread *thread, u32 flags, u64 from_ip,
 		 * address, so try to pop that. Also, do not expect a call made
 		 * when the trace ended, to return, so pop that.
 		 */
-		thread_stack__pop(thread->ts, to_ip);
-		thread_stack__pop_trace_end(thread->ts);
+		thread_stack__pop(ts, to_ip);
+		thread_stack__pop_trace_end(ts);
 	} else if ((flags & PERF_IP_FLAG_RETURN) && from_ip) {
-		thread_stack__pop(thread->ts, to_ip);
+		thread_stack__pop(ts, to_ip);
 	}
 
 	return 0;
@@ -291,21 +302,25 @@ int thread_stack__event(struct thread *thread, u32 flags, u64 from_ip,
 
 void thread_stack__set_trace_nr(struct thread *thread, u64 trace_nr)
 {
-	if (!thread || !thread->ts)
+	struct thread_stack *ts = thread__stack(thread);
+
+	if (!ts)
 		return;
 
-	if (trace_nr != thread->ts->trace_nr) {
-		if (thread->ts->trace_nr)
-			__thread_stack__flush(thread, thread->ts);
-		thread->ts->trace_nr = trace_nr;
+	if (trace_nr != ts->trace_nr) {
+		if (ts->trace_nr)
+			__thread_stack__flush(thread, ts);
+		ts->trace_nr = trace_nr;
 	}
 }
 
 void thread_stack__free(struct thread *thread)
 {
-	if (thread->ts) {
-		__thread_stack__flush(thread, thread->ts);
-		zfree(&thread->ts->stack);
+	struct thread_stack *ts = thread->ts;
+
+	if (ts) {
+		__thread_stack__flush(thread, ts);
+		zfree(&ts->stack);
 		zfree(&thread->ts);
 	}
 }
@@ -318,6 +333,7 @@ static inline u64 callchain_context(u64 ip, u64 kernel_start)
 void thread_stack__sample(struct thread *thread, struct ip_callchain *chain,
 			  size_t sz, u64 ip, u64 kernel_start)
 {
+	struct thread_stack *ts = thread__stack(thread);
 	u64 context = callchain_context(ip, kernel_start);
 	u64 last_context;
 	size_t i, j;
@@ -330,15 +346,15 @@ void thread_stack__sample(struct thread *thread, struct ip_callchain *chain,
 	chain->ips[0] = context;
 	chain->ips[1] = ip;
 
-	if (!thread || !thread->ts) {
+	if (!ts) {
 		chain->nr = 2;
 		return;
 	}
 
 	last_context = context;
 
-	for (i = 2, j = 1; i < sz && j <= thread->ts->cnt; i++, j++) {
-		ip = thread->ts->stack[thread->ts->cnt - j].ret_addr;
+	for (i = 2, j = 1; i < sz && j <= ts->cnt; i++, j++) {
+		ip = ts->stack[ts->cnt - j].ret_addr;
 		context = callchain_context(ip, kernel_start);
 		if (context != last_context) {
 			if (i >= sz - 1)
@@ -590,7 +606,7 @@ int thread_stack__process(struct thread *thread, struct comm *comm,
 			  struct addr_location *to_al, u64 ref,
 			  struct call_return_processor *crp)
 {
-	struct thread_stack *ts = thread->ts;
+	struct thread_stack *ts = thread__stack(thread);
 	int err = 0;
 
 	if (ts && !ts->crp) {
@@ -600,10 +616,9 @@ int thread_stack__process(struct thread *thread, struct comm *comm,
 	}
 
 	if (!ts) {
-		thread->ts = thread_stack__new(thread, crp);
-		if (!thread->ts)
+		ts = thread_stack__new(thread, crp);
+		if (!ts)
 			return -ENOMEM;
-		ts = thread->ts;
 		ts->comm = comm;
 	}
 
@@ -668,7 +683,9 @@ int thread_stack__process(struct thread *thread, struct comm *comm,
 
 size_t thread_stack__depth(struct thread *thread)
 {
-	if (!thread->ts)
+	struct thread_stack *ts = thread__stack(thread);
+
+	if (!ts)
 		return 0;
-	return thread->ts->cnt;
+	return ts->cnt;
 }

[tip:perf/urgent] perf thread-stack: Allow for a thread stack array

[tip-bot for Adrian Hunter]

Commit-ID:  f6060ac60190c625101a0b94c2d96e9ca14a7d73
Gitweb:     https://git.kernel.org/tip/f6060ac60190c625101a0b94c2d96e9ca14a7d73
Author:     Adrian Hunter <adrian.hunter@intel.com>
AuthorDate: Fri, 21 Dec 2018 14:06:16 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 2 Jan 2019 10:49:51 -0300

perf thread-stack: Allow for a thread stack array

In preparation for fixing thread stack processing for the idle task,
allow for a thread stack array.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Link: http://lkml.kernel.org/r/20181221120620.9659-5-adrian.hunter@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/thread-stack.c | 40 ++++++++++++++++++++++++++++++++++------
 1 file changed, 34 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/thread-stack.c b/tools/perf/util/thread-stack.c
index d93cd286b048..a5f7b9d8fc23 100644
--- a/tools/perf/util/thread-stack.c
+++ b/tools/perf/util/thread-stack.c
@@ -60,6 +60,7 @@ struct thread_stack_entry {
  * @last_time: last timestamp
  * @crp: call/return processor
  * @comm: current comm
+ * @arr_sz: size of array if this is the first element of an array
  */
 struct thread_stack {
 	struct thread_stack_entry *stack;
@@ -71,6 +72,7 @@ struct thread_stack {
 	u64 last_time;
 	struct call_return_processor *crp;
 	struct comm *comm;
+	unsigned int arr_sz;
 };
 
 static int thread_stack__grow(struct thread_stack *ts)
@@ -100,6 +102,8 @@ static struct thread_stack *thread_stack__new(struct thread *thread,
 	if (!ts)
 		return NULL;
 
+	ts->arr_sz = 1;
+
 	if (thread_stack__grow(ts)) {
 		free(ts);
 		return NULL;
@@ -234,11 +238,19 @@ static int __thread_stack__flush(struct thread *thread, struct thread_stack *ts)
 int thread_stack__flush(struct thread *thread)
 {
 	struct thread_stack *ts = thread->ts;
+	unsigned int pos;
+	int err = 0;
 
-	if (ts)
-		return __thread_stack__flush(thread, ts);
+	if (ts) {
+		for (pos = 0; pos < ts->arr_sz; pos++) {
+			int ret = __thread_stack__flush(thread, ts + pos);
 
-	return 0;
+			if (ret)
+				err = ret;
+		}
+	}
+
+	return err;
 }
 
 int thread_stack__event(struct thread *thread, u32 flags, u64 from_ip,
@@ -314,13 +326,29 @@ void thread_stack__set_trace_nr(struct thread *thread, u64 trace_nr)
 	}
 }
 
+static void __thread_stack__free(struct thread *thread, struct thread_stack *ts)
+{
+	__thread_stack__flush(thread, ts);
+	zfree(&ts->stack);
+}
+
+static void thread_stack__reset(struct thread *thread, struct thread_stack *ts)
+{
+	unsigned int arr_sz = ts->arr_sz;
+
+	__thread_stack__free(thread, ts);
+	memset(ts, 0, sizeof(*ts));
+	ts->arr_sz = arr_sz;
+}
+
 void thread_stack__free(struct thread *thread)
 {
 	struct thread_stack *ts = thread->ts;
+	unsigned int pos;
 
 	if (ts) {
-		__thread_stack__flush(thread, ts);
-		zfree(&ts->stack);
+		for (pos = 0; pos < ts->arr_sz; pos++)
+			__thread_stack__free(thread, ts + pos);
 		zfree(&thread->ts);
 	}
 }
@@ -611,7 +639,7 @@ int thread_stack__process(struct thread *thread, struct comm *comm,
 
 	if (ts && !ts->crp) {
 		/* Supersede thread_stack__event() */
-		thread_stack__free(thread);
+		thread_stack__reset(thread, ts);
 		ts = NULL;
 	}
 

[tip:perf/urgent] perf thread-stack: Factor out thread_stack__init()

[tip-bot for Adrian Hunter]

Commit-ID:  2e9e8688763ff80f032d9a78c3b4b951fb6dd7a4
Gitweb:     https://git.kernel.org/tip/2e9e8688763ff80f032d9a78c3b4b951fb6dd7a4
Author:     Adrian Hunter <adrian.hunter@intel.com>
AuthorDate: Fri, 21 Dec 2018 14:06:17 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 2 Jan 2019 10:53:41 -0300

perf thread-stack: Factor out thread_stack__init()

In preparation for fixing thread stack processing for the idle task,
factor out thread_stack__init().

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Link: http://lkml.kernel.org/r/20181221120620.9659-6-adrian.hunter@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/thread-stack.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/tools/perf/util/thread-stack.c b/tools/perf/util/thread-stack.c
index a5f7b9d8fc23..03770af9e5cd 100644
--- a/tools/perf/util/thread-stack.c
+++ b/tools/perf/util/thread-stack.c
@@ -93,6 +93,24 @@ static int thread_stack__grow(struct thread_stack *ts)
 	return 0;
 }
 
+static int thread_stack__init(struct thread_stack *ts, struct thread *thread,
+			      struct call_return_processor *crp)
+{
+	int err;
+
+	err = thread_stack__grow(ts);
+	if (err)
+		return err;
+
+	if (thread->mg && thread->mg->machine)
+		ts->kernel_start = machine__kernel_start(thread->mg->machine);
+	else
+		ts->kernel_start = 1ULL << 63;
+	ts->crp = crp;
+
+	return 0;
+}
+
 static struct thread_stack *thread_stack__new(struct thread *thread,
 					      struct call_return_processor *crp)
 {
@@ -104,17 +122,11 @@ static struct thread_stack *thread_stack__new(struct thread *thread,
 
 	ts->arr_sz = 1;
 
-	if (thread_stack__grow(ts)) {
+	if (thread_stack__init(ts, thread, crp)) {
 		free(ts);
 		return NULL;
 	}
 
-	if (thread->mg && thread->mg->machine)
-		ts->kernel_start = machine__kernel_start(thread->mg->machine);
-	else
-		ts->kernel_start = 1ULL << 63;
-	ts->crp = crp;
-
 	thread->ts = ts;
 
 	return ts;

[tip:perf/urgent] perf thread-stack: Allocate an array of thread stacks

[tip-bot for Adrian Hunter]

Commit-ID:  139f42f3b3b495e61bb2cfef40e1dd5e845e3052
Gitweb:     https://git.kernel.org/tip/139f42f3b3b495e61bb2cfef40e1dd5e845e3052
Author:     Adrian Hunter <adrian.hunter@intel.com>
AuthorDate: Fri, 21 Dec 2018 14:06:18 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 2 Jan 2019 10:55:55 -0300

perf thread-stack: Allocate an array of thread stacks

In preparation for fixing thread stack processing for the idle task,
allocate an array of thread stacks.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Link: http://lkml.kernel.org/r/20181221120620.9659-7-adrian.hunter@intel.com
[ No need to check for NULL when calling zfree(), noticed by Jiri Olsa ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/thread-stack.c | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/tools/perf/util/thread-stack.c b/tools/perf/util/thread-stack.c
index 03770af9e5cd..248ed3945bec 100644
--- a/tools/perf/util/thread-stack.c
+++ b/tools/perf/util/thread-stack.c
@@ -114,20 +114,25 @@ static int thread_stack__init(struct thread_stack *ts, struct thread *thread,
 static struct thread_stack *thread_stack__new(struct thread *thread,
 					      struct call_return_processor *crp)
 {
-	struct thread_stack *ts;
-
-	ts = zalloc(sizeof(struct thread_stack));
-	if (!ts)
-		return NULL;
-
-	ts->arr_sz = 1;
-
-	if (thread_stack__init(ts, thread, crp)) {
-		free(ts);
-		return NULL;
+	struct thread_stack *ts = thread->ts, *new_ts;
+	unsigned int old_sz = ts ? ts->arr_sz : 0;
+	unsigned int new_sz = 1;
+
+	if (!ts || new_sz > old_sz) {
+		new_ts = calloc(new_sz, sizeof(*ts));
+		if (!new_ts)
+			return NULL;
+		if (ts)
+			memcpy(new_ts, ts, old_sz * sizeof(*ts));
+		new_ts->arr_sz = new_sz;
+		zfree(&thread->ts);
+		thread->ts = new_ts;
+		ts = new_ts;
 	}
 
-	thread->ts = ts;
+	if (!ts->stack &&
+	    thread_stack__init(ts, thread, crp))
+		return NULL;
 
 	return ts;
 }

[tip:perf/urgent] perf session: Add comment for perf_session__register_idle_thread()

[tip-bot for Adrian Hunter]

Commit-ID:  b25756df5b28cd7b6e91200fc5012e7c76e8ec69
Gitweb:     https://git.kernel.org/tip/b25756df5b28cd7b6e91200fc5012e7c76e8ec69
Author:     Adrian Hunter <adrian.hunter@intel.com>
AuthorDate: Fri, 21 Dec 2018 14:06:20 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 2 Jan 2019 11:05:06 -0300

perf session: Add comment for perf_session__register_idle_thread()

Add a comment to perf_session__register_idle_thread() to bring attention to
a pitfall with the idle task thread structure. The pitfall is that there
should really be a 'struct thread' for the idle task of each cpu, but there
is only one that can have pid == tid == 0.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Link: http://lkml.kernel.org/r/20181221120620.9659-9-adrian.hunter@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/session.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 78a067777144..5456c84c7dd1 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1527,6 +1527,13 @@ struct thread *perf_session__findnew(struct perf_session *session, pid_t pid)
 	return machine__findnew_thread(&session->machines.host, -1, pid);
 }
 
+/*
+ * Threads are identified by pid and tid, and the idle task has pid == tid == 0.
+ * So here a single thread is created for that, but actually there is a separate
+ * idle task per cpu, so there should be one 'struct thread' per cpu, but there
+ * is only 1. That causes problems for some tools, requiring workarounds. For
+ * example get_idle_thread() in builtin-sched.c, or thread_stack__per_cpu().
+ */
 int perf_session__register_idle_thread(struct perf_session *session)
 {
 	struct thread *thread;