[PATCH 2/2]: sparc64: Add ftrace support. - Kernel

This is a discussion on [PATCH 2/2]: sparc64: Add ftrace support. - Kernel ; Signed-off-by: David S. Miller --- arch/sparc64/Kconfig | 1 + arch/sparc64/Kconfig.debug | 2 +- arch/sparc64/kernel/Makefile | 1 + arch/sparc64/kernel/ftrace.c | 99 ++++++++++++++++++++++++++++++++++++++++++ arch/sparc64/lib/mcount.S | 58 +++++++++++++++++++++++-- 5 files changed, 156 insertions(+), 5 deletions(-) create mode 100644 arch/sparc64/kernel/ftrace.c diff --git a/arch/sparc64/Kconfig b/arch/sparc64/Kconfig ...

+ Reply to Thread
Results 1 to 5 of 5

Thread: [PATCH 2/2]: sparc64: Add ftrace support.

  1. [PATCH 2/2]: sparc64: Add ftrace support.


    Signed-off-by: David S. Miller
    ---
    arch/sparc64/Kconfig | 1 +
    arch/sparc64/Kconfig.debug | 2 +-
    arch/sparc64/kernel/Makefile | 1 +
    arch/sparc64/kernel/ftrace.c | 99 ++++++++++++++++++++++++++++++++++++++++++
    arch/sparc64/lib/mcount.S | 58 +++++++++++++++++++++++--
    5 files changed, 156 insertions(+), 5 deletions(-)
    create mode 100644 arch/sparc64/kernel/ftrace.c

    diff --git a/arch/sparc64/Kconfig b/arch/sparc64/Kconfig
    index 4c40862..56344b1 100644
    --- a/arch/sparc64/Kconfig
    +++ b/arch/sparc64/Kconfig
    @@ -15,6 +15,7 @@ config SPARC64
    select HAVE_LMB
    select HAVE_ARCH_KGDB
    select HAVE_IMMEDIATE
    + select HAVE_FTRACE

    config GENERIC_TIME
    bool
    diff --git a/arch/sparc64/Kconfig.debug b/arch/sparc64/Kconfig.debug
    index 6a4d28a..d6d32d1 100644
    --- a/arch/sparc64/Kconfig.debug
    +++ b/arch/sparc64/Kconfig.debug
    @@ -33,7 +33,7 @@ config DEBUG_PAGEALLOC

    config MCOUNT
    bool
    - depends on STACK_DEBUG
    + depends on STACK_DEBUG || FTRACE
    default y

    config FRAME_POINTER
    diff --git a/arch/sparc64/kernel/Makefile b/arch/sparc64/kernel/Makefile
    index 311c797..52e933f 100644
    --- a/arch/sparc64/kernel/Makefile
    +++ b/arch/sparc64/kernel/Makefile
    @@ -14,6 +14,7 @@ obj-y := process.o setup.o cpu.o idprom.o \
    power.o sbus.o sparc64_ksyms.o chmc.o \
    visemul.o prom.o of_device.o hvapi.o sstate.o mdesc.o

    +obj-$(CONFIG_DYNAMIC_FTRACE) += ftrace.o
    obj-$(CONFIG_STACKTRACE) += stacktrace.o
    obj-$(CONFIG_PCI) += ebus.o pci_common.o \
    pci_psycho.o pci_sabre.o pci_schizo.o \
    diff --git a/arch/sparc64/kernel/ftrace.c b/arch/sparc64/kernel/ftrace.c
    new file mode 100644
    index 0000000..f449e6d
    --- /dev/null
    +++ b/arch/sparc64/kernel/ftrace.c
    @@ -0,0 +1,99 @@
    +#include
    +#include
    +#include
    +#include
    +#include
    +#include
    +
    +static const u32 ftrace_nop = 0x01000000;
    +
    +notrace int ftrace_ip_converted(unsigned long ip)
    +{
    + u32 insn = *(u32 *) ip;
    +
    + return (insn == ftrace_nop);
    +}
    +
    +notrace unsigned char *ftrace_nop_replace(void)
    +{
    + return (char *)&ftrace_nop;
    +}
    +
    +notrace unsigned char *ftrace_call_replace(unsigned long ip, unsigned long addr)
    +{
    + static u32 call;
    + s32 off;
    +
    + off = ((s32)addr - (s32)ip);
    + call = 0x40000000 | ((u32)off >> 2);
    +
    + return (unsigned char *) &call;
    +}
    +
    +notrace int
    +ftrace_modify_code(unsigned long ip, unsigned char *old_code,
    + unsigned char *new_code)
    +{
    + u32 old = *(u32 *)old_code;
    + u32 new = *(u32 *)new_code;
    + u32 replaced;
    + int faulted;
    +
    + __asm__ __volatile__(
    + "1: cas [%[ip]], %[old], %[new]\n"
    + " flush %[ip]\n"
    + " mov 0, %[faulted]\n"
    + "2:\n"
    + " .section .fixup,#alloc,#execinstr\n"
    + " .align 4\n"
    + "3: sethi %%hi(2b), %[faulted]\n"
    + " jmpl %[faulted] + %%lo(2b), %%g0\n"
    + " mov 1, %[faulted]\n"
    + " .previous\n"
    + " .section __ex_table,\"a\"\n"
    + " .align 4\n"
    + " .word 1b, 3b\n"
    + " .previous\n"
    + : "=r" (replaced), [faulted] "=r" (faulted)
    + : [new] "0" (new), [old] "r" (old), [ip] "r" (ip)
    + : "memory");
    +
    + if (replaced != old && replaced != new)
    + faulted = 2;
    +
    + return faulted;
    +}
    +
    +notrace int ftrace_update_ftrace_func(ftrace_func_t func)
    +{
    + unsigned long ip = (unsigned long)(&ftrace_call);
    + unsigned char old[4], *new;
    +
    + memcpy(old, &ftrace_call, 4);
    + new = ftrace_call_replace(ip, (unsigned long)func);
    + return ftrace_modify_code(ip, old, new);
    +}
    +
    +notrace int ftrace_mcount_set(unsigned long *data)
    +{
    + unsigned long ip = (long)(&mcount_call);
    + unsigned long *addr = data;
    + unsigned char old[4], *new;
    +
    + /*
    + * Replace the mcount stub with a pointer to the
    + * ip recorder function.
    + */
    + memcpy(old, &mcount_call, 4);
    + new = ftrace_call_replace(ip, *addr);
    + *addr = ftrace_modify_code(ip, old, new);
    +
    + return 0;
    +}
    +
    +
    +int __init ftrace_dyn_arch_init(void *data)
    +{
    + ftrace_mcount_set(data);
    + return 0;
    +}
    diff --git a/arch/sparc64/lib/mcount.S b/arch/sparc64/lib/mcount.S
    index 9e4534b..7735a7a 100644
    --- a/arch/sparc64/lib/mcount.S
    +++ b/arch/sparc64/lib/mcount.S
    @@ -28,10 +28,13 @@ ovstack:
    .skip OVSTACKSIZE
    #endif
    .text
    - .align 32
    - .globl mcount, _mcount
    -mcount:
    + .align 32
    + .globl _mcount
    + .type _mcount,#function
    + .globl mcount
    + .type mcount,#function
    _mcount:
    +mcount:
    #ifdef CONFIG_STACK_DEBUG
    /*
    * Check whether %sp is dangerously low.
    @@ -55,6 +58,53 @@ _mcount:
    or %g3, %lo(panicstring), %o0
    call prom_halt
    nop
    +1:
    +#endif
    +#ifdef CONFIG_FTRACE
    +#ifdef CONFIG_DYNAMIC_FTRACE
    + mov %o7, %o0
    + .globl mcount_call
    +mcount_call:
    + call ftrace_stub
    + mov %o0, %o7
    +#else
    + sethi %hi(ftrace_trace_function), %g1
    + sethi %hi(ftrace_stub), %g2
    + ldx [%g1 + %lo(ftrace_trace_function)], %g1
    + or %g2, %lo(ftrace_stub), %g2
    + cmp %g1, %g2
    + be,pn %icc, 1f
    + mov %i7, %o1
    + jmpl %g1, %g0
    + mov %o7, %o0
    + /* not reached */
    +1:
    #endif
    -1: retl
    +#endif
    + retl
    nop
    + .size _mcount,.-_mcount
    + .size mcount,.-mcount
    +
    +#ifdef CONFIG_FTRACE
    + .globl ftrace_stub
    + .type ftrace_stub,#function
    +ftrace_stub:
    + retl
    + nop
    + .size ftrace_stub,.-ftrace_stub
    +#ifdef CONFIG_DYNAMIC_FTRACE
    + .globl ftrace_caller
    + .type ftrace_caller,#function
    +ftrace_caller:
    + mov %i7, %o1
    + mov %o7, %o0
    + .globl ftrace_call
    +ftrace_call:
    + call ftrace_stub
    + mov %o0, %o7
    + retl
    + nop
    + .size ftrace_caller,.-ftrace_caller
    +#endif
    +#endif
    --
    1.5.5.rc0.16.g02b00

    --
    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/

  2. Re: [PATCH 2/2]: sparc64: Add ftrace support.

    David Miller wrote:
    > Signed-off-by: David S. Miller
    > ---
    > arch/sparc64/Kconfig | 1 +
    > arch/sparc64/Kconfig.debug | 2 +-
    > arch/sparc64/kernel/Makefile | 1 +
    > arch/sparc64/kernel/ftrace.c | 99 ++++++++++++++++++++++++++++++++++++++++++
    > arch/sparc64/lib/mcount.S | 58 +++++++++++++++++++++++--
    > 5 files changed, 156 insertions(+), 5 deletions(-)
    > create mode 100644 arch/sparc64/kernel/ftrace.c
    >
    > diff --git a/arch/sparc64/Kconfig b/arch/sparc64/Kconfig
    > index 4c40862..56344b1 100644
    > --- a/arch/sparc64/Kconfig
    > +++ b/arch/sparc64/Kconfig
    > @@ -15,6 +15,7 @@ config SPARC64
    > select HAVE_LMB
    > select HAVE_ARCH_KGDB
    > select HAVE_IMMEDIATE
    > + select HAVE_FTRACE
    >
    > config GENERIC_TIME
    > bool
    > diff --git a/arch/sparc64/Kconfig.debug b/arch/sparc64/Kconfig.debug
    > index 6a4d28a..d6d32d1 100644
    > --- a/arch/sparc64/Kconfig.debug
    > +++ b/arch/sparc64/Kconfig.debug
    > @@ -33,7 +33,7 @@ config DEBUG_PAGEALLOC
    >
    > config MCOUNT
    > bool
    > - depends on STACK_DEBUG
    > + depends on STACK_DEBUG || FTRACE
    > default y
    >
    > config FRAME_POINTER
    > diff --git a/arch/sparc64/kernel/Makefile b/arch/sparc64/kernel/Makefile
    > index 311c797..52e933f 100644
    > --- a/arch/sparc64/kernel/Makefile
    > +++ b/arch/sparc64/kernel/Makefile
    > @@ -14,6 +14,7 @@ obj-y := process.o setup.o cpu.o idprom.o \
    > power.o sbus.o sparc64_ksyms.o chmc.o \
    > visemul.o prom.o of_device.o hvapi.o sstate.o mdesc.o
    >
    > +obj-$(CONFIG_DYNAMIC_FTRACE) += ftrace.o
    > obj-$(CONFIG_STACKTRACE) += stacktrace.o
    > obj-$(CONFIG_PCI) += ebus.o pci_common.o \
    > pci_psycho.o pci_sabre.o pci_schizo.o \
    > diff --git a/arch/sparc64/kernel/ftrace.c b/arch/sparc64/kernel/ftrace.c
    > new file mode 100644
    > index 0000000..f449e6d
    > --- /dev/null
    > +++ b/arch/sparc64/kernel/ftrace.c
    > @@ -0,0 +1,99 @@
    > +#include
    > +#include
    > +#include
    > +#include
    > +#include
    > +#include
    > +
    > +static const u32 ftrace_nop = 0x01000000;
    > +
    > +notrace int ftrace_ip_converted(unsigned long ip)


    FYI,

    If you look in the kernel/Makefile you will see a way to have the whole
    file not be traced. This will eliminate the need for the "notrace"
    annotation on all functions. I haven't update the x86 version of ftrace
    to do this so you probably just copied the old method from us.

    If you add the following in arch/sparc64/kernel/Makefile

    ifdef CONFIG_DYNAMIC_FTRACE
    obj-y += ftrace.o
    # Do not trace the ftrace code itself
    ORIG_CFLAGS := $(KBUILD_CFLAGS)
    KBUILD_CFLAGS = $(if $(filter-out ftrace, $(basename $(notdir $@))), \
    $(ORIG_CFLAGS), \
    $(subst -pg,,$(ORIG_CFLAGS)))
    endif

    Then you wont need the "notrace" annotations on the functions.

    /me needs to send Ingo a patch to do this for x86 as well.

    > +{
    > + u32 insn = *(u32 *) ip;
    > +
    > + return (insn == ftrace_nop);
    > +}
    > +
    > +notrace unsigned char *ftrace_nop_replace(void)
    > +{
    > + return (char *)&ftrace_nop;
    > +}
    > +
    > +notrace unsigned char *ftrace_call_replace(unsigned long ip, unsigned long addr)
    > +{
    > + static u32 call;
    > + s32 off;
    > +
    > + off = ((s32)addr - (s32)ip);
    > + call = 0x40000000 | ((u32)off >> 2);
    > +
    > + return (unsigned char *) &call;
    > +}
    > +
    > +notrace int
    > +ftrace_modify_code(unsigned long ip, unsigned char *old_code,
    > + unsigned char *new_code)
    > +{
    > + u32 old = *(u32 *)old_code;
    > + u32 new = *(u32 *)new_code;
    > + u32 replaced;
    > + int faulted;
    > +
    > + __asm__ __volatile__(
    > + "1: cas [%[ip]], %[old], %[new]\n"
    > + " flush %[ip]\n"
    > + " mov 0, %[faulted]\n"
    > + "2:\n"
    > + " .section .fixup,#alloc,#execinstr\n"
    > + " .align 4\n"
    > + "3: sethi %%hi(2b), %[faulted]\n"
    > + " jmpl %[faulted] + %%lo(2b), %%g0\n"
    > + " mov 1, %[faulted]\n"
    > + " .previous\n"
    > + " .section __ex_table,\"a\"\n"
    > + " .align 4\n"
    > + " .word 1b, 3b\n"
    > + " .previous\n"
    > + : "=r" (replaced), [faulted] "=r" (faulted)
    > + : [new] "0" (new), [old] "r" (old), [ip] "r" (ip)
    > + : "memory");


    I'm so use to the %1 %2 and %3's I should update the x86 code to use the
    names as well. (nice)

    > +
    > + if (replaced != old && replaced != new)
    > + faulted = 2;


    Thanks for putting in the "2", I may plan on doing something different
    later if the code didn't "fault" but something else happened. I should
    also make MACROS out of that and not uses 1 and 2.

    > +
    > + return faulted;
    > +}
    > +
    > +notrace int ftrace_update_ftrace_func(ftrace_func_t func)
    > +{
    > + unsigned long ip = (unsigned long)(&ftrace_call);
    > + unsigned char old[4], *new;
    > +
    > + memcpy(old, &ftrace_call, 4);
    > + new = ftrace_call_replace(ip, (unsigned long)func);
    > + return ftrace_modify_code(ip, old, new);
    > +}
    > +
    > +notrace int ftrace_mcount_set(unsigned long *data)
    > +{
    > + unsigned long ip = (long)(&mcount_call);
    > + unsigned long *addr = data;
    > + unsigned char old[4], *new;
    > +
    > + /*
    > + * Replace the mcount stub with a pointer to the
    > + * ip recorder function.
    > + */
    > + memcpy(old, &mcount_call, 4);
    > + new = ftrace_call_replace(ip, *addr);
    > + *addr = ftrace_modify_code(ip, old, new);
    > +
    > + return 0;
    > +}
    > +
    > +
    > +int __init ftrace_dyn_arch_init(void *data)
    > +{
    > + ftrace_mcount_set(data);
    > + return 0;
    > +}
    > diff --git a/arch/sparc64/lib/mcount.S b/arch/sparc64/lib/mcount.S
    > index 9e4534b..7735a7a 100644
    > --- a/arch/sparc64/lib/mcount.S
    > +++ b/arch/sparc64/lib/mcount.S
    > @@ -28,10 +28,13 @@ ovstack:
    > .skip OVSTACKSIZE
    > #endif
    > .text
    > - .align 32
    > - .globl mcount, _mcount
    > -mcount:
    > + .align 32
    > + .globl _mcount
    > + .type _mcount,#function
    > + .globl mcount
    > + .type mcount,#function
    > _mcount:
    > +mcount:
    > #ifdef CONFIG_STACK_DEBUG
    > /*
    > * Check whether %sp is dangerously low.
    > @@ -55,6 +58,53 @@ _mcount:
    > or %g3, %lo(panicstring), %o0
    > call prom_halt
    > nop
    > +1:
    > +#endif
    > +#ifdef CONFIG_FTRACE
    > +#ifdef CONFIG_DYNAMIC_FTRACE
    > + mov %o7, %o0
    > + .globl mcount_call
    > +mcount_call:
    > + call ftrace_stub
    > + mov %o0, %o7


    WOW! Sparc64 has some really efficient context saving!

    > +#else
    > + sethi %hi(ftrace_trace_function), %g1
    > + sethi %hi(ftrace_stub), %g2
    > + ldx [%g1 + %lo(ftrace_trace_function)], %g1
    > + or %g2, %lo(ftrace_stub), %g2
    > + cmp %g1, %g2
    > + be,pn %icc, 1f
    > + mov %i7, %o1
    > + jmpl %g1, %g0
    > + mov %o7, %o0
    > + /* not reached */
    > +1:
    > #endif
    > -1: retl
    > +#endif
    > + retl
    > nop
    > + .size _mcount,.-_mcount
    > + .size mcount,.-mcount
    > +
    > +#ifdef CONFIG_FTRACE
    > + .globl ftrace_stub
    > + .type ftrace_stub,#function
    > +ftrace_stub:
    > + retl
    > + nop
    > + .size ftrace_stub,.-ftrace_stub
    > +#ifdef CONFIG_DYNAMIC_FTRACE
    > + .globl ftrace_caller
    > + .type ftrace_caller,#function
    > +ftrace_caller:
    > + mov %i7, %o1
    > + mov %o7, %o0
    > + .globl ftrace_call
    > +ftrace_call:
    > + call ftrace_stub
    > + mov %o0, %o7
    > + retl
    > + nop
    > + .size ftrace_caller,.-ftrace_caller
    > +#endif
    > +#endif


    Looks great!

    Acked-by: Steven Rostedt

    -- 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/

  3. Re: [PATCH 2/2]: sparc64: Add ftrace support.

    From: Steven Rostedt
    Date: Wed, 14 May 2008 09:32:18 -0400

    > If you add the following in arch/sparc64/kernel/Makefile
    >
    > ifdef CONFIG_DYNAMIC_FTRACE
    > obj-y += ftrace.o
    > # Do not trace the ftrace code itself
    > ORIG_CFLAGS := $(KBUILD_CFLAGS)
    > KBUILD_CFLAGS = $(if $(filter-out ftrace, $(basename $(notdir $@))), \
    > $(ORIG_CFLAGS), \
    > $(subst -pg,,$(ORIG_CFLAGS)))
    > endif
    >
    > Then you wont need the "notrace" annotations on the functions.


    That frankly looks like crap compared to the simple "notrace"
    annotations in the *.c file.

    This Makefile turd is in fact seemingly multiplying in various
    places in the ftrace tree, please reverse this trend.
    --
    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/

  4. Re: [PATCH 2/2]: sparc64: Add ftrace support.

    David Miller wrote:
    > From: Steven Rostedt
    > Date: Wed, 14 May 2008 09:32:18 -0400
    >
    >> If you add the following in arch/sparc64/kernel/Makefile
    >>
    >> ifdef CONFIG_DYNAMIC_FTRACE
    >> obj-y += ftrace.o
    >> # Do not trace the ftrace code itself
    >> ORIG_CFLAGS := $(KBUILD_CFLAGS)
    >> KBUILD_CFLAGS = $(if $(filter-out ftrace, $(basename $(notdir $@))), \
    >> $(ORIG_CFLAGS), \
    >> $(subst -pg,,$(ORIG_CFLAGS)))
    >> endif
    >>
    >> Then you wont need the "notrace" annotations on the functions.

    >
    > That frankly looks like crap compared to the simple "notrace"
    > annotations in the *.c file.
    >
    > This Makefile turd is in fact seemingly multiplying in various
    > places in the ftrace tree, please reverse this trend.


    I wish it wasn't so ugly. The Makefile scripts allow you to add more
    CFLAGS to specific files but does not have a way to remove generic
    flags. If there was a better way then I would love to do it.

    The problem is that adding "notrace" on ever function that needs it,
    also becomes quite messy. Especially when you have files where all
    functions in the file needs a notrace and there is 40 or so small
    functions that this applies to.

    To add flags to file foo.c one simply needs to do

    CFLAGS_foo.o = -newflags

    It would be nice to have something like

    CFLAGS_REMOVE_foo.o = -pg

    Perhaps that is possible to do. Then the above "Makefile turd" wont be
    so crappy.

    -- 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/

  5. Re: [PATCH 2/2]: sparc64: Add ftrace support.

    From: Steven Rostedt
    Date: Wed, 14 May 2008 17:58:13 -0400

    > It would be nice to have something like
    >
    > CFLAGS_REMOVE_foo.o = -pg
    >
    > Perhaps that is possible to do. Then the above "Makefile turd" wont be
    > so crappy.


    That would be much nicer.
    --
    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/

+ Reply to Thread