mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] Perf: support to unwind userspace application stacks generated with thumb.
@ 2020-04-16  5:38 Jiping Ma
  2020-04-16  7:10 ` Will Deacon
  0 siblings, 1 reply; 4+ messages in thread
From: Jiping Ma @ 2020-04-16  5:38 UTC (permalink / raw)
  To: will.deacon, mark.rutland, catalin.marinas, alexander.shishkin,
	jolsa, namhyung, peterz, mingo, acme
  Cc: linux-kernel, linux-arm-kernel, jiping.ma2

Only push sp and lr in the stack for thumb mode. it will go
through the stack find sp and lr.

Change this to the more detailed description of the patch

Signed-off-by: Jiping Ma <jiping.ma2@windriver.com>
---
 arch/arm64/kernel/perf_callchain.c | 36 +++++++++++++++++++++++++++---
 1 file changed, 33 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kernel/perf_callchain.c b/arch/arm64/kernel/perf_callchain.c
index bcafd7dcfe8b..97dde271c121 100644
--- a/arch/arm64/kernel/perf_callchain.c
+++ b/arch/arm64/kernel/perf_callchain.c
@@ -104,6 +104,30 @@ compat_user_backtrace(struct compat_frame_tail __user *tail,
 
 	return (struct compat_frame_tail __user *)compat_ptr(buftail.fp) - 1;
 }
+
+void
+user_backtrace_thumb(struct perf_callchain_entry_ctx *entry,
+		     struct pt_regs *regs)
+{
+	u32 sp;
+	u32 *sp_t;
+	/*
+	 * Only push sp, lr to stack.
+	 */
+	for (sp = regs->compat_sp; (sp < current->mm->start_stack) &&
+		(entry->nr < entry->max_stack); sp += 4) {
+		sp_t = (u32 *)(unsigned long)sp;
+		if ((*sp_t > regs->compat_sp) &&
+			(*sp_t < current->mm->start_stack)) {
+			if (*(sp_t + 1) < current->mm->end_code &&
+				*(sp_t + 1) > current->mm->start_code) {
+				perf_callchain_store(entry,  *(sp_t + 1)-1);
+				sp += 4;
+			}
+		}
+	}
+}
+
 #endif /* CONFIG_COMPAT */
 
 void perf_callchain_user(struct perf_callchain_entry_ctx *entry,
@@ -132,9 +156,15 @@ void perf_callchain_user(struct perf_callchain_entry_ctx *entry,
 
 		tail = (struct compat_frame_tail __user *)regs->compat_fp - 1;
 
-		while ((entry->nr < entry->max_stack) &&
-			tail && !((unsigned long)tail & 0x3))
-			tail = compat_user_backtrace(tail, entry);
+		if (((unsigned long)tail > current->mm->start_stack) ||
+			((unsigned long)tail < regs->compat_sp) ||
+			regs->compat_sp == regs->compat_usr(7)) {
+			user_backtrace_thumb(entry, regs);
+		} else {
+			while ((entry->nr < entry->max_stack) &&
+				tail && !((unsigned long)tail & 0x3))
+				tail = compat_user_backtrace(tail, entry);
+		}
 #endif
 	}
 }
-- 
2.18.1


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

* Re: [PATCH] Perf: support to unwind userspace application stacks generated with thumb.
  2020-04-16  5:38 [PATCH] Perf: support to unwind userspace application stacks generated with thumb Jiping Ma
@ 2020-04-16  7:10 ` Will Deacon
  2020-04-22  2:56   ` Jiping Ma
  0 siblings, 1 reply; 4+ messages in thread
From: Will Deacon @ 2020-04-16  7:10 UTC (permalink / raw)
  To: Jiping Ma
  Cc: will.deacon, mark.rutland, catalin.marinas, alexander.shishkin,
	jolsa, namhyung, peterz, mingo, acme, linux-kernel,
	linux-arm-kernel

On Thu, Apr 16, 2020 at 01:38:29PM +0800, Jiping Ma wrote:
> Only push sp and lr in the stack for thumb mode. it will go
> through the stack find sp and lr.
> 
> Change this to the more detailed description of the patch
> 
> Signed-off-by: Jiping Ma <jiping.ma2@windriver.com>
> ---
>  arch/arm64/kernel/perf_callchain.c | 36 +++++++++++++++++++++++++++---
>  1 file changed, 33 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/kernel/perf_callchain.c b/arch/arm64/kernel/perf_callchain.c
> index bcafd7dcfe8b..97dde271c121 100644
> --- a/arch/arm64/kernel/perf_callchain.c
> +++ b/arch/arm64/kernel/perf_callchain.c
> @@ -104,6 +104,30 @@ compat_user_backtrace(struct compat_frame_tail __user *tail,
>  
>  	return (struct compat_frame_tail __user *)compat_ptr(buftail.fp) - 1;
>  }
> +
> +void
> +user_backtrace_thumb(struct perf_callchain_entry_ctx *entry,
> +		     struct pt_regs *regs)
> +{
> +	u32 sp;
> +	u32 *sp_t;
> +	/*
> +	 * Only push sp, lr to stack.
> +	 */
> +	for (sp = regs->compat_sp; (sp < current->mm->start_stack) &&
> +		(entry->nr < entry->max_stack); sp += 4) {
> +		sp_t = (u32 *)(unsigned long)sp;
> +		if ((*sp_t > regs->compat_sp) &&
> +			(*sp_t < current->mm->start_stack)) {
> +			if (*(sp_t + 1) < current->mm->end_code &&
> +				*(sp_t + 1) > current->mm->start_code) {
> +				perf_callchain_store(entry,  *(sp_t + 1)-1);
> +				sp += 4;
> +			}
> +		}
> +	}
> +}

This looks like a pile of fragile heuristics to me. Why don't you just use
libunwind in userspace, the same way you'd have to if you compiled without
framepointers?

Will

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

* Re: [PATCH] Perf: support to unwind userspace application stacks generated with thumb.
  2020-04-16  7:10 ` Will Deacon
@ 2020-04-22  2:56   ` Jiping Ma
  2020-04-22  8:19     ` Will Deacon
  0 siblings, 1 reply; 4+ messages in thread
From: Jiping Ma @ 2020-04-22  2:56 UTC (permalink / raw)
  To: Will Deacon
  Cc: will.deacon, mark.rutland, catalin.marinas, alexander.shishkin,
	jolsa, namhyung, peterz, mingo, acme, linux-kernel,
	linux-arm-kernel



On 04/16/2020 03:10 PM, Will Deacon wrote:
> On Thu, Apr 16, 2020 at 01:38:29PM +0800, Jiping Ma wrote:
>> Only push sp and lr in the stack for thumb mode. it will go
>> through the stack find sp and lr.
>>
>> Change this to the more detailed description of the patch
>>
>> Signed-off-by: Jiping Ma <jiping.ma2@windriver.com>
>> ---
>>   arch/arm64/kernel/perf_callchain.c | 36 +++++++++++++++++++++++++++---
>>   1 file changed, 33 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm64/kernel/perf_callchain.c b/arch/arm64/kernel/perf_callchain.c
>> index bcafd7dcfe8b..97dde271c121 100644
>> --- a/arch/arm64/kernel/perf_callchain.c
>> +++ b/arch/arm64/kernel/perf_callchain.c
>> @@ -104,6 +104,30 @@ compat_user_backtrace(struct compat_frame_tail __user *tail,
>>   
>>   	return (struct compat_frame_tail __user *)compat_ptr(buftail.fp) - 1;
>>   }
>> +
>> +void
>> +user_backtrace_thumb(struct perf_callchain_entry_ctx *entry,
>> +		     struct pt_regs *regs)
>> +{
>> +	u32 sp;
>> +	u32 *sp_t;
>> +	/*
>> +	 * Only push sp, lr to stack.
>> +	 */
>> +	for (sp = regs->compat_sp; (sp < current->mm->start_stack) &&
>> +		(entry->nr < entry->max_stack); sp += 4) {
>> +		sp_t = (u32 *)(unsigned long)sp;
>> +		if ((*sp_t > regs->compat_sp) &&
>> +			(*sp_t < current->mm->start_stack)) {
>> +			if (*(sp_t + 1) < current->mm->end_code &&
>> +				*(sp_t + 1) > current->mm->start_code) {
>> +				perf_callchain_store(entry,  *(sp_t + 1)-1);
>> +				sp += 4;
>> +			}
>> +		}
>> +	}
>> +}
> This looks like a pile of fragile heuristics to me. Why don't you just use
> libunwind in userspace, the same way you'd have to if you compiled without
> framepointers?
Hi, Will

Could you share more details for libunwind in userspace? Following is 
our build command for test app.
bt_perf.thumb: arm-wrs-linux-gnueabi-gcc -march=armv7ve -mthumb 
-mfpu=neon -mfloat-abi=softfp -mcpu=cortex-a15 
--sysroot=sysroots/cortexa15t2-neon-wrs-linux-gnueabi -O0 -g -ggdb -Wall 
-funwind-tables -fno-omit-frame-pointer -frecord-gcc-switches 
-mapcs-frame bt_perf.c -o bt_perf.thumb

Thanks,
Jiping

>
> Will
>


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

* Re: [PATCH] Perf: support to unwind userspace application stacks generated with thumb.
  2020-04-22  2:56   ` Jiping Ma
@ 2020-04-22  8:19     ` Will Deacon
  0 siblings, 0 replies; 4+ messages in thread
From: Will Deacon @ 2020-04-22  8:19 UTC (permalink / raw)
  To: Jiping Ma
  Cc: will.deacon, mark.rutland, catalin.marinas, alexander.shishkin,
	jolsa, namhyung, peterz, mingo, acme, linux-kernel,
	linux-arm-kernel

On Wed, Apr 22, 2020 at 10:56:04AM +0800, Jiping Ma wrote:
> On 04/16/2020 03:10 PM, Will Deacon wrote:
> > On Thu, Apr 16, 2020 at 01:38:29PM +0800, Jiping Ma wrote:
> > > +void
> > > +user_backtrace_thumb(struct perf_callchain_entry_ctx *entry,
> > > +		     struct pt_regs *regs)
> > > +{
> > > +	u32 sp;
> > > +	u32 *sp_t;
> > > +	/*
> > > +	 * Only push sp, lr to stack.
> > > +	 */
> > > +	for (sp = regs->compat_sp; (sp < current->mm->start_stack) &&
> > > +		(entry->nr < entry->max_stack); sp += 4) {
> > > +		sp_t = (u32 *)(unsigned long)sp;
> > > +		if ((*sp_t > regs->compat_sp) &&
> > > +			(*sp_t < current->mm->start_stack)) {
> > > +			if (*(sp_t + 1) < current->mm->end_code &&
> > > +				*(sp_t + 1) > current->mm->start_code) {
> > > +				perf_callchain_store(entry,  *(sp_t + 1)-1);
> > > +				sp += 4;
> > > +			}
> > > +		}
> > > +	}
> > > +}
> > This looks like a pile of fragile heuristics to me. Why don't you just use
> > libunwind in userspace, the same way you'd have to if you compiled without
> > framepointers?
> 
> Could you share more details for libunwind in userspace? Following is our
> build command for test app.
> bt_perf.thumb: arm-wrs-linux-gnueabi-gcc -march=armv7ve -mthumb -mfpu=neon
> -mfloat-abi=softfp -mcpu=cortex-a15
> --sysroot=sysroots/cortexa15t2-neon-wrs-linux-gnueabi -O0 -g -ggdb -Wall
> -funwind-tables -fno-omit-frame-pointer -frecord-gcc-switches -mapcs-frame
> bt_perf.c -o bt_perf.thumb

You can build the perf tool with support for libunwind, and then it should
handle the unwinding in userspace. I haven't used it for ages, but you may
need to pass '--call-graph dwarf'. Have a play (and you can also read the
code in tools/perf/).

Will

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

end of thread, other threads:[~2020-04-22  8:19 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-16  5:38 [PATCH] Perf: support to unwind userspace application stacks generated with thumb Jiping Ma
2020-04-16  7:10 ` Will Deacon
2020-04-22  2:56   ` Jiping Ma
2020-04-22  8:19     ` Will Deacon

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

all inboxes | Powered by JetHome®