lkml.org 
[lkml]   [2010]   [Oct]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 7/7] jump label: Add work around to i386 gcc asm goto bug
    * Steven Rostedt (rostedt@goodmis.org) wrote:
    > From: Steven Rostedt <srostedt@redhat.com>
    >
    > On i386 (not x86_64) early implementations of gcc would have a bug
    > with asm goto causing it to produce code like the following:
    >
    > (This was noticed by Peter Zijlstra)
    >
    > 56 pushl 0
    > 67 nopl jmp 0x6f
    > popl
    > jmp 0x8c
    >
    > 6f mov
    > test
    > je 0x8c
    >
    > 8c mov
    > call *(%esp)
    >
    > The jump added in the asm goto skipped over the popl that matched
    > the pushl 0, which lead up to a quick crash of the system when
    > the jump was enabled. The nopl is defined in the asm goto () statement
    > and when tracepoints are enabled, the nop changes to a jump to the label
    > that was specified by the asm goto. asm goto is suppose to tell gcc that
    > the code in the asm might jump to an external label. Here gcc obviously
    > fails to make that work.
    >
    > The bug report for gcc is here:
    >
    > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46226
    >
    > The bug only appears on x86 when not compiled with
    > -maccumulate-outgoing-args. This option is always set on x86_64 and it
    > is also the work around for a function graph tracer i386 bug.
    > (See commit: 746357d6a526d6da9d89a2ec645b28406e959c2e)
    > This explains why the bug only showed up on i386 when function graph
    > tracer was not enabled.
    >
    > This patch now adds a CONFIG_JUMP_LABEL option that is default
    > off instead of using jump labels by default. When jump labels are
    > enabled, the -maccumulate-outgoing-args will be used (causing a
    > slightly larger kernel image on i386). This option will exist
    > until we have a way to detect if the gcc compiler in use is safe
    > to use on all configurations without the work around.
    >
    > Note, there exists such a test, but for now we will keep the enabling
    > of jump label as a manual option.
    >
    > Archs that know the compiler is safe with asm goto, may choose to
    > select JUMP_LABEL and enable it by default.

    Looks good. You can add my

    Acked-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

    if you feel like it.

    Thanks,

    Mathieu

    >
    > Reported-by: Ingo Molnar <mingo@elte.hu>
    > Cause-discovered-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
    > Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
    > Cc: Thomas Gleixner <tglx@linutronix.de>
    > Cc: Jason Baron <jbaron@redhat.com>
    > Cc: H. Peter Anvin <hpa@zytor.com>
    > Cc: David Daney <ddaney@caviumnetworks.com>
    > Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
    > Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
    > Cc: David Miller <davem@davemloft.net>
    > Cc: Richard Henderson <rth@redhat.com>
    > LKML-Reference: <1288028746.3673.11.camel@laptop>
    > Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
    > ---
    > arch/Kconfig | 14 ++++++++++++++
    > arch/x86/Makefile_32.cpu | 13 ++++++++++++-
    > include/linux/jump_label.h | 2 +-
    > 3 files changed, 27 insertions(+), 2 deletions(-)
    >
    > diff --git a/arch/Kconfig b/arch/Kconfig
    > index 53d7f61..8bf0fa6 100644
    > --- a/arch/Kconfig
    > +++ b/arch/Kconfig
    > @@ -42,6 +42,20 @@ config KPROBES
    > for kernel debugging, non-intrusive instrumentation and testing.
    > If in doubt, say "N".
    >
    > +config JUMP_LABEL
    > + bool "Optimize trace point call sites"
    > + depends on HAVE_ARCH_JUMP_LABEL
    > + help
    > + If it is detected that the compiler has support for "asm goto",
    > + the kernel will compile trace point locations with just a
    > + nop instruction. When trace points are enabled, the nop will
    > + be converted to a jump to the trace function. This technique
    > + lowers overhead and stress on the branch prediction of the
    > + processor.
    > +
    > + On i386, options added to the compiler flags may increase
    > + the size of the kernel slightly.
    > +
    > config OPTPROBES
    > def_bool y
    > depends on KPROBES && HAVE_OPTPROBES
    > diff --git a/arch/x86/Makefile_32.cpu b/arch/x86/Makefile_32.cpu
    > index 1255d95..f2ee1ab 100644
    > --- a/arch/x86/Makefile_32.cpu
    > +++ b/arch/x86/Makefile_32.cpu
    > @@ -51,7 +51,18 @@ cflags-$(CONFIG_X86_GENERIC) += $(call tune,generic,$(call tune,i686))
    > # prologue (push %ebp, mov %esp, %ebp) which breaks the function graph
    > # tracer assumptions. For i686, generic, core2 this is set by the
    > # compiler anyway
    > -cflags-$(CONFIG_FUNCTION_GRAPH_TRACER) += $(call cc-option,-maccumulate-outgoing-args)
    > +ifeq ($(CONFIG_FUNCTION_GRAPH_TRACER), y)
    > +ADD_ACCUMULATE_OUTGOING_ARGS := y
    > +endif
    > +
    > +# Work around to a bug with asm goto with first implementations of it
    > +# in gcc causing gcc to mess up the push and pop of the stack in some
    > +# uses of asm goto.
    > +ifeq ($(CONFIG_JUMP_LABEL), y)
    > +ADD_ACCUMULATE_OUTGOING_ARGS := y
    > +endif
    > +
    > +cflags-$(ADD_ACCUMULATE_OUTGOING_ARGS) += $(call cc-option,-maccumulate-outgoing-args)
    >
    > # Bug fix for binutils: this option is required in order to keep
    > # binutils from generating NOPL instructions against our will.
    > diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
    > index 1947a12..7880f18 100644
    > --- a/include/linux/jump_label.h
    > +++ b/include/linux/jump_label.h
    > @@ -1,7 +1,7 @@
    > #ifndef _LINUX_JUMP_LABEL_H
    > #define _LINUX_JUMP_LABEL_H
    >
    > -#if defined(CC_HAVE_ASM_GOTO) && defined(CONFIG_HAVE_ARCH_JUMP_LABEL)
    > +#if defined(CC_HAVE_ASM_GOTO) && defined(CONFIG_JUMP_LABEL)
    > # include <asm/jump_label.h>
    > # define HAVE_JUMP_LABEL
    > #endif
    > --
    > 1.7.1
    >
    >

    --
    Mathieu Desnoyers
    Operating System Efficiency R&D Consultant
    EfficiOS Inc.
    http://www.efficios.com


    \
     
     \ /
      Last update: 2010-10-29 22:05    [W:0.032 / U:3.212 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site