| Message ID | 20180821222416.7771-1-stefan@agner.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ARM: use choice for kernel unwinders | expand |
On Wed, Aug 22, 2018 at 12:24 AM Stefan Agner <stefan@agner.ch> wrote: > > While in theory multiple unwinders could be compiled in, it does > not make sense in practise. Use a choice to make the unwinder > selection mutually exclusive and mandatory. > > Already before this commit it has not been possible to deselect > FRAME_POINTER. Remove the obsolete comment. > > Furthermore, to produce a meaningful backtrace with FRAME_POINTER > enabled the kernel needs a specific function prologue: > mov ip, sp > stmfd sp!, {fp, ip, lr, pc} > sub fp, ip, #4 > > To get to the required prologue gcc uses apcs and no-sched-prolog. > This compiler options are not available on clang, and clang is not > able to generate the required prologue. Make the FRAME_POINTER > config symbol depending on !clang. > > Suggested-by: Arnd Bergmann <arnd@arndb.de> > Signed-off-by: Stefan Agner <stefan@agner.ch> Looks ok to me. I've added it to my randconfig test environment, you will hear from me within a day if I run into build regressions. We may still want to clean up these three lines: lib/Kconfig.debug: select FRAME_POINTER if !MIPS && !PPC && !ARM_UNWIND && !S390 && !MICROBLAZE && !ARC && !X86 lib/Kconfig.debug: select FRAME_POINTER if !MIPS && !PPC && !S390 && !MICROBLAZE && !ARM_UNWIND && !ARC && !X86 lib/Kconfig.debug: select FRAME_POINTER if !MIPS && !PPC && !S390 && !MICROBLAZE && !ARM_UNWIND && !ARC && !X86 in which ARM is the odd case that currently depends on an architecture specific rather than the architecture itself. We could introduce a 'config ARCH_HAS_UNWINDER' symbol that gets selected by mips, ppc, s390, microblaze, arm and x86 unconditionally, and then simplify the 'select' statements here. Arnd
On Wed, Aug 22, 2018 at 12:02 PM Arnd Bergmann <arnd@arndb.de> wrote: > > On Wed, Aug 22, 2018 at 12:24 AM Stefan Agner <stefan@agner.ch> wrote: > > > > While in theory multiple unwinders could be compiled in, it does > > not make sense in practise. Use a choice to make the unwinder > > selection mutually exclusive and mandatory. > > > > Already before this commit it has not been possible to deselect > > FRAME_POINTER. Remove the obsolete comment. > > > > Furthermore, to produce a meaningful backtrace with FRAME_POINTER > > enabled the kernel needs a specific function prologue: > > mov ip, sp > > stmfd sp!, {fp, ip, lr, pc} > > sub fp, ip, #4 > > > > To get to the required prologue gcc uses apcs and no-sched-prolog. > > This compiler options are not available on clang, and clang is not > > able to generate the required prologue. Make the FRAME_POINTER > > config symbol depending on !clang. > > > > Suggested-by: Arnd Bergmann <arnd@arndb.de> > > Signed-off-by: Stefan Agner <stefan@agner.ch> > > Looks ok to me. I've added it to my randconfig test environment, you > will hear from me within a day if I run into build regressions. I needed a small hunk to avoid a warning: WARNING: unmet direct dependencies detected for FRAME_POINTER Depends on [n]: DEBUG_KERNEL [=y] && (M68K || UML || SUPERH) || ARCH_WANT_FRAME_POINTERS [=n] Selected by [y]: - LOCKDEP [=y] && DEBUG_KERNEL [=y] && LOCK_DEBUGGING_SUPPORT [=y] && !MIPS && !PPC && !ARM_UNWIND [=n] && !S390 && !MICROBLAZE && !ARC && !X86 - UNWINDER_FRAME_POINTER [=y] && <choice> && !THUMB2_KERNEL [=n] && !CC_IS_CLANG [=n] diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug index 6e27419b90c0..08f914be6248 100644 --- a/arch/arm/Kconfig.debug +++ b/arch/arm/Kconfig.debug @@ -57,6 +57,7 @@ choice config UNWINDER_FRAME_POINTER bool "Frame pointer unwinder" depends on !THUMB2_KERNEL && !CC_IS_CLANG + select ARCH_WANT_FRAME_POINTERS select FRAME_POINTER help This option enables the frame pointer unwinder for unwinding
On 22.08.2018 12:02, Arnd Bergmann wrote: > On Wed, Aug 22, 2018 at 12:24 AM Stefan Agner <stefan@agner.ch> wrote: >> >> While in theory multiple unwinders could be compiled in, it does >> not make sense in practise. Use a choice to make the unwinder >> selection mutually exclusive and mandatory. >> >> Already before this commit it has not been possible to deselect >> FRAME_POINTER. Remove the obsolete comment. >> >> Furthermore, to produce a meaningful backtrace with FRAME_POINTER >> enabled the kernel needs a specific function prologue: >> mov ip, sp >> stmfd sp!, {fp, ip, lr, pc} >> sub fp, ip, #4 >> >> To get to the required prologue gcc uses apcs and no-sched-prolog. >> This compiler options are not available on clang, and clang is not >> able to generate the required prologue. Make the FRAME_POINTER >> config symbol depending on !clang. >> >> Suggested-by: Arnd Bergmann <arnd@arndb.de> >> Signed-off-by: Stefan Agner <stefan@agner.ch> > > Looks ok to me. I've added it to my randconfig test environment, you > will hear from me within a day if I run into build regressions. > > We may still want to clean up these three lines: > > lib/Kconfig.debug: select FRAME_POINTER if !MIPS && !PPC && > !ARM_UNWIND && !S390 && !MICROBLAZE && !ARC && !X86 > lib/Kconfig.debug: select FRAME_POINTER if !MIPS && !PPC && !S390 && > !MICROBLAZE && !ARM_UNWIND && !ARC && !X86 > lib/Kconfig.debug: select FRAME_POINTER if !MIPS && !PPC && !S390 && > !MICROBLAZE && !ARM_UNWIND && !ARC && !X86 > > in which ARM is the odd case that currently depends on an architecture > specific rather than the architecture itself. I guess we would just follow X86 lead by saying ARM is guaranteed to have unwinding support, and hence we can add !ARM. > We could introduce a 'config ARCH_HAS_UNWINDER' symbol that gets > selected by mips, ppc, s390, microblaze, arm and x86 unconditionally, > and then simplify the 'select' statements here. Yeah I was thinking about something like that too. It seems to be a bit weird to me that lib/Kconfig.debug selects a specific stack unwinding technique... Ideally other config symbol should just ask arch to make sure a unwinding technique is available (NEED_STACK_UNWINDING?) and arch then makes sure to provide a reasonable default. This then also would make it possible to select no stack unwinding in case arch supports that and all the users of stack unwinding are disabled too. Not sure how that exactly would look like in Kconfig, I was thinking like: choice prompt "Choose kernel unwinder" optional if !NEED_STACK_UNWINDING default UNWINDER_ARM if AEABI && !FUNCTION_GRAPH_TRACER default UNWINDER_FRAME_POINTER if !AEABI || FUNCTION_GRAPH_TRACER But "optional if" does not exist yet :-) Reading the comments in arch/arm/Kconfig.debug seems to suggest that once upon a time it was possible to disable stack unwinding on ARM. But then, maybe we don't really want to go there? Might be interesting for tinification efforts. -- Stefan
On Wed, Aug 22, 2018 at 4:38 PM Stefan Agner <stefan@agner.ch> wrote: > On 22.08.2018 12:02, Arnd Bergmann wrote: > > On Wed, Aug 22, 2018 at 12:24 AM Stefan Agner <stefan@agner.ch> wrote: > > > > Looks ok to me. I've added it to my randconfig test environment, you > > will hear from me within a day if I run into build regressions. > > > > We may still want to clean up these three lines: > > > > lib/Kconfig.debug: select FRAME_POINTER if !MIPS && !PPC && > > !ARM_UNWIND && !S390 && !MICROBLAZE && !ARC && !X86 > > lib/Kconfig.debug: select FRAME_POINTER if !MIPS && !PPC && !S390 && > > !MICROBLAZE && !ARM_UNWIND && !ARC && !X86 > > lib/Kconfig.debug: select FRAME_POINTER if !MIPS && !PPC && !S390 && > > !MICROBLAZE && !ARM_UNWIND && !ARC && !X86 > > > > in which ARM is the odd case that currently depends on an architecture > > specific rather than the architecture itself. > > I guess we would just follow X86 lead by saying ARM is guaranteed to > have unwinding support, and hence we can add !ARM. Right, that was the idea. > > We could introduce a 'config ARCH_HAS_UNWINDER' symbol that gets > > selected by mips, ppc, s390, microblaze, arm and x86 unconditionally, > > and then simplify the 'select' statements here. > > Yeah I was thinking about something like that too. > > It seems to be a bit weird to me that lib/Kconfig.debug selects a > specific stack unwinding technique... This must be a historic artifact from the time when FRAME_POINTER was the only unwinding method that existed. We may also have some architectures that don't support any unwinding. > Ideally other config symbol should just ask arch to make sure a > unwinding technique is available (NEED_STACK_UNWINDING?) and arch then > makes sure to provide a reasonable default. > > This then also would make it possible to select no stack unwinding in > case arch supports that and all the users of stack unwinding are > disabled too. Not sure how that exactly would look like in Kconfig, I > was thinking like: > > choice > prompt "Choose kernel unwinder" > optional if !NEED_STACK_UNWINDING > default UNWINDER_ARM if AEABI && !FUNCTION_GRAPH_TRACER > default UNWINDER_FRAME_POINTER if !AEABI || FUNCTION_GRAPH_TRACER > > But "optional if" does not exist yet :-) You can write that as choice prompt "Choose kernel unwinder" if NEED_STACK_UNWINDING This will hide the prompt when NEED_STACK_UNWINDING is disabled, making it impossible to pick one of the two unwinders. > Reading the comments in arch/arm/Kconfig.debug seems to suggest that > once upon a time it was possible to disable stack unwinding on ARM. > > But then, maybe we don't really want to go there? Might be interesting > for tinification efforts. I'd leave that for another day ;-) Arnd
diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug index 693f84392f1b..ba09e744384a 100644 --- a/arch/arm/Kconfig.debug +++ b/arch/arm/Kconfig.debug @@ -48,30 +48,41 @@ config DEBUG_WX If in doubt, say "Y". -# RMK wants arm kernels compiled with frame pointers or stack unwinding. -# If you know what you are doing and are willing to live without stack -# traces, you can get a slightly smaller kernel by setting this option to -# n, but then RMK will have to kill you ;). -config FRAME_POINTER - bool - depends on !THUMB2_KERNEL - default y if !ARM_UNWIND || FUNCTION_GRAPH_TRACER +choice + prompt "Choose kernel unwinder" + default UNWINDER_ARM if AEABI && !FUNCTION_GRAPH_TRACER + default UNWINDER_FRAME_POINTER if !AEABI || FUNCTION_GRAPH_TRACER + help + This determines which method will be used for unwinding kernel stack + traces for panics, oopses, bugs, warnings, perf, /proc/<pid>/stack, + livepatch, lockdep, and more. + +config UNWINDER_FRAME_POINTER + bool "Frame pointer unwinder" + depends on !THUMB2_KERNEL && !CC_IS_CLANG + select FRAME_POINTER help - If you say N here, the resulting kernel will be slightly smaller and - faster. However, if neither FRAME_POINTER nor ARM_UNWIND are enabled, - when a problem occurs with the kernel, the information that is - reported is severely limited. + This option enables the frame pointer unwinder for unwinding + kernel stack traces. -config ARM_UNWIND - bool "Enable stack unwinding support (EXPERIMENTAL)" +config UNWINDER_ARM + bool "ARM EABI stack unwinder" depends on AEABI - default y + select ARM_UNWIND help This option enables stack unwinding support in the kernel using the information automatically generated by the compiler. The resulting kernel image is slightly bigger but the performance is not affected. Currently, this feature - only works with EABI compilers. If unsure say Y. + only works with EABI compilers. + +endchoice + +config ARM_UNWIND + bool + +config FRAME_POINTER + bool config OLD_MCOUNT bool
While in theory multiple unwinders could be compiled in, it does not make sense in practise. Use a choice to make the unwinder selection mutually exclusive and mandatory. Already before this commit it has not been possible to deselect FRAME_POINTER. Remove the obsolete comment. Furthermore, to produce a meaningful backtrace with FRAME_POINTER enabled the kernel needs a specific function prologue: mov ip, sp stmfd sp!, {fp, ip, lr, pc} sub fp, ip, #4 To get to the required prologue gcc uses apcs and no-sched-prolog. This compiler options are not available on clang, and clang is not able to generate the required prologue. Make the FRAME_POINTER config symbol depending on !clang. Suggested-by: Arnd Bergmann <arnd@arndb.de> Signed-off-by: Stefan Agner <stefan@agner.ch> --- arch/arm/Kconfig.debug | 43 ++++++++++++++++++++++++++---------------- 1 file changed, 27 insertions(+), 16 deletions(-)