lkml.org 
[lkml]   [2013]   [Jan]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
SubjectRe: [PATCH 19/19] [INCOMPLETE] ARM: make return_address available for ARM_UNWIND
From
Hello guys,

Could you please review the patch of fixing bug first of returning
wrong address when using frame pointer?
I am wondering if the first patch is not delivered to the mailing.

~~~~~~~~~~~~~~~~~~~~~snip~~~~~~~~~~~~~~~~~~~~~~~~~
From 3a60b536d22a2043d735c890a9aac9e7cb72de8f Mon Sep 17 00:00:00 2001
From: sahara <keun-o.park@windriver.com>
Date: Thu, 3 Jan 2013 17:12:37 +0900
Subject: [PATCH 1/2] arm: fix returning wrong CALLER_ADDRx

This makes return_address return correct value for ftrace feature.
unwind_frame does not update frame->lr but frame->pc for backtrace.
And, the initialization for data.addr was missing so that wrong value
returned when unwind_frame failed.

Signed-off-by: sahara <keun-o.park@windriver.com>
---
arch/arm/kernel/return_address.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm/kernel/return_address.c b/arch/arm/kernel/return_address.c
index 8085417..fafedd8 100644
--- a/arch/arm/kernel/return_address.c
+++ b/arch/arm/kernel/return_address.c
@@ -26,7 +26,7 @@ static int save_return_addr(struct stackframe *frame, void *d)
struct return_address_data *data = d;

if (!data->level) {
- data->addr = (void *)frame->lr;
+ data->addr = (void *)frame->pc;

return 1;
} else {
@@ -41,7 +41,8 @@ void *return_address(unsigned int level)
struct stackframe frame;
register unsigned long current_sp asm ("sp");

- data.level = level + 1;
+ data.level = level + 2;
+ data.addr = NULL;

frame.fp = (unsigned long)__builtin_frame_address(0);
frame.sp = current_sp;
--
1.7.1
~~~~~~~~~~~~~~~~~~~~~snip~~~~~~~~~~~~~~~~~~~~~~~~~
Without this patch, when I added the following printk line and did
sync command,
it returned wrong return addresses using frame pointer.

Added line in __bdi_start_writeback():
+ printk("TEST: CALLER_ADDR0=(%pS), CALLER_ADDR1=(%pS),
CALLER_ADDR2=(%pS)\n", (void *)CALLER_ADDR0, (void *)CALLER_ADDR1,
(void *)CALLER_ADDR2);

Call sequence:
sys_sync() -> wakeup_flusher_threads() -> __bdi_start_writeback()

Result of sync after boot up:
~ # sync
TEST: CALLER_ADDR0=(wakeup_flusher_threads+0x9c/0xb8),
CALLER_ADDR1=(__bdi_start_writeback+0x30/0x120),
CALLER_ADDR2=(__bdi_start_writeback+0x3c/0x120)

As you see, the result of CALLER_ADDR1 and CALLER_ADDR2 is wrong.

With this patch, you will be able to see the following result.
~ # sync
TEST: CALLER_ADDR0=(wakeup_flusher_threads+0x9c/0xb8),
CALLER_ADDR1=(sys_sync+0x34/0xac),
CALLER_ADDR2=(ret_fast_syscall+0x0/0x48)

Based on this patch, if you apply the second patch which enables the
arm unwind,
and turning on CONFIG_ARM_UNWIND, you will see the correct result.

What I am currently concerning is if I use unwind info and ftrace's irqsoff,
I presume the ftrace might need architecture specific function to make
irqsoff work correctly.
For example, when I tried to test irqsoff, I got the message from
trace like the following.

cat-563 0d... 2us+: __irq_svc <-_raw_spin_unlock_irqrestore

Actually I could see the call sequence from the end of the trace.
~~~~~~~~~~~~~~~~~~snip~~~~~~~~~~~~~~~~~~
=> gic_handle_irq
=> __irq_svc
=> _raw_spin_unlock_irqrestore
=> uart_start
=> uart_write
~~~~~~~~~~~~~~~~~~snip~~~~~~~~~~~~~~~~~~
Seeing this call sequences, the output need to be
'_raw_spin_unlock_irqrestore <- uart_start'.
But, the CALLER_ADDR1 in trace_hardirqs_off() returned correct value.
So there's no problem in output.
I think trace_hardirqs_off() should call CALLER_ADDR1 and CALLER_ADDR2
respectively for its arguments for start_critical_timing(). This
thought leads me to the necessity of creating architecture specific
trace_hardirqs_off function. Any opinion on this?
-- kpark


\
 
 \ /
  Last update: 2013-01-28 04:01    [W:0.329 / U:8.000 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site