[PATCH] perf report: Support caller callchain order when using DWARF unwinder.

[PATCH] perf report: Support caller callchain order when using DWARF unwinder.

[Milian Wolff]

We cannot reverse the order of the libunwind stepper. To workaround
this, we store the IPs in a temporary stack buffer and then walk
this buffer in reverse order when callchain_param.order is set to
ORDER_CALLER.

Signed-off-by: Milian Wolff <milian.wolff@kdab.com>

Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
---
 tools/perf/util/unwind-libunwind.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/unwind-libunwind.c b/tools/perf/util/unwind-libunwind.c
index 4c00507..bf631f1 100644
--- a/tools/perf/util/unwind-libunwind.c
+++ b/tools/perf/util/unwind-libunwind.c
@@ -621,11 +621,24 @@ static int get_entries(struct unwind_info *ui, unwind_entry_cb_t cb,
 	if (ret)
 		display_error(ret);
 
-	while (!ret && (unw_step(&c) > 0) && max_stack--) {
-		unw_word_t ip;
+	if (callchain_param.order == ORDER_CALLEE) {
+		while (!ret && (unw_step(&c) > 0) && max_stack--) {
+			unw_word_t ip;
 
-		unw_get_reg(&c, UNW_REG_IP, &ip);
-		ret = ip ? entry(ip, ui->thread, cb, arg) : 0;
+			unw_get_reg(&c, UNW_REG_IP, &ip);
+			ret = ip ? entry(ip, ui->thread, cb, arg) : 0;
+		}
+	} else {
+		unw_word_t ips[max_stack];
+		int i = 0;
+
+		while ((unw_step(&c) > 0) && i < max_stack) {
+			unw_get_reg(&c, UNW_REG_IP, &ips[i]);
+			++i;
+		}
+		max_stack = i;
+		for (i = max_stack - 1; i >= 0; --i)
+			entry(ips[i], ui->thread, cb, arg);
 	}
 
 	return ret;
-- 
2.6.0

Re: [PATCH] perf report: Support caller callchain order when using DWARF unwinder.

[Arnaldo Carvalho de Melo]

Em Sun, Oct 04, 2015 at 05:16:37PM +0200, Milian Wolff escreveu:
> We cannot reverse the order of the libunwind stepper. To workaround
> this, we store the IPs in a temporary stack buffer and then walk
> this buffer in reverse order when callchain_param.order is set to
> ORDER_CALLER.
> 
> Signed-off-by: Milian Wolff <milian.wolff@kdab.com>

Jiri,

	Can you please take a look at this?

- Arnaldo
 
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> ---
>  tools/perf/util/unwind-libunwind.c | 21 +++++++++++++++++----
>  1 file changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/perf/util/unwind-libunwind.c b/tools/perf/util/unwind-libunwind.c
> index 4c00507..bf631f1 100644
> --- a/tools/perf/util/unwind-libunwind.c
> +++ b/tools/perf/util/unwind-libunwind.c
> @@ -621,11 +621,24 @@ static int get_entries(struct unwind_info *ui, unwind_entry_cb_t cb,
>  	if (ret)
>  		display_error(ret);
>  
> -	while (!ret && (unw_step(&c) > 0) && max_stack--) {
> -		unw_word_t ip;
> +	if (callchain_param.order == ORDER_CALLEE) {
> +		while (!ret && (unw_step(&c) > 0) && max_stack--) {
> +			unw_word_t ip;
>  
> -		unw_get_reg(&c, UNW_REG_IP, &ip);
> -		ret = ip ? entry(ip, ui->thread, cb, arg) : 0;
> +			unw_get_reg(&c, UNW_REG_IP, &ip);
> +			ret = ip ? entry(ip, ui->thread, cb, arg) : 0;
> +		}
> +	} else {
> +		unw_word_t ips[max_stack];
> +		int i = 0;
> +
> +		while ((unw_step(&c) > 0) && i < max_stack) {
> +			unw_get_reg(&c, UNW_REG_IP, &ips[i]);
> +			++i;
> +		}
> +		max_stack = i;
> +		for (i = max_stack - 1; i >= 0; --i)
> +			entry(ips[i], ui->thread, cb, arg);
>  	}
>  
>  	return ret;
> -- 
> 2.6.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-perf-users" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] perf report: Support caller callchain order when using DWARF unwinder.

[Jiri Olsa]

On Sun, Oct 04, 2015 at 05:38:17PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Sun, Oct 04, 2015 at 05:16:37PM +0200, Milian Wolff escreveu:
> > We cannot reverse the order of the libunwind stepper. To workaround
> > this, we store the IPs in a temporary stack buffer and then walk
> > this buffer in reverse order when callchain_param.order is set to
> > ORDER_CALLER.
> > 
> > Signed-off-by: Milian Wolff <milian.wolff@kdab.com>
> 
> Jiri,
> 
> 	Can you please take a look at this?
> 
> - Arnaldo
>  
> > Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> > ---
> >  tools/perf/util/unwind-libunwind.c | 21 +++++++++++++++++----
> >  1 file changed, 17 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tools/perf/util/unwind-libunwind.c b/tools/perf/util/unwind-libunwind.c
> > index 4c00507..bf631f1 100644
> > --- a/tools/perf/util/unwind-libunwind.c
> > +++ b/tools/perf/util/unwind-libunwind.c
> > @@ -621,11 +621,24 @@ static int get_entries(struct unwind_info *ui, unwind_entry_cb_t cb,
> >  	if (ret)
> >  		display_error(ret);
> >  
> > -	while (!ret && (unw_step(&c) > 0) && max_stack--) {
> > -		unw_word_t ip;
> > +	if (callchain_param.order == ORDER_CALLEE) {
> > +		while (!ret && (unw_step(&c) > 0) && max_stack--) {
> > +			unw_word_t ip;
> >  
> > -		unw_get_reg(&c, UNW_REG_IP, &ip);
> > -		ret = ip ? entry(ip, ui->thread, cb, arg) : 0;
> > +			unw_get_reg(&c, UNW_REG_IP, &ip);
> > +			ret = ip ? entry(ip, ui->thread, cb, arg) : 0;
> > +		}
> > +	} else {
> > +		unw_word_t ips[max_stack];
> > +		int i = 0;
> > +
> > +		while ((unw_step(&c) > 0) && i < max_stack) {
> > +			unw_get_reg(&c, UNW_REG_IP, &ips[i]);
> > +			++i;
> > +		}
> > +		max_stack = i;
> > +		for (i = max_stack - 1; i >= 0; --i)
> > +			entry(ips[i], ui->thread, cb, arg);

there's no check for return value of entry callback

also I wonder would it be better to store into ips[] within
the single loop all the time, and iterate throught it after
forward/backward based on the callchain_param.order

please check attached patch, totaly untested, probably leaking some index ;-)

any chance this could be done also for util/unwind-libdw.c ?

thanks,
jirka


---
diff --git a/tools/perf/util/unwind-libunwind.c b/tools/perf/util/unwind-libunwind.c
index 4c00507ee3fd..f91433f868f9 100644
--- a/tools/perf/util/unwind-libunwind.c
+++ b/tools/perf/util/unwind-libunwind.c
@@ -609,9 +609,10 @@ void unwind__finish_access(struct thread *thread)
 static int get_entries(struct unwind_info *ui, unwind_entry_cb_t cb,
 		       void *arg, int max_stack)
 {
+	unw_word_t ips[max_stack];
 	unw_addr_space_t addr_space;
 	unw_cursor_t c;
-	int ret;
+	int ret, i = 0;
 
 	addr_space = thread__priv(ui->thread);
 	if (addr_space == NULL)
@@ -621,11 +622,19 @@ static int get_entries(struct unwind_info *ui, unwind_entry_cb_t cb,
 	if (ret)
 		display_error(ret);
 
-	while (!ret && (unw_step(&c) > 0) && max_stack--) {
-		unw_word_t ip;
+	while ((unw_step(&c) > 0) && i < max_stack) {
+		unw_get_reg(&c, UNW_REG_IP, &ips[i]);
+		++i;
+	}
+
+	max_stack = i;
+
+	for (i = 0; i < max_stack && !ret; i++) {
+		int j = i;
 
-		unw_get_reg(&c, UNW_REG_IP, &ip);
-		ret = ip ? entry(ip, ui->thread, cb, arg) : 0;
+		if (callchain_param.order == ORDER_CALLER)
+			j = max_stack - i - 1;
+		ret = entry(ips[j], ui->thread, cb, arg);
 	}
 
 	return ret;

Re: [PATCH] perf report: Support caller callchain order when using DWARF unwinder.

[Milian Wolff]

[-- Attachment #1: Type: text/plain, Size: 2824 bytes --]

On Montag, 5. Oktober 2015 13:08:36 CEST Jiri Olsa wrote:
> On Sun, Oct 04, 2015 at 05:38:17PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Sun, Oct 04, 2015 at 05:16:37PM +0200, Milian Wolff escreveu:
> > > We cannot reverse the order of the libunwind stepper. To workaround
> > > this, we store the IPs in a temporary stack buffer and then walk
> > > this buffer in reverse order when callchain_param.order is set to
> > > ORDER_CALLER.
> > > 
> > > Signed-off-by: Milian Wolff <milian.wolff@kdab.com>
> > 
> > Jiri,
> > 
> > 	Can you please take a look at this?
> > 
> > - Arnaldo
> > 
> > > Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> > > ---
> > > 
> > >  tools/perf/util/unwind-libunwind.c | 21 +++++++++++++++++----
> > >  1 file changed, 17 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/tools/perf/util/unwind-libunwind.c
> > > b/tools/perf/util/unwind-libunwind.c index 4c00507..bf631f1 100644
> > > --- a/tools/perf/util/unwind-libunwind.c
> > > +++ b/tools/perf/util/unwind-libunwind.c
> > > @@ -621,11 +621,24 @@ static int get_entries(struct unwind_info *ui,
> > > unwind_entry_cb_t cb,> > 
> > >  	if (ret)
> > >  	
> > >  		display_error(ret);
> > > 
> > > -	while (!ret && (unw_step(&c) > 0) && max_stack--) {
> > > -		unw_word_t ip;
> > > +	if (callchain_param.order == ORDER_CALLEE) {
> > > +		while (!ret && (unw_step(&c) > 0) && max_stack--) {
> > > +			unw_word_t ip;
> > > 
> > > -		unw_get_reg(&c, UNW_REG_IP, &ip);
> > > -		ret = ip ? entry(ip, ui->thread, cb, arg) : 0;
> > > +			unw_get_reg(&c, UNW_REG_IP, &ip);
> > > +			ret = ip ? entry(ip, ui->thread, cb, arg) : 0;
> > > +		}
> > > +	} else {
> > > +		unw_word_t ips[max_stack];
> > > +		int i = 0;
> > > +
> > > +		while ((unw_step(&c) > 0) && i < max_stack) {
> > > +			unw_get_reg(&c, UNW_REG_IP, &ips[i]);
> > > +			++i;
> > > +		}
> > > +		max_stack = i;
> > > +		for (i = max_stack - 1; i >= 0; --i)
> > > +			entry(ips[i], ui->thread, cb, arg);
> 
> there's no check for return value of entry callback
> 
> also I wonder would it be better to store into ips[] within
> the single loop all the time, and iterate throught it after
> forward/backward based on the callchain_param.order
> 
> please check attached patch, totaly untested, probably leaking some index
> ;-)
> 
> any chance this could be done also for util/unwind-libdw.c ?

That patch looks much better than mine. I'll try it out later next week and 
will also have a look at util/unwind-libdw.c. Question: How can I test the 
behavior of the latter? Do I need to uninstall libunwind, or can I change the 
unwinder at runtime somehow (env var?).

Also, are there unit tests for this behavior somewhere?
-- 
Milian Wolff | milian.wolff@kdab.com | Software Engineer
KDAB (Deutschland) GmbH&Co KG, a KDAB Group company
Tel: +49-30-521325470
KDAB - The Qt Experts

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5903 bytes --]

Re: [PATCH] perf report: Support caller callchain order when using DWARF unwinder.

[Arnaldo Carvalho de Melo]

Em Fri, Oct 09, 2015 at 07:29:34PM +0200, Milian Wolff escreveu:
> On Montag, 5. Oktober 2015 13:08:36 CEST Jiri Olsa wrote:
> > On Sun, Oct 04, 2015 at 05:38:17PM -0300, Arnaldo Carvalho de Melo wrote:
> > > Em Sun, Oct 04, 2015 at 05:16:37PM +0200, Milian Wolff escreveu:
> > > > We cannot reverse the order of the libunwind stepper. To workaround
> > > > this, we store the IPs in a temporary stack buffer and then walk
> > > > this buffer in reverse order when callchain_param.order is set to
> > > > ORDER_CALLER.
> > > > 
> > > > Signed-off-by: Milian Wolff <milian.wolff@kdab.com>
> > > 
> > > Jiri,
> > > 
> > > 	Can you please take a look at this?
> > > 
> > > - Arnaldo
> > > 
> > > > Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> > > > ---
> > > > 
> > > >  tools/perf/util/unwind-libunwind.c | 21 +++++++++++++++++----
> > > >  1 file changed, 17 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/tools/perf/util/unwind-libunwind.c
> > > > b/tools/perf/util/unwind-libunwind.c index 4c00507..bf631f1 100644
> > > > --- a/tools/perf/util/unwind-libunwind.c
> > > > +++ b/tools/perf/util/unwind-libunwind.c
> > > > @@ -621,11 +621,24 @@ static int get_entries(struct unwind_info *ui,
> > > > unwind_entry_cb_t cb,> > 
> > > >  	if (ret)
> > > >  	
> > > >  		display_error(ret);
> > > > 
> > > > -	while (!ret && (unw_step(&c) > 0) && max_stack--) {
> > > > -		unw_word_t ip;
> > > > +	if (callchain_param.order == ORDER_CALLEE) {
> > > > +		while (!ret && (unw_step(&c) > 0) && max_stack--) {
> > > > +			unw_word_t ip;
> > > > 
> > > > -		unw_get_reg(&c, UNW_REG_IP, &ip);
> > > > -		ret = ip ? entry(ip, ui->thread, cb, arg) : 0;
> > > > +			unw_get_reg(&c, UNW_REG_IP, &ip);
> > > > +			ret = ip ? entry(ip, ui->thread, cb, arg) : 0;
> > > > +		}
> > > > +	} else {
> > > > +		unw_word_t ips[max_stack];
> > > > +		int i = 0;
> > > > +
> > > > +		while ((unw_step(&c) > 0) && i < max_stack) {
> > > > +			unw_get_reg(&c, UNW_REG_IP, &ips[i]);
> > > > +			++i;
> > > > +		}
> > > > +		max_stack = i;
> > > > +		for (i = max_stack - 1; i >= 0; --i)
> > > > +			entry(ips[i], ui->thread, cb, arg);
> > 
> > there's no check for return value of entry callback
> > 
> > also I wonder would it be better to store into ips[] within
> > the single loop all the time, and iterate throught it after
> > forward/backward based on the callchain_param.order
> > 
> > please check attached patch, totaly untested, probably leaking some index
> > ;-)
> > 
> > any chance this could be done also for util/unwind-libdw.c ?
> 
> That patch looks much better than mine. I'll try it out later next week and 
> will also have a look at util/unwind-libdw.c. Question: How can I test the 

So, you tried this patch, right? Jiri, have you submitted this in some
other message I missed?

- Arnaldo

> behavior of the latter? Do I need to uninstall libunwind, or can I change the 
> unwinder at runtime somehow (env var?).
> 
> Also, are there unit tests for this behavior somewhere?
> -- 
> Milian Wolff | milian.wolff@kdab.com | Software Engineer
> KDAB (Deutschland) GmbH&Co KG, a KDAB Group company
> Tel: +49-30-521325470
> KDAB - The Qt Experts

Re: [PATCH] perf report: Support caller callchain order when using DWARF unwinder.

[Jiri Olsa]

On Mon, Nov 02, 2015 at 06:19:44PM -0300, Arnaldo Carvalho de Melo wrote:

SNIP

> > > 
> > > there's no check for return value of entry callback
> > > 
> > > also I wonder would it be better to store into ips[] within
> > > the single loop all the time, and iterate throught it after
> > > forward/backward based on the callchain_param.order
> > > 
> > > please check attached patch, totaly untested, probably leaking some index
> > > ;-)
> > > 
> > > any chance this could be done also for util/unwind-libdw.c ?
> > 
> > That patch looks much better than mine. I'll try it out later next week and 
> > will also have a look at util/unwind-libdw.c. Question: How can I test the 
> 
> So, you tried this patch, right? Jiri, have you submitted this in some
> other message I missed?

nope.. I thought Milian would take it ;-)

jirka

Re: [PATCH] perf report: Support caller callchain order when using DWARF unwinder.

[Arnaldo Carvalho de Melo]

Em Tue, Nov 03, 2015 at 08:33:25AM +0100, Jiri Olsa escreveu:
> On Mon, Nov 02, 2015 at 06:19:44PM -0300, Arnaldo Carvalho de Melo wrote:
> 
> SNIP
> 
> > > > 
> > > > there's no check for return value of entry callback
> > > > 
> > > > also I wonder would it be better to store into ips[] within
> > > > the single loop all the time, and iterate throught it after
> > > > forward/backward based on the callchain_param.order
> > > > 
> > > > please check attached patch, totaly untested, probably leaking some index
> > > > ;-)
> > > > 
> > > > any chance this could be done also for util/unwind-libdw.c ?
> > > 
> > > That patch looks much better than mine. I'll try it out later next week and 
> > > will also have a look at util/unwind-libdw.c. Question: How can I test the 
> > 
> > So, you tried this patch, right? Jiri, have you submitted this in some
> > other message I missed?
> 
> nope.. I thought Milian would take it ;-)

I can take it, as soon as you guys agree its something I should :-)

- Arnaldo

Re: [PATCH] perf report: Support caller callchain order when using DWARF unwinder.

[Milian Wolff]

[-- Attachment #1: Type: text/plain, Size: 1504 bytes --]

On Tuesday, November 3, 2015 9:06:31 AM CET Arnaldo Carvalho de Melo wrote:
> Em Tue, Nov 03, 2015 at 08:33:25AM +0100, Jiri Olsa escreveu:
> > On Mon, Nov 02, 2015 at 06:19:44PM -0300, Arnaldo Carvalho de Melo wrote:
> > 
> > SNIP
> > 
> > > > > there's no check for return value of entry callback
> > > > > 
> > > > > also I wonder would it be better to store into ips[] within
> > > > > the single loop all the time, and iterate throught it after
> > > > > forward/backward based on the callchain_param.order
> > > > > 
> > > > > please check attached patch, totaly untested, probably leaking some
> > > > > index
> > > > > ;-)
> > > > > 
> > > > > any chance this could be done also for util/unwind-libdw.c ?
> > > > 
> > > > That patch looks much better than mine. I'll try it out later next
> > > > week and
> > > > will also have a look at util/unwind-libdw.c. Question: How can I test
> > > > the
> > > 
> > > So, you tried this patch, right? Jiri, have you submitted this in some
> > > other message I missed?
> > 
> > nope.. I thought Milian would take it ;-)
> 
> I can take it, as soon as you guys agree its something I should :-)

Yes, I think it's good as-is. Should I resubmit Jiris patch? Considering that 
he rewrote the patch, should he send it and add me as tester? How do you 
handle such situations in the Kernel land?

Thanks
-- 
Milian Wolff | milian.wolff@kdab.com | Software Engineer
KDAB (Deutschland) GmbH&Co KG, a KDAB Group company
Tel: +49-30-521325470
KDAB - The Qt Experts

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5903 bytes --]

Re: [PATCH] perf report: Support caller callchain order when using DWARF unwinder.

[Arnaldo Carvalho de Melo]

Em Tue, Nov 03, 2015 at 01:54:58PM +0100, Milian Wolff escreveu:
> On Tuesday, November 3, 2015 9:06:31 AM CET Arnaldo Carvalho de Melo wrote:
> > Em Tue, Nov 03, 2015 at 08:33:25AM +0100, Jiri Olsa escreveu:
> > > On Mon, Nov 02, 2015 at 06:19:44PM -0300, Arnaldo Carvalho de Melo wrote:
> > > 
> > > SNIP
> > > 
> > > > > > there's no check for return value of entry callback
> > > > > > 
> > > > > > also I wonder would it be better to store into ips[] within
> > > > > > the single loop all the time, and iterate throught it after
> > > > > > forward/backward based on the callchain_param.order
> > > > > > 
> > > > > > please check attached patch, totaly untested, probably leaking some
> > > > > > index
> > > > > > ;-)
> > > > > > 
> > > > > > any chance this could be done also for util/unwind-libdw.c ?
> > > > > 
> > > > > That patch looks much better than mine. I'll try it out later next
> > > > > week and
> > > > > will also have a look at util/unwind-libdw.c. Question: How can I test
> > > > > the
> > > > 
> > > > So, you tried this patch, right? Jiri, have you submitted this in some
> > > > other message I missed?
> > > 
> > > nope.. I thought Milian would take it ;-)
> > 
> > I can take it, as soon as you guys agree its something I should :-)
> 
> Yes, I think it's good as-is. Should I resubmit Jiris patch? Considering that 
> he rewrote the patch, should he send it and add me as tester? How do you 
> handle such situations in the Kernel land?

So, I'll fix this all up by adding a Signed-off-by: Jiri and a
Tested-by: Millian, which I think I can do according to the above
messages and past experience, but the Correct Way to do this would
be for Jiri to collect your Tested-by and resubmit in a separate
message, not as a patch added to the text of a thread.

Thanks,

- Arnaldo

Re: [PATCH] perf report: Support caller callchain order when using DWARF unwinder.

[Arnaldo Carvalho de Melo]

Em Tue, Nov 03, 2015 at 11:28:37AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Nov 03, 2015 at 01:54:58PM +0100, Milian Wolff escreveu:
> > On Tuesday, November 3, 2015 9:06:31 AM CET Arnaldo Carvalho de Melo wrote:
> > > Em Tue, Nov 03, 2015 at 08:33:25AM +0100, Jiri Olsa escreveu:
> > > > On Mon, Nov 02, 2015 at 06:19:44PM -0300, Arnaldo Carvalho de Melo wrote:
> > > > > So, you tried this patch, right? Jiri, have you submitted this in some
> > > > > other message I missed?
> > > > 
> > > > nope.. I thought Milian would take it ;-)
> > > 
> > > I can take it, as soon as you guys agree its something I should :-)
> > 
> > Yes, I think it's good as-is. Should I resubmit Jiris patch? Considering that 
> > he rewrote the patch, should he send it and add me as tester? How do you 
> > handle such situations in the Kernel land?
> 
> So, I'll fix this all up by adding a Signed-off-by: Jiri and a

I take that back, can one of you please write a nice description to the
problem and how the proposed patch fixes it, then send as a separate
message with your Tested-by + Signed-off-by?

- Arnaldo

> Tested-by: Millian, which I think I can do according to the above
> messages and past experience, but the Correct Way to do this would
> be for Jiri to collect your Tested-by and resubmit in a separate
> message, not as a patch added to the text of a thread.

Re: [PATCH] perf report: Support caller callchain order when using DWARF unwinder.

[Jiri Olsa]

On Tue, Nov 03, 2015 at 11:28:37AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Nov 03, 2015 at 01:54:58PM +0100, Milian Wolff escreveu:
> > On Tuesday, November 3, 2015 9:06:31 AM CET Arnaldo Carvalho de Melo wrote:
> > > Em Tue, Nov 03, 2015 at 08:33:25AM +0100, Jiri Olsa escreveu:
> > > > On Mon, Nov 02, 2015 at 06:19:44PM -0300, Arnaldo Carvalho de Melo wrote:
> > > > 
> > > > SNIP
> > > > 
> > > > > > > there's no check for return value of entry callback
> > > > > > > 
> > > > > > > also I wonder would it be better to store into ips[] within
> > > > > > > the single loop all the time, and iterate throught it after
> > > > > > > forward/backward based on the callchain_param.order
> > > > > > > 
> > > > > > > please check attached patch, totaly untested, probably leaking some
> > > > > > > index
> > > > > > > ;-)
> > > > > > > 
> > > > > > > any chance this could be done also for util/unwind-libdw.c ?
> > > > > > 
> > > > > > That patch looks much better than mine. I'll try it out later next
> > > > > > week and
> > > > > > will also have a look at util/unwind-libdw.c. Question: How can I test
> > > > > > the
> > > > > 
> > > > > So, you tried this patch, right? Jiri, have you submitted this in some
> > > > > other message I missed?
> > > > 
> > > > nope.. I thought Milian would take it ;-)
> > > 
> > > I can take it, as soon as you guys agree its something I should :-)
> > 
> > Yes, I think it's good as-is. Should I resubmit Jiris patch? Considering that 
> > he rewrote the patch, should he send it and add me as tester? How do you 
> > handle such situations in the Kernel land?
> 
> So, I'll fix this all up by adding a Signed-off-by: Jiri and a
> Tested-by: Millian, which I think I can do according to the above
> messages and past experience, but the Correct Way to do this would
> be for Jiri to collect your Tested-by and resubmit in a separate
> message, not as a patch added to the text of a thread.

well, I'd rather take it then and test it.. I sent
it out completely untested.. just to show what I meant

I'll resend and add some test code

jirka

Re: [PATCH] perf report: Support caller callchain order when using DWARF unwinder.

[Arnaldo Carvalho de Melo]

Em Tue, Nov 03, 2015 at 03:32:07PM +0100, Jiri Olsa escreveu:
> On Tue, Nov 03, 2015 at 11:28:37AM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Tue, Nov 03, 2015 at 01:54:58PM +0100, Milian Wolff escreveu:
> > > On Tuesday, November 3, 2015 9:06:31 AM CET Arnaldo Carvalho de Melo wrote:
> > > > I can take it, as soon as you guys agree its something I should :-)
> > > 
> > > Yes, I think it's good as-is. Should I resubmit Jiris patch? Considering that 
> > > he rewrote the patch, should he send it and add me as tester? How do you 
> > > handle such situations in the Kernel land?

> > So, I'll fix this all up by adding a Signed-off-by: Jiri and a
> > Tested-by: Millian, which I think I can do according to the above
> > messages and past experience, but the Correct Way to do this would
> > be for Jiri to collect your Tested-by and resubmit in a separate
> > message, not as a patch added to the text of a thread.
 
> well, I'd rather take it then and test it.. I sent
> it out completely untested.. just to show what I meant
> 
> I'll resend and add some test code

Sounds like a plan, thanks in advance,

- Arnaldo

Re: [PATCH] perf report: Support caller callchain order when using DWARF unwinder.

[Jiri Olsa]

On Fri, Oct 09, 2015 at 07:29:34PM +0200, Milian Wolff wrote:

SNIP

> > ;-)
> > 
> > any chance this could be done also for util/unwind-libdw.c ?
> 
> That patch looks much better than mine. I'll try it out later next week and 
> will also have a look at util/unwind-libdw.c. Question: How can I test the 
> behavior of the latter? Do I need to uninstall libunwind, or can I change the 
> unwinder at runtime somehow (env var?).

It's either libunwind or libdw compiled in, so I'd suggest
generic test (see below) and compile with make variables
NO_LIBUNWIND=1 or NO_LIBDW_DWARF_UNWIND=1 to get desired
unwinder compiled in

> 
> Also, are there unit tests for this behavior somewhere?

please check tests/dwarf-unwind.c, I think you can add similar test

thanks,
jirka

Re: [PATCH] perf report: Support caller callchain order when using DWARF unwinder.

[Milian Wolff]

[-- Attachment #1: Type: text/plain, Size: 1282 bytes --]

On Tuesday, November 3, 2015 8:37:11 AM CET Jiri Olsa wrote:
> On Fri, Oct 09, 2015 at 07:29:34PM +0200, Milian Wolff wrote:
> 
> SNIP
> 
> > > ;-)
> > > 
> > > any chance this could be done also for util/unwind-libdw.c ?
> > 
> > That patch looks much better than mine. I'll try it out later next week
> > and
> > will also have a look at util/unwind-libdw.c. Question: How can I test the
> > behavior of the latter? Do I need to uninstall libunwind, or can I change
> > the unwinder at runtime somehow (env var?).
> 
> It's either libunwind or libdw compiled in, so I'd suggest
> generic test (see below) and compile with make variables
> NO_LIBUNWIND=1 or NO_LIBDW_DWARF_UNWIND=1 to get desired
> unwinder compiled in
> 
> > Also, are there unit tests for this behavior somewhere?
> 
> please check tests/dwarf-unwind.c, I think you can add similar test

OK, I'll try to find the time to test this later this week. I have also some 
more on my todo list regarding documentation and testing. If you don't hear 
back from me though, I'd appreciate if someone could adopt this patch and make 
sure it gets integrated.

Cheers
-- 
Milian Wolff | milian.wolff@kdab.com | Software Engineer
KDAB (Deutschland) GmbH&Co KG, a KDAB Group company
Tel: +49-30-521325470
KDAB - The Qt Experts

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5903 bytes --]