diff mbox

[v8,1/8] ftrace: make CALLER_ADDRx macros more generic

Message ID 1398851676-16389-2-git-send-email-takahiro.akashi@linaro.org
State Superseded
Headers show

Commit Message

AKASHI Takahiro April 30, 2014, 9:54 a.m. UTC
Most archs with HAVE_ARCH_CALLER_ADDR have the almost same definitions
of CALLER_ADDRx(n), and so put them into linux/ftrace.h.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 arch/arm/include/asm/ftrace.h      |   10 +---------
 arch/blackfin/include/asm/ftrace.h |   11 +----------
 arch/parisc/include/asm/ftrace.h   |   10 +---------
 arch/sh/include/asm/ftrace.h       |   10 +---------
 arch/xtensa/include/asm/ftrace.h   |   14 ++++----------
 include/linux/ftrace.h             |   34 ++++++++++++++++++----------------
 6 files changed, 26 insertions(+), 63 deletions(-)

Comments

Will Deacon May 2, 2014, 6:13 p.m. UTC | #1
Hi Akashi,

On Wed, Apr 30, 2014 at 10:54:29AM +0100, AKASHI Takahiro wrote:
> Most archs with HAVE_ARCH_CALLER_ADDR have the almost same definitions
> of CALLER_ADDRx(n), and so put them into linux/ftrace.h.
> 
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>  arch/arm/include/asm/ftrace.h      |   10 +---------
>  arch/blackfin/include/asm/ftrace.h |   11 +----------
>  arch/parisc/include/asm/ftrace.h   |   10 +---------
>  arch/sh/include/asm/ftrace.h       |   10 +---------
>  arch/xtensa/include/asm/ftrace.h   |   14 ++++----------
>  include/linux/ftrace.h             |   34 ++++++++++++++++++----------------
>  6 files changed, 26 insertions(+), 63 deletions(-)

This one looks a bit too widespread to be merged via the arm64 tree. I
wonder if the ftrace maintainers would consider taking it as a cleanup?

Will

> diff --git a/arch/arm/include/asm/ftrace.h b/arch/arm/include/asm/ftrace.h
> index f89515a..eb577f4 100644
> --- a/arch/arm/include/asm/ftrace.h
> +++ b/arch/arm/include/asm/ftrace.h
> @@ -52,15 +52,7 @@ extern inline void *return_address(unsigned int level)
>  
>  #endif
>  
> -#define HAVE_ARCH_CALLER_ADDR
> -
> -#define CALLER_ADDR0 ((unsigned long)__builtin_return_address(0))
> -#define CALLER_ADDR1 ((unsigned long)return_address(1))
> -#define CALLER_ADDR2 ((unsigned long)return_address(2))
> -#define CALLER_ADDR3 ((unsigned long)return_address(3))
> -#define CALLER_ADDR4 ((unsigned long)return_address(4))
> -#define CALLER_ADDR5 ((unsigned long)return_address(5))
> -#define CALLER_ADDR6 ((unsigned long)return_address(6))
> +#define ftrace_return_addr(n) return_address(n)
>  
>  #endif /* ifndef __ASSEMBLY__ */
>  
> diff --git a/arch/blackfin/include/asm/ftrace.h b/arch/blackfin/include/asm/ftrace.h
> index 8a02950..2f1c3c2 100644
> --- a/arch/blackfin/include/asm/ftrace.h
> +++ b/arch/blackfin/include/asm/ftrace.h
> @@ -66,16 +66,7 @@ extern inline void *return_address(unsigned int level)
>  
>  #endif /* CONFIG_FRAME_POINTER */
>  
> -#define HAVE_ARCH_CALLER_ADDR
> -
> -/* inline function or macro may lead to unexpected result */
> -#define CALLER_ADDR0 ((unsigned long)__builtin_return_address(0))
> -#define CALLER_ADDR1 ((unsigned long)return_address(1))
> -#define CALLER_ADDR2 ((unsigned long)return_address(2))
> -#define CALLER_ADDR3 ((unsigned long)return_address(3))
> -#define CALLER_ADDR4 ((unsigned long)return_address(4))
> -#define CALLER_ADDR5 ((unsigned long)return_address(5))
> -#define CALLER_ADDR6 ((unsigned long)return_address(6))
> +#define ftrace_return_address(n) return_address(n)
>  
>  #endif /* __ASSEMBLY__ */
>  
> diff --git a/arch/parisc/include/asm/ftrace.h b/arch/parisc/include/asm/ftrace.h
> index 72c0faf..544ed8e 100644
> --- a/arch/parisc/include/asm/ftrace.h
> +++ b/arch/parisc/include/asm/ftrace.h
> @@ -24,15 +24,7 @@ extern void return_to_handler(void);
>  
>  extern unsigned long return_address(unsigned int);
>  
> -#define HAVE_ARCH_CALLER_ADDR
> -
> -#define CALLER_ADDR0 ((unsigned long)__builtin_return_address(0))
> -#define CALLER_ADDR1 return_address(1)
> -#define CALLER_ADDR2 return_address(2)
> -#define CALLER_ADDR3 return_address(3)
> -#define CALLER_ADDR4 return_address(4)
> -#define CALLER_ADDR5 return_address(5)
> -#define CALLER_ADDR6 return_address(6)
> +#define ftrace_return_address(n) return_address(n)
>  
>  #endif /* __ASSEMBLY__ */
>  
> diff --git a/arch/sh/include/asm/ftrace.h b/arch/sh/include/asm/ftrace.h
> index 13e9966..e79fb6e 100644
> --- a/arch/sh/include/asm/ftrace.h
> +++ b/arch/sh/include/asm/ftrace.h
> @@ -40,15 +40,7 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr)
>  /* arch/sh/kernel/return_address.c */
>  extern void *return_address(unsigned int);
>  
> -#define HAVE_ARCH_CALLER_ADDR
> -
> -#define CALLER_ADDR0 ((unsigned long)__builtin_return_address(0))
> -#define CALLER_ADDR1 ((unsigned long)return_address(1))
> -#define CALLER_ADDR2 ((unsigned long)return_address(2))
> -#define CALLER_ADDR3 ((unsigned long)return_address(3))
> -#define CALLER_ADDR4 ((unsigned long)return_address(4))
> -#define CALLER_ADDR5 ((unsigned long)return_address(5))
> -#define CALLER_ADDR6 ((unsigned long)return_address(6))
> +#define ftrace_return_address(n) return_address(n)
>  
>  #endif /* __ASSEMBLY__ */
>  
> diff --git a/arch/xtensa/include/asm/ftrace.h b/arch/xtensa/include/asm/ftrace.h
> index 736b9d2..6c6d9a9 100644
> --- a/arch/xtensa/include/asm/ftrace.h
> +++ b/arch/xtensa/include/asm/ftrace.h
> @@ -12,24 +12,18 @@
>  
>  #include <asm/processor.h>
>  
> -#define HAVE_ARCH_CALLER_ADDR
>  #ifndef __ASSEMBLY__
> -#define CALLER_ADDR0 ({ unsigned long a0, a1; \
> +#define ftrace_return_address0 ({ unsigned long a0, a1; \
>  		__asm__ __volatile__ ( \
>  			"mov %0, a0\n" \
>  			"mov %1, a1\n" \
>  			: "=r"(a0), "=r"(a1)); \
>  		MAKE_PC_FROM_RA(a0, a1); })
> +
>  #ifdef CONFIG_FRAME_POINTER
>  extern unsigned long return_address(unsigned level);
> -#define CALLER_ADDR1 return_address(1)
> -#define CALLER_ADDR2 return_address(2)
> -#define CALLER_ADDR3 return_address(3)
> -#else /* CONFIG_FRAME_POINTER */
> -#define CALLER_ADDR1 (0)
> -#define CALLER_ADDR2 (0)
> -#define CALLER_ADDR3 (0)
> -#endif /* CONFIG_FRAME_POINTER */
> +#define ftrace_return_address(n) return_address(n)
> +#endif
>  #endif /* __ASSEMBLY__ */
>  
>  #ifdef CONFIG_FUNCTION_TRACER
> diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> index 9212b01..18f1ae7 100644
> --- a/include/linux/ftrace.h
> +++ b/include/linux/ftrace.h
> @@ -614,25 +614,27 @@ static inline void __ftrace_enabled_restore(int enabled)
>  #endif
>  }
>  
> -#ifndef HAVE_ARCH_CALLER_ADDR
> +/* All archs should have this, but we define it for consistency */
> +#ifndef ftrace_return_address0
> +# define ftrace_return_address0 __builtin_return_address(0)
> +#endif
> +
> +/* Archs may use other ways for ADDR1 and beyond */
> +#ifndef ftrace_return_address
>  # ifdef CONFIG_FRAME_POINTER
> -#  define CALLER_ADDR0 ((unsigned long)__builtin_return_address(0))
> -#  define CALLER_ADDR1 ((unsigned long)__builtin_return_address(1))
> -#  define CALLER_ADDR2 ((unsigned long)__builtin_return_address(2))
> -#  define CALLER_ADDR3 ((unsigned long)__builtin_return_address(3))
> -#  define CALLER_ADDR4 ((unsigned long)__builtin_return_address(4))
> -#  define CALLER_ADDR5 ((unsigned long)__builtin_return_address(5))
> -#  define CALLER_ADDR6 ((unsigned long)__builtin_return_address(6))
> +#  define ftrace_return_address(n) __builtin_return_address(n)
>  # else
> -#  define CALLER_ADDR0 ((unsigned long)__builtin_return_address(0))
> -#  define CALLER_ADDR1 0UL
> -#  define CALLER_ADDR2 0UL
> -#  define CALLER_ADDR3 0UL
> -#  define CALLER_ADDR4 0UL
> -#  define CALLER_ADDR5 0UL
> -#  define CALLER_ADDR6 0UL
> +#  define ftrace_return_address(n) 0UL
>  # endif
> -#endif /* ifndef HAVE_ARCH_CALLER_ADDR */
> +#endif
> +
> +#define CALLER_ADDR0 ((unsigned long)ftrace_return_address0)
> +#define CALLER_ADDR1 ((unsigned long)ftrace_return_address(1))
> +#define CALLER_ADDR2 ((unsigned long)ftrace_return_address(2))
> +#define CALLER_ADDR3 ((unsigned long)ftrace_return_address(3))
> +#define CALLER_ADDR4 ((unsigned long)ftrace_return_address(4))
> +#define CALLER_ADDR5 ((unsigned long)ftrace_return_address(5))
> +#define CALLER_ADDR6 ((unsigned long)ftrace_return_address(6))
>  
>  #ifdef CONFIG_IRQSOFF_TRACER
>    extern void time_hardirqs_on(unsigned long a0, unsigned long a1);
> -- 
> 1.7.9.5
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Steven Rostedt May 2, 2014, 7:02 p.m. UTC | #2
On Fri, 2 May 2014 19:13:48 +0100
Will Deacon <will.deacon@arm.com> wrote:

> Hi Akashi,
> 
> On Wed, Apr 30, 2014 at 10:54:29AM +0100, AKASHI Takahiro wrote:
> > Most archs with HAVE_ARCH_CALLER_ADDR have the almost same definitions
> > of CALLER_ADDRx(n), and so put them into linux/ftrace.h.
> > 
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> >  arch/arm/include/asm/ftrace.h      |   10 +---------
> >  arch/blackfin/include/asm/ftrace.h |   11 +----------
> >  arch/parisc/include/asm/ftrace.h   |   10 +---------
> >  arch/sh/include/asm/ftrace.h       |   10 +---------
> >  arch/xtensa/include/asm/ftrace.h   |   14 ++++----------
> >  include/linux/ftrace.h             |   34 ++++++++++++++++++----------------
> >  6 files changed, 26 insertions(+), 63 deletions(-)
> 
> This one looks a bit too widespread to be merged via the arm64 tree. I
> wonder if the ftrace maintainers would consider taking it as a cleanup?
> 

I actually was the one to recommend this approach. But I have some
small comments to the patch. I'll reply directly to the patch with them.

-- Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Steven Rostedt May 2, 2014, 7:03 p.m. UTC | #3
On Fri, 2 May 2014 19:13:48 +0100
Will Deacon <will.deacon@arm.com> wrote:

> This one looks a bit too widespread to be merged via the arm64 tree. I
> wonder if the ftrace maintainers would consider taking it as a cleanup?

Oh, and I can take the patch if you feel uncomfortable with taking
something so spread out. As it deals with ftrace I probably can get
away with a cross the board arch change to it.

-- Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Steven Rostedt May 2, 2014, 7:19 p.m. UTC | #4
On Wed, 30 Apr 2014 18:54:29 +0900
AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:

> Most archs with HAVE_ARCH_CALLER_ADDR have the almost same definitions
> of CALLER_ADDRx(n), and so put them into linux/ftrace.h.

Please add a bit more to the change log. Like what you did. Something
like:

----
Most archs with HAVE_ARCH_CALLER_ADDR have pretty much the same
definitions of CALLER_ADDRx(n). Instead of duplicating the code for all
the archs, define a ftrace_return_address0() and
ftrace_return_address(n) that can be overwritten by the archs if they
need to do something different. Instead of 7 macros in every arch, we
now only have at most 2 (and actually only 1 as
ftrace_return_address0() should be the same for all archs).

The CALLER_ADDRx(n) will now be defined in linux/ftrace.h and use the
ftrace_return_address*(n?) macros. This removes a lot of the duplicate
code.
-----

Use that if you want :-)


> 
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>  arch/arm/include/asm/ftrace.h      |   10 +---------
>  arch/blackfin/include/asm/ftrace.h |   11 +----------
>  arch/parisc/include/asm/ftrace.h   |   10 +---------
>  arch/sh/include/asm/ftrace.h       |   10 +---------
>  arch/xtensa/include/asm/ftrace.h   |   14 ++++----------
>  include/linux/ftrace.h             |   34 ++++++++++++++++++----------------
>  6 files changed, 26 insertions(+), 63 deletions(-)
> 
> diff --git a/arch/arm/include/asm/ftrace.h b/arch/arm/include/asm/ftrace.h
> index f89515a..eb577f4 100644
> --- a/arch/arm/include/asm/ftrace.h
> +++ b/arch/arm/include/asm/ftrace.h
> @@ -52,15 +52,7 @@ extern inline void *return_address(unsigned int level)
>  
>  #endif
>  
> -#define HAVE_ARCH_CALLER_ADDR
> -
> -#define CALLER_ADDR0 ((unsigned long)__builtin_return_address(0))
> -#define CALLER_ADDR1 ((unsigned long)return_address(1))
> -#define CALLER_ADDR2 ((unsigned long)return_address(2))
> -#define CALLER_ADDR3 ((unsigned long)return_address(3))
> -#define CALLER_ADDR4 ((unsigned long)return_address(4))
> -#define CALLER_ADDR5 ((unsigned long)return_address(5))
> -#define CALLER_ADDR6 ((unsigned long)return_address(6))
> +#define ftrace_return_addr(n) return_address(n)
>  
>  #endif /* ifndef __ASSEMBLY__ */
>  
> diff --git a/arch/blackfin/include/asm/ftrace.h b/arch/blackfin/include/asm/ftrace.h
> index 8a02950..2f1c3c2 100644
> --- a/arch/blackfin/include/asm/ftrace.h
> +++ b/arch/blackfin/include/asm/ftrace.h
> @@ -66,16 +66,7 @@ extern inline void *return_address(unsigned int level)
>  
>  #endif /* CONFIG_FRAME_POINTER */
>  
> -#define HAVE_ARCH_CALLER_ADDR
> -
> -/* inline function or macro may lead to unexpected result */
> -#define CALLER_ADDR0 ((unsigned long)__builtin_return_address(0))
> -#define CALLER_ADDR1 ((unsigned long)return_address(1))
> -#define CALLER_ADDR2 ((unsigned long)return_address(2))
> -#define CALLER_ADDR3 ((unsigned long)return_address(3))
> -#define CALLER_ADDR4 ((unsigned long)return_address(4))
> -#define CALLER_ADDR5 ((unsigned long)return_address(5))
> -#define CALLER_ADDR6 ((unsigned long)return_address(6))
> +#define ftrace_return_address(n) return_address(n)
>  
>  #endif /* __ASSEMBLY__ */
>  
> diff --git a/arch/parisc/include/asm/ftrace.h b/arch/parisc/include/asm/ftrace.h
> index 72c0faf..544ed8e 100644
> --- a/arch/parisc/include/asm/ftrace.h
> +++ b/arch/parisc/include/asm/ftrace.h
> @@ -24,15 +24,7 @@ extern void return_to_handler(void);
>  
>  extern unsigned long return_address(unsigned int);
>  
> -#define HAVE_ARCH_CALLER_ADDR
> -
> -#define CALLER_ADDR0 ((unsigned long)__builtin_return_address(0))
> -#define CALLER_ADDR1 return_address(1)
> -#define CALLER_ADDR2 return_address(2)
> -#define CALLER_ADDR3 return_address(3)
> -#define CALLER_ADDR4 return_address(4)
> -#define CALLER_ADDR5 return_address(5)
> -#define CALLER_ADDR6 return_address(6)
> +#define ftrace_return_address(n) return_address(n)
>  
>  #endif /* __ASSEMBLY__ */
>  
> diff --git a/arch/sh/include/asm/ftrace.h b/arch/sh/include/asm/ftrace.h
> index 13e9966..e79fb6e 100644
> --- a/arch/sh/include/asm/ftrace.h
> +++ b/arch/sh/include/asm/ftrace.h
> @@ -40,15 +40,7 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr)
>  /* arch/sh/kernel/return_address.c */
>  extern void *return_address(unsigned int);
>  
> -#define HAVE_ARCH_CALLER_ADDR
> -
> -#define CALLER_ADDR0 ((unsigned long)__builtin_return_address(0))
> -#define CALLER_ADDR1 ((unsigned long)return_address(1))
> -#define CALLER_ADDR2 ((unsigned long)return_address(2))
> -#define CALLER_ADDR3 ((unsigned long)return_address(3))
> -#define CALLER_ADDR4 ((unsigned long)return_address(4))
> -#define CALLER_ADDR5 ((unsigned long)return_address(5))
> -#define CALLER_ADDR6 ((unsigned long)return_address(6))
> +#define ftrace_return_address(n) return_address(n)
>  
>  #endif /* __ASSEMBLY__ */
>  
> diff --git a/arch/xtensa/include/asm/ftrace.h b/arch/xtensa/include/asm/ftrace.h
> index 736b9d2..6c6d9a9 100644
> --- a/arch/xtensa/include/asm/ftrace.h
> +++ b/arch/xtensa/include/asm/ftrace.h
> @@ -12,24 +12,18 @@
>  
>  #include <asm/processor.h>
>  
> -#define HAVE_ARCH_CALLER_ADDR
>  #ifndef __ASSEMBLY__
> -#define CALLER_ADDR0 ({ unsigned long a0, a1; \
> +#define ftrace_return_address0 ({ unsigned long a0, a1; \
>  		__asm__ __volatile__ ( \
>  			"mov %0, a0\n" \
>  			"mov %1, a1\n" \
>  			: "=r"(a0), "=r"(a1)); \
>  		MAKE_PC_FROM_RA(a0, a1); })
> +
>  #ifdef CONFIG_FRAME_POINTER
>  extern unsigned long return_address(unsigned level);
> -#define CALLER_ADDR1 return_address(1)
> -#define CALLER_ADDR2 return_address(2)
> -#define CALLER_ADDR3 return_address(3)
> -#else /* CONFIG_FRAME_POINTER */
> -#define CALLER_ADDR1 (0)
> -#define CALLER_ADDR2 (0)
> -#define CALLER_ADDR3 (0)
> -#endif /* CONFIG_FRAME_POINTER */
> +#define ftrace_return_address(n) return_address(n)

I would add a comment here that states:

/*
 * #else !CONFIG_FRAME_POINTER
 *
 * Define CALLER_ADDRn (n > 0) to 0 is the default in linux/ftrace.h if
 * ftrace_return_address(n) and CONFIG_FRAME_POINTER are not defined.
 */

Otherwise it looks like you are missing the #else statement.

> +#endif
>  #endif /* __ASSEMBLY__ */
>  
>  #ifdef CONFIG_FUNCTION_TRACER
> diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> index 9212b01..18f1ae7 100644
> --- a/include/linux/ftrace.h
> +++ b/include/linux/ftrace.h
> @@ -614,25 +614,27 @@ static inline void __ftrace_enabled_restore(int enabled)
>  #endif
>  }
>  
> -#ifndef HAVE_ARCH_CALLER_ADDR
> +/* All archs should have this, but we define it for consistency */

The comment is a little confusing. I think it may be better stated as:

/*
 * All archs should be able to use __builtin_return_address(0) but
 * we allow them to redefine it for consistency.
 */

> +#ifndef ftrace_return_address0
> +# define ftrace_return_address0 __builtin_return_address(0)
> +#endif
> +
> +/* Archs may use other ways for ADDR1 and beyond */

How about:

/* Not all archs can use __builtin_return_address(n) where n > 0 */

> +#ifndef ftrace_return_address
>  # ifdef CONFIG_FRAME_POINTER
> -#  define CALLER_ADDR0 ((unsigned long)__builtin_return_address(0))
> -#  define CALLER_ADDR1 ((unsigned long)__builtin_return_address(1))
> -#  define CALLER_ADDR2 ((unsigned long)__builtin_return_address(2))
> -#  define CALLER_ADDR3 ((unsigned long)__builtin_return_address(3))
> -#  define CALLER_ADDR4 ((unsigned long)__builtin_return_address(4))
> -#  define CALLER_ADDR5 ((unsigned long)__builtin_return_address(5))
> -#  define CALLER_ADDR6 ((unsigned long)__builtin_return_address(6))
> +#  define ftrace_return_address(n) __builtin_return_address(n)
>  # else
> -#  define CALLER_ADDR0 ((unsigned long)__builtin_return_address(0))
> -#  define CALLER_ADDR1 0UL
> -#  define CALLER_ADDR2 0UL
> -#  define CALLER_ADDR3 0UL
> -#  define CALLER_ADDR4 0UL
> -#  define CALLER_ADDR5 0UL
> -#  define CALLER_ADDR6 0UL
> +#  define ftrace_return_address(n) 0UL
>  # endif
> -#endif /* ifndef HAVE_ARCH_CALLER_ADDR */
> +#endif
> +
> +#define CALLER_ADDR0 ((unsigned long)ftrace_return_address0)
> +#define CALLER_ADDR1 ((unsigned long)ftrace_return_address(1))
> +#define CALLER_ADDR2 ((unsigned long)ftrace_return_address(2))
> +#define CALLER_ADDR3 ((unsigned long)ftrace_return_address(3))
> +#define CALLER_ADDR4 ((unsigned long)ftrace_return_address(4))
> +#define CALLER_ADDR5 ((unsigned long)ftrace_return_address(5))
> +#define CALLER_ADDR6 ((unsigned long)ftrace_return_address(6))

Other than that, it looks good. You can send me the patch and I'll add
it to my 3.16 queue.

Feel free to reply to this email with a v9. When I pull patches into
git, it adds the link to the thread for the patch in LKML. To keep this
entire thread, just reply to it.

Thanks!

-- Steve

>  
>  #ifdef CONFIG_IRQSOFF_TRACER
>    extern void time_hardirqs_on(unsigned long a0, unsigned long a1);

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Will Deacon May 12, 2014, 3:58 p.m. UTC | #5
Hi guys,

On Fri, May 02, 2014 at 08:19:57PM +0100, Steven Rostedt wrote:
> On Wed, 30 Apr 2014 18:54:29 +0900
> AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
> 
> > Most archs with HAVE_ARCH_CALLER_ADDR have the almost same definitions
> > of CALLER_ADDRx(n), and so put them into linux/ftrace.h.
> 
> Please add a bit more to the change log. Like what you did. Something
> like:
> 
> ----
> Most archs with HAVE_ARCH_CALLER_ADDR have pretty much the same
> definitions of CALLER_ADDRx(n). Instead of duplicating the code for all
> the archs, define a ftrace_return_address0() and
> ftrace_return_address(n) that can be overwritten by the archs if they
> need to do something different. Instead of 7 macros in every arch, we
> now only have at most 2 (and actually only 1 as
> ftrace_return_address0() should be the same for all archs).
> 
> The CALLER_ADDRx(n) will now be defined in linux/ftrace.h and use the
> ftrace_return_address*(n?) macros. This removes a lot of the duplicate
> code.
> -----
> 
> Use that if you want :-)

Akashi: did you get around to posting a new version of this patch? We can't
take your arm64 patches until you get the core stuff merged...

Will
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Steven Rostedt May 12, 2014, 4:05 p.m. UTC | #6
On Mon, 12 May 2014 16:58:11 +0100
Will Deacon <will.deacon@arm.com> wrote:

> Akashi: did you get around to posting a new version of this patch? We can't
> take your arm64 patches until you get the core stuff merged...

I haven't seen any patches yet.

What I can also do is to create a separate branch based on mainline,
and just apply this change to the core. Then you could pull that branch
with a note to Linus that it is also in my tree with the common
ancestor.

If he pulls your work first, he'll only get that change from my tree,
and if he pulls mine first it doesn't matter as all the changes will be
there.

This is something he said at Kernel Summit that was normal procedure if
there's a change in one tree that another tree is dependent on.

-- Steve

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Will Deacon May 12, 2014, 4:12 p.m. UTC | #7
Hey Steve,

On Mon, May 12, 2014 at 05:05:35PM +0100, Steven Rostedt wrote:
> On Mon, 12 May 2014 16:58:11 +0100
> Will Deacon <will.deacon@arm.com> wrote:
> 
> > Akashi: did you get around to posting a new version of this patch? We can't
> > take your arm64 patches until you get the core stuff merged...
> 
> I haven't seen any patches yet.
> 
> What I can also do is to create a separate branch based on mainline,
> and just apply this change to the core. Then you could pull that branch
> with a note to Linus that it is also in my tree with the common
> ancestor.
> 
> If he pulls your work first, he'll only get that change from my tree,
> and if he pulls mine first it doesn't matter as all the changes will be
> there.
> 
> This is something he said at Kernel Summit that was normal procedure if
> there's a change in one tree that another tree is dependent on.

That sounds great, and nicely solves the dependency between our trees for
3.16 at the same time.

If you point us at the branch and promise not to rebase it, that would be
fantastic.

Cheers,

Will
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Steven Rostedt May 12, 2014, 5:23 p.m. UTC | #8
On Mon, 12 May 2014 17:12:46 +0100
Will Deacon <will.deacon@arm.com> wrote:


> If you point us at the branch and promise not to rebase it, that would be
> fantastic.

I will when I get the patch(es) :-) and run it through all my tests.

-- Steve

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
AKASHI Takahiro May 20, 2014, 11:29 a.m. UTC | #9
Hi Will, Steven,

So sorry, I completely missed this message thread. I will submit a new patch (replacement of [1/8])
in the following e-mail.

-Takahiro AKASHI

On 05/03/2014 04:19 AM, Steven Rostedt wrote:
> On Wed, 30 Apr 2014 18:54:29 +0900
> AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
>
>> Most archs with HAVE_ARCH_CALLER_ADDR have the almost same definitions
>> of CALLER_ADDRx(n), and so put them into linux/ftrace.h.
>
> Please add a bit more to the change log. Like what you did. Something
> like:
>
> ----
> Most archs with HAVE_ARCH_CALLER_ADDR have pretty much the same
> definitions of CALLER_ADDRx(n). Instead of duplicating the code for all
> the archs, define a ftrace_return_address0() and
> ftrace_return_address(n) that can be overwritten by the archs if they
> need to do something different. Instead of 7 macros in every arch, we
> now only have at most 2 (and actually only 1 as
> ftrace_return_address0() should be the same for all archs).
>
> The CALLER_ADDRx(n) will now be defined in linux/ftrace.h and use the
> ftrace_return_address*(n?) macros. This removes a lot of the duplicate
> code.
> -----
>
> Use that if you want :-)
>
>
>>
>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
>> ---
>>   arch/arm/include/asm/ftrace.h      |   10 +---------
>>   arch/blackfin/include/asm/ftrace.h |   11 +----------
>>   arch/parisc/include/asm/ftrace.h   |   10 +---------
>>   arch/sh/include/asm/ftrace.h       |   10 +---------
>>   arch/xtensa/include/asm/ftrace.h   |   14 ++++----------
>>   include/linux/ftrace.h             |   34 ++++++++++++++++++----------------
>>   6 files changed, 26 insertions(+), 63 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/ftrace.h b/arch/arm/include/asm/ftrace.h
>> index f89515a..eb577f4 100644
>> --- a/arch/arm/include/asm/ftrace.h
>> +++ b/arch/arm/include/asm/ftrace.h
>> @@ -52,15 +52,7 @@ extern inline void *return_address(unsigned int level)
>>
>>   #endif
>>
>> -#define HAVE_ARCH_CALLER_ADDR
>> -
>> -#define CALLER_ADDR0 ((unsigned long)__builtin_return_address(0))
>> -#define CALLER_ADDR1 ((unsigned long)return_address(1))
>> -#define CALLER_ADDR2 ((unsigned long)return_address(2))
>> -#define CALLER_ADDR3 ((unsigned long)return_address(3))
>> -#define CALLER_ADDR4 ((unsigned long)return_address(4))
>> -#define CALLER_ADDR5 ((unsigned long)return_address(5))
>> -#define CALLER_ADDR6 ((unsigned long)return_address(6))
>> +#define ftrace_return_addr(n) return_address(n)
>>
>>   #endif /* ifndef __ASSEMBLY__ */
>>
>> diff --git a/arch/blackfin/include/asm/ftrace.h b/arch/blackfin/include/asm/ftrace.h
>> index 8a02950..2f1c3c2 100644
>> --- a/arch/blackfin/include/asm/ftrace.h
>> +++ b/arch/blackfin/include/asm/ftrace.h
>> @@ -66,16 +66,7 @@ extern inline void *return_address(unsigned int level)
>>
>>   #endif /* CONFIG_FRAME_POINTER */
>>
>> -#define HAVE_ARCH_CALLER_ADDR
>> -
>> -/* inline function or macro may lead to unexpected result */
>> -#define CALLER_ADDR0 ((unsigned long)__builtin_return_address(0))
>> -#define CALLER_ADDR1 ((unsigned long)return_address(1))
>> -#define CALLER_ADDR2 ((unsigned long)return_address(2))
>> -#define CALLER_ADDR3 ((unsigned long)return_address(3))
>> -#define CALLER_ADDR4 ((unsigned long)return_address(4))
>> -#define CALLER_ADDR5 ((unsigned long)return_address(5))
>> -#define CALLER_ADDR6 ((unsigned long)return_address(6))
>> +#define ftrace_return_address(n) return_address(n)
>>
>>   #endif /* __ASSEMBLY__ */
>>
>> diff --git a/arch/parisc/include/asm/ftrace.h b/arch/parisc/include/asm/ftrace.h
>> index 72c0faf..544ed8e 100644
>> --- a/arch/parisc/include/asm/ftrace.h
>> +++ b/arch/parisc/include/asm/ftrace.h
>> @@ -24,15 +24,7 @@ extern void return_to_handler(void);
>>
>>   extern unsigned long return_address(unsigned int);
>>
>> -#define HAVE_ARCH_CALLER_ADDR
>> -
>> -#define CALLER_ADDR0 ((unsigned long)__builtin_return_address(0))
>> -#define CALLER_ADDR1 return_address(1)
>> -#define CALLER_ADDR2 return_address(2)
>> -#define CALLER_ADDR3 return_address(3)
>> -#define CALLER_ADDR4 return_address(4)
>> -#define CALLER_ADDR5 return_address(5)
>> -#define CALLER_ADDR6 return_address(6)
>> +#define ftrace_return_address(n) return_address(n)
>>
>>   #endif /* __ASSEMBLY__ */
>>
>> diff --git a/arch/sh/include/asm/ftrace.h b/arch/sh/include/asm/ftrace.h
>> index 13e9966..e79fb6e 100644
>> --- a/arch/sh/include/asm/ftrace.h
>> +++ b/arch/sh/include/asm/ftrace.h
>> @@ -40,15 +40,7 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr)
>>   /* arch/sh/kernel/return_address.c */
>>   extern void *return_address(unsigned int);
>>
>> -#define HAVE_ARCH_CALLER_ADDR
>> -
>> -#define CALLER_ADDR0 ((unsigned long)__builtin_return_address(0))
>> -#define CALLER_ADDR1 ((unsigned long)return_address(1))
>> -#define CALLER_ADDR2 ((unsigned long)return_address(2))
>> -#define CALLER_ADDR3 ((unsigned long)return_address(3))
>> -#define CALLER_ADDR4 ((unsigned long)return_address(4))
>> -#define CALLER_ADDR5 ((unsigned long)return_address(5))
>> -#define CALLER_ADDR6 ((unsigned long)return_address(6))
>> +#define ftrace_return_address(n) return_address(n)
>>
>>   #endif /* __ASSEMBLY__ */
>>
>> diff --git a/arch/xtensa/include/asm/ftrace.h b/arch/xtensa/include/asm/ftrace.h
>> index 736b9d2..6c6d9a9 100644
>> --- a/arch/xtensa/include/asm/ftrace.h
>> +++ b/arch/xtensa/include/asm/ftrace.h
>> @@ -12,24 +12,18 @@
>>
>>   #include <asm/processor.h>
>>
>> -#define HAVE_ARCH_CALLER_ADDR
>>   #ifndef __ASSEMBLY__
>> -#define CALLER_ADDR0 ({ unsigned long a0, a1; \
>> +#define ftrace_return_address0 ({ unsigned long a0, a1; \
>>   		__asm__ __volatile__ ( \
>>   			"mov %0, a0\n" \
>>   			"mov %1, a1\n" \
>>   			: "=r"(a0), "=r"(a1)); \
>>   		MAKE_PC_FROM_RA(a0, a1); })
>> +
>>   #ifdef CONFIG_FRAME_POINTER
>>   extern unsigned long return_address(unsigned level);
>> -#define CALLER_ADDR1 return_address(1)
>> -#define CALLER_ADDR2 return_address(2)
>> -#define CALLER_ADDR3 return_address(3)
>> -#else /* CONFIG_FRAME_POINTER */
>> -#define CALLER_ADDR1 (0)
>> -#define CALLER_ADDR2 (0)
>> -#define CALLER_ADDR3 (0)
>> -#endif /* CONFIG_FRAME_POINTER */
>> +#define ftrace_return_address(n) return_address(n)
>
> I would add a comment here that states:
>
> /*
>   * #else !CONFIG_FRAME_POINTER
>   *
>   * Define CALLER_ADDRn (n > 0) to 0 is the default in linux/ftrace.h if
>   * ftrace_return_address(n) and CONFIG_FRAME_POINTER are not defined.
>   */
>
> Otherwise it looks like you are missing the #else statement.
>
>> +#endif
>>   #endif /* __ASSEMBLY__ */
>>
>>   #ifdef CONFIG_FUNCTION_TRACER
>> diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
>> index 9212b01..18f1ae7 100644
>> --- a/include/linux/ftrace.h
>> +++ b/include/linux/ftrace.h
>> @@ -614,25 +614,27 @@ static inline void __ftrace_enabled_restore(int enabled)
>>   #endif
>>   }
>>
>> -#ifndef HAVE_ARCH_CALLER_ADDR
>> +/* All archs should have this, but we define it for consistency */
>
> The comment is a little confusing. I think it may be better stated as:
>
> /*
>   * All archs should be able to use __builtin_return_address(0) but
>   * we allow them to redefine it for consistency.
>   */
>
>> +#ifndef ftrace_return_address0
>> +# define ftrace_return_address0 __builtin_return_address(0)
>> +#endif
>> +
>> +/* Archs may use other ways for ADDR1 and beyond */
>
> How about:
>
> /* Not all archs can use __builtin_return_address(n) where n > 0 */
>
>> +#ifndef ftrace_return_address
>>   # ifdef CONFIG_FRAME_POINTER
>> -#  define CALLER_ADDR0 ((unsigned long)__builtin_return_address(0))
>> -#  define CALLER_ADDR1 ((unsigned long)__builtin_return_address(1))
>> -#  define CALLER_ADDR2 ((unsigned long)__builtin_return_address(2))
>> -#  define CALLER_ADDR3 ((unsigned long)__builtin_return_address(3))
>> -#  define CALLER_ADDR4 ((unsigned long)__builtin_return_address(4))
>> -#  define CALLER_ADDR5 ((unsigned long)__builtin_return_address(5))
>> -#  define CALLER_ADDR6 ((unsigned long)__builtin_return_address(6))
>> +#  define ftrace_return_address(n) __builtin_return_address(n)
>>   # else
>> -#  define CALLER_ADDR0 ((unsigned long)__builtin_return_address(0))
>> -#  define CALLER_ADDR1 0UL
>> -#  define CALLER_ADDR2 0UL
>> -#  define CALLER_ADDR3 0UL
>> -#  define CALLER_ADDR4 0UL
>> -#  define CALLER_ADDR5 0UL
>> -#  define CALLER_ADDR6 0UL
>> +#  define ftrace_return_address(n) 0UL
>>   # endif
>> -#endif /* ifndef HAVE_ARCH_CALLER_ADDR */
>> +#endif
>> +
>> +#define CALLER_ADDR0 ((unsigned long)ftrace_return_address0)
>> +#define CALLER_ADDR1 ((unsigned long)ftrace_return_address(1))
>> +#define CALLER_ADDR2 ((unsigned long)ftrace_return_address(2))
>> +#define CALLER_ADDR3 ((unsigned long)ftrace_return_address(3))
>> +#define CALLER_ADDR4 ((unsigned long)ftrace_return_address(4))
>> +#define CALLER_ADDR5 ((unsigned long)ftrace_return_address(5))
>> +#define CALLER_ADDR6 ((unsigned long)ftrace_return_address(6))
>
> Other than that, it looks good. You can send me the patch and I'll add
> it to my 3.16 queue.
>
> Feel free to reply to this email with a v9. When I pull patches into
> git, it adds the link to the thread for the patch in LKML. To keep this
> entire thread, just reply to it.
>
> Thanks!
>
> -- Steve
>
>>
>>   #ifdef CONFIG_IRQSOFF_TRACER
>>     extern void time_hardirqs_on(unsigned long a0, unsigned long a1);
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Geert Uytterhoeven June 11, 2014, 12:34 p.m. UTC | #10
On Wed, Apr 30, 2014 at 11:54 AM, AKASHI Takahiro
<takahiro.akashi@linaro.org> wrote:
> Most archs with HAVE_ARCH_CALLER_ADDR have the almost same definitions
> of CALLER_ADDRx(n), and so put them into linux/ftrace.h.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>

On arm (at least shmobile_defconfig and versatile_defconfig) with gcc 4.6.3:

kernel/sched/core.c: In function 'get_parent_ip':
kernel/sched/core.c:2520:10: warning: unsupported argument to
'__builtin_return_address' [enabled by default]
kernel/sched/core.c:2522:11: warning: unsupported argument to
'__builtin_return_address' [enabled by default]

With gcc 4.9.0:

kernel/sched/core.c: In function 'get_parent_ip':
include/linux/ftrace.h:627:36: warning: unsupported argument to
'__builtin_return_address'
 #  define ftrace_return_address(n) __builtin_return_address(n)
                                    ^
include/linux/ftrace.h:635:38: note: in expansion of macro
'ftrace_return_address'
 #define CALLER_ADDR2 ((unsigned long)ftrace_return_address(2))
                                      ^
kernel/sched/core.c:2520:10: note: in expansion of macro 'CALLER_ADDR2'
   addr = CALLER_ADDR2;
          ^
include/linux/ftrace.h:627:36: warning: unsupported argument to
'__builtin_return_address'
 #  define ftrace_return_address(n) __builtin_return_address(n)
                                    ^
include/linux/ftrace.h:636:38: note: in expansion of macro
'ftrace_return_address'
 #define CALLER_ADDR3 ((unsigned long)ftrace_return_address(3))
                                      ^
kernel/sched/core.c:2522:11: note: in expansion of macro 'CALLER_ADDR3'
    addr = CALLER_ADDR3;
           ^

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Geert Uytterhoeven June 11, 2014, 1:38 p.m. UTC | #11
Hi Steven,

On Wed, Jun 11, 2014 at 3:23 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
> The clean up of CALLER_ADDR*() functions required the archs to either
> use the default __builtin_return_address(X) (where X > 0) or override
> it with something the arch can use. To override it, the arch would
> define function_return_address(x).

ftrace_return_address(x)

> The arm architecture requires this to be redefined but instead of
> defining function_return_address(x) it defined function_return_addr(x).

ftrace_return_address(x) ... ftrace_return_address(x)

> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>

Nevertheless, your patch kills the warnings. Thanks!

Tested-by: Geert Uytterhoeven <geert@linux-m68k.org>

> ----
> diff --git a/arch/arm/include/asm/ftrace.h b/arch/arm/include/asm/ftrace.h
> index eb577f4..39eb16b 100644
> --- a/arch/arm/include/asm/ftrace.h
> +++ b/arch/arm/include/asm/ftrace.h
> @@ -52,7 +52,7 @@ extern inline void *return_address(unsigned int level)
>
>  #endif
>
> -#define ftrace_return_addr(n) return_address(n)
> +#define ftrace_return_address(n) return_address(n)
>
>  #endif /* ifndef __ASSEMBLY__ */

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Steven Rostedt June 11, 2014, 1:44 p.m. UTC | #12
On Wed, 11 Jun 2014 15:38:49 +0200
Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> Hi Steven,
> 
> On Wed, Jun 11, 2014 at 3:23 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
> > The clean up of CALLER_ADDR*() functions required the archs to either
> > use the default __builtin_return_address(X) (where X > 0) or override
> > it with something the arch can use. To override it, the arch would
> > define function_return_address(x).
> 
> ftrace_return_address(x)
> 
> > The arm architecture requires this to be redefined but instead of
> > defining function_return_address(x) it defined function_return_addr(x).
> 
> ftrace_return_address(x) ... ftrace_return_address(x)

As long as I got the patch part right ;-)

Will, please update the change log with the correct name. Including the
subject. Thanks!

-- Steve

> 
> > Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> > Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> 
> Nevertheless, your patch kills the warnings. Thanks!
> 
> Tested-by: Geert Uytterhoeven <geert@linux-m68k.org>
> 
> > ----
> > diff --git a/arch/arm/include/asm/ftrace.h b/arch/arm/include/asm/ftrace.h
> > index eb577f4..39eb16b 100644
> > --- a/arch/arm/include/asm/ftrace.h
> > +++ b/arch/arm/include/asm/ftrace.h
> > @@ -52,7 +52,7 @@ extern inline void *return_address(unsigned int level)
> >
> >  #endif
> >
> > -#define ftrace_return_addr(n) return_address(n)
> > +#define ftrace_return_address(n) return_address(n)
> >
> >  #endif /* ifndef __ASSEMBLY__ */
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
diff mbox

Patch

diff --git a/arch/arm/include/asm/ftrace.h b/arch/arm/include/asm/ftrace.h
index f89515a..eb577f4 100644
--- a/arch/arm/include/asm/ftrace.h
+++ b/arch/arm/include/asm/ftrace.h
@@ -52,15 +52,7 @@  extern inline void *return_address(unsigned int level)
 
 #endif
 
-#define HAVE_ARCH_CALLER_ADDR
-
-#define CALLER_ADDR0 ((unsigned long)__builtin_return_address(0))
-#define CALLER_ADDR1 ((unsigned long)return_address(1))
-#define CALLER_ADDR2 ((unsigned long)return_address(2))
-#define CALLER_ADDR3 ((unsigned long)return_address(3))
-#define CALLER_ADDR4 ((unsigned long)return_address(4))
-#define CALLER_ADDR5 ((unsigned long)return_address(5))
-#define CALLER_ADDR6 ((unsigned long)return_address(6))
+#define ftrace_return_addr(n) return_address(n)
 
 #endif /* ifndef __ASSEMBLY__ */
 
diff --git a/arch/blackfin/include/asm/ftrace.h b/arch/blackfin/include/asm/ftrace.h
index 8a02950..2f1c3c2 100644
--- a/arch/blackfin/include/asm/ftrace.h
+++ b/arch/blackfin/include/asm/ftrace.h
@@ -66,16 +66,7 @@  extern inline void *return_address(unsigned int level)
 
 #endif /* CONFIG_FRAME_POINTER */
 
-#define HAVE_ARCH_CALLER_ADDR
-
-/* inline function or macro may lead to unexpected result */
-#define CALLER_ADDR0 ((unsigned long)__builtin_return_address(0))
-#define CALLER_ADDR1 ((unsigned long)return_address(1))
-#define CALLER_ADDR2 ((unsigned long)return_address(2))
-#define CALLER_ADDR3 ((unsigned long)return_address(3))
-#define CALLER_ADDR4 ((unsigned long)return_address(4))
-#define CALLER_ADDR5 ((unsigned long)return_address(5))
-#define CALLER_ADDR6 ((unsigned long)return_address(6))
+#define ftrace_return_address(n) return_address(n)
 
 #endif /* __ASSEMBLY__ */
 
diff --git a/arch/parisc/include/asm/ftrace.h b/arch/parisc/include/asm/ftrace.h
index 72c0faf..544ed8e 100644
--- a/arch/parisc/include/asm/ftrace.h
+++ b/arch/parisc/include/asm/ftrace.h
@@ -24,15 +24,7 @@  extern void return_to_handler(void);
 
 extern unsigned long return_address(unsigned int);
 
-#define HAVE_ARCH_CALLER_ADDR
-
-#define CALLER_ADDR0 ((unsigned long)__builtin_return_address(0))
-#define CALLER_ADDR1 return_address(1)
-#define CALLER_ADDR2 return_address(2)
-#define CALLER_ADDR3 return_address(3)
-#define CALLER_ADDR4 return_address(4)
-#define CALLER_ADDR5 return_address(5)
-#define CALLER_ADDR6 return_address(6)
+#define ftrace_return_address(n) return_address(n)
 
 #endif /* __ASSEMBLY__ */
 
diff --git a/arch/sh/include/asm/ftrace.h b/arch/sh/include/asm/ftrace.h
index 13e9966..e79fb6e 100644
--- a/arch/sh/include/asm/ftrace.h
+++ b/arch/sh/include/asm/ftrace.h
@@ -40,15 +40,7 @@  static inline unsigned long ftrace_call_adjust(unsigned long addr)
 /* arch/sh/kernel/return_address.c */
 extern void *return_address(unsigned int);
 
-#define HAVE_ARCH_CALLER_ADDR
-
-#define CALLER_ADDR0 ((unsigned long)__builtin_return_address(0))
-#define CALLER_ADDR1 ((unsigned long)return_address(1))
-#define CALLER_ADDR2 ((unsigned long)return_address(2))
-#define CALLER_ADDR3 ((unsigned long)return_address(3))
-#define CALLER_ADDR4 ((unsigned long)return_address(4))
-#define CALLER_ADDR5 ((unsigned long)return_address(5))
-#define CALLER_ADDR6 ((unsigned long)return_address(6))
+#define ftrace_return_address(n) return_address(n)
 
 #endif /* __ASSEMBLY__ */
 
diff --git a/arch/xtensa/include/asm/ftrace.h b/arch/xtensa/include/asm/ftrace.h
index 736b9d2..6c6d9a9 100644
--- a/arch/xtensa/include/asm/ftrace.h
+++ b/arch/xtensa/include/asm/ftrace.h
@@ -12,24 +12,18 @@ 
 
 #include <asm/processor.h>
 
-#define HAVE_ARCH_CALLER_ADDR
 #ifndef __ASSEMBLY__
-#define CALLER_ADDR0 ({ unsigned long a0, a1; \
+#define ftrace_return_address0 ({ unsigned long a0, a1; \
 		__asm__ __volatile__ ( \
 			"mov %0, a0\n" \
 			"mov %1, a1\n" \
 			: "=r"(a0), "=r"(a1)); \
 		MAKE_PC_FROM_RA(a0, a1); })
+
 #ifdef CONFIG_FRAME_POINTER
 extern unsigned long return_address(unsigned level);
-#define CALLER_ADDR1 return_address(1)
-#define CALLER_ADDR2 return_address(2)
-#define CALLER_ADDR3 return_address(3)
-#else /* CONFIG_FRAME_POINTER */
-#define CALLER_ADDR1 (0)
-#define CALLER_ADDR2 (0)
-#define CALLER_ADDR3 (0)
-#endif /* CONFIG_FRAME_POINTER */
+#define ftrace_return_address(n) return_address(n)
+#endif
 #endif /* __ASSEMBLY__ */
 
 #ifdef CONFIG_FUNCTION_TRACER
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 9212b01..18f1ae7 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -614,25 +614,27 @@  static inline void __ftrace_enabled_restore(int enabled)
 #endif
 }
 
-#ifndef HAVE_ARCH_CALLER_ADDR
+/* All archs should have this, but we define it for consistency */
+#ifndef ftrace_return_address0
+# define ftrace_return_address0 __builtin_return_address(0)
+#endif
+
+/* Archs may use other ways for ADDR1 and beyond */
+#ifndef ftrace_return_address
 # ifdef CONFIG_FRAME_POINTER
-#  define CALLER_ADDR0 ((unsigned long)__builtin_return_address(0))
-#  define CALLER_ADDR1 ((unsigned long)__builtin_return_address(1))
-#  define CALLER_ADDR2 ((unsigned long)__builtin_return_address(2))
-#  define CALLER_ADDR3 ((unsigned long)__builtin_return_address(3))
-#  define CALLER_ADDR4 ((unsigned long)__builtin_return_address(4))
-#  define CALLER_ADDR5 ((unsigned long)__builtin_return_address(5))
-#  define CALLER_ADDR6 ((unsigned long)__builtin_return_address(6))
+#  define ftrace_return_address(n) __builtin_return_address(n)
 # else
-#  define CALLER_ADDR0 ((unsigned long)__builtin_return_address(0))
-#  define CALLER_ADDR1 0UL
-#  define CALLER_ADDR2 0UL
-#  define CALLER_ADDR3 0UL
-#  define CALLER_ADDR4 0UL
-#  define CALLER_ADDR5 0UL
-#  define CALLER_ADDR6 0UL
+#  define ftrace_return_address(n) 0UL
 # endif
-#endif /* ifndef HAVE_ARCH_CALLER_ADDR */
+#endif
+
+#define CALLER_ADDR0 ((unsigned long)ftrace_return_address0)
+#define CALLER_ADDR1 ((unsigned long)ftrace_return_address(1))
+#define CALLER_ADDR2 ((unsigned long)ftrace_return_address(2))
+#define CALLER_ADDR3 ((unsigned long)ftrace_return_address(3))
+#define CALLER_ADDR4 ((unsigned long)ftrace_return_address(4))
+#define CALLER_ADDR5 ((unsigned long)ftrace_return_address(5))
+#define CALLER_ADDR6 ((unsigned long)ftrace_return_address(6))
 
 #ifdef CONFIG_IRQSOFF_TRACER
   extern void time_hardirqs_on(unsigned long a0, unsigned long a1);