[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 10 Oct 2006 16:16:11 -0700
From: Jeremy Fitzhardinge <jeremy@...p.org>
To: Paul Mackerras <paulus@...ba.org>
CC: Andrew Morton <akpm@...l.org>, linux-kernel@...r.kernel.org,
Michael Ellerman <michael@...erman.id.au>
Subject: Re: 2.6.19-rc1-mm1
Paul Mackerras wrote:
> Andrew Morton writes:
>
>
>> My plan was to pathetically spam the powerpc guys with it once all the
>> above is merged up. I took a close look and couldn't see why it was
>> failing.
>>
>
> What was the failure?
>
I've included it below. But Michael Ellerman said it worked OK for him
when applied to the plain Linus tree, and Andrew said that
ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.18/2.6.18-mm3/hot-fixes/slab-reduce-numa-text-size-tidy-fix.patch
fixed it. So I thought it was OK.
J
> On Wed, 04 Oct 2006 00:22:33 -0600
> akpm@...l.org wrote:
>
>> > Subject: generic-implementatation-of-bug-fix
>
> x86_64 works OK. powerpc compiles now, but hangs after "returning from
> prom_init". I can't see why. The quickest way to fix this is to merge it
> into mainline as-is then whistle innocently.
>
> Can any of the powerpc guys spot-the-bug??
>
> Thanks.
>
>
> From: Jeremy Fitzhardinge <jeremy@...p.org>
>
> This makes powerpc use the generic BUG machinery. The biggest reports the
> function name, since it is redundant with kallsyms, and not needed in general.
>
> There is an overall reduction of code, since module_32/64 duplicated several
> functions.
>
> Unfortunately there's no way to tell gcc that BUG won't return, so the BUG
> macro includes a goto loop. This will generate a real jmp instruction, which
> is never used.
>
> BTW, powerpc doesn't seem to be using BUG_OPCODE or BUG_ILLEGAL_INSTRUCTION
> for actual BUGs any more (I presume they were once used). There are still a
> couple of uses of those macros elsewhere (kernel/prom_init.c and
> kernel/head_64.S); should be converted to "twi 31,0,0" as well?
>
> [akpm@...l.org: build fixes]
> Signed-off-by: Jeremy Fitzhardinge <jeremy@...p.org>
> Cc: Andi Kleen <ak@....de>
> Cc: Hugh Dickens <hugh@...itas.com>
> Cc: Michael Ellerman <michael@...erman.id.au>
> Cc: Paul Mackerras <paulus@...ba.org>
> Cc: Benjamin Herrenschmidt <benh@...nel.crashing.org>
> Cc: Rusty Russell <rusty@...tcorp.com.au>
> Signed-off-by: Andrew Morton <akpm@...l.org>
> ---
>
> arch/powerpc/Kconfig | 5 +
> arch/powerpc/kernel/module_32.c | 43 +-------------
> arch/powerpc/kernel/module_64.c | 43 +-------------
> arch/powerpc/kernel/traps.c | 54 ++----------------
> arch/powerpc/kernel/vmlinux.lds.S | 6 --
> arch/powerpc/xmon/xmon.c | 10 +--
> include/asm-powerpc/bug.h | 83 ++++++++++++++--------------
> include/asm-powerpc/module.h | 2
> 8 files changed, 65 insertions(+), 181 deletions(-)
>
> diff -puN arch/powerpc/Kconfig~generic-bug-for-powerpc arch/powerpc/Kconfig
> --- a/arch/powerpc/Kconfig~generic-bug-for-powerpc
> +++ a/arch/powerpc/Kconfig
> @@ -99,6 +99,11 @@ config AUDIT_ARCH
> bool
> default y
>
> +config GENERIC_BUG
> + bool
> + default y
> + depends on BUG
> +
> config DEFAULT_UIMAGE
> bool
> help
> diff -puN arch/powerpc/kernel/module_32.c~generic-bug-for-powerpc arch/powerpc/kernel/module_32.c
> --- a/arch/powerpc/kernel/module_32.c~generic-bug-for-powerpc
> +++ a/arch/powerpc/kernel/module_32.c
> @@ -23,6 +23,7 @@
> #include <linux/string.h>
> #include <linux/kernel.h>
> #include <linux/cache.h>
> +#include <linux/bug.h>
>
> #if 0
> #define DEBUGP printk
> @@ -273,48 +274,10 @@ int module_finalize(const Elf_Ehdr *hdr,
> const Elf_Shdr *sechdrs,
> struct module *me)
> {
> - char *secstrings;
> - unsigned int i;
> -
> - me->arch.bug_table = NULL;
> - me->arch.num_bugs = 0;
> -
> - /* Find the __bug_table section, if present */
> - secstrings = (char *)hdr + sechdrs[hdr->e_shstrndx].sh_offset;
> - for (i = 1; i < hdr->e_shnum; i++) {
> - if (strcmp(secstrings+sechdrs[i].sh_name, "__bug_table"))
> - continue;
> - me->arch.bug_table = (void *) sechdrs[i].sh_addr;
> - me->arch.num_bugs = sechdrs[i].sh_size / sizeof(struct bug_entry);
> - break;
> - }
> -
> - /*
> - * Strictly speaking this should have a spinlock to protect against
> - * traversals, but since we only traverse on BUG()s, a spinlock
> - * could potentially lead to deadlock and thus be counter-productive.
> - */
> - list_add(&me->arch.bug_list, &module_bug_list);
> -
> - return 0;
> + return module_bug_finalize(hdr, sechdrs, me);
> }
>
> void module_arch_cleanup(struct module *mod)
> {
> - list_del(&mod->arch.bug_list);
> -}
> -
> -struct bug_entry *module_find_bug(unsigned long bugaddr)
> -{
> - struct mod_arch_specific *mod;
> - unsigned int i;
> - struct bug_entry *bug;
> -
> - list_for_each_entry(mod, &module_bug_list, bug_list) {
> - bug = mod->bug_table;
> - for (i = 0; i < mod->num_bugs; ++i, ++bug)
> - if (bugaddr == bug->bug_addr)
> - return bug;
> - }
> - return NULL;
> + module_bug_cleanup(mod);
> }
> diff -puN arch/powerpc/kernel/module_64.c~generic-bug-for-powerpc arch/powerpc/kernel/module_64.c
> --- a/arch/powerpc/kernel/module_64.c~generic-bug-for-powerpc
> +++ a/arch/powerpc/kernel/module_64.c
> @@ -20,6 +20,7 @@
> #include <linux/moduleloader.h>
> #include <linux/err.h>
> #include <linux/vmalloc.h>
> +#include <linux/bug.h>
> #include <asm/module.h>
> #include <asm/uaccess.h>
>
> @@ -416,48 +417,10 @@ LIST_HEAD(module_bug_list);
> int module_finalize(const Elf_Ehdr *hdr,
> const Elf_Shdr *sechdrs, struct module *me)
> {
> - char *secstrings;
> - unsigned int i;
> -
> - me->arch.bug_table = NULL;
> - me->arch.num_bugs = 0;
> -
> - /* Find the __bug_table section, if present */
> - secstrings = (char *)hdr + sechdrs[hdr->e_shstrndx].sh_offset;
> - for (i = 1; i < hdr->e_shnum; i++) {
> - if (strcmp(secstrings+sechdrs[i].sh_name, "__bug_table"))
> - continue;
> - me->arch.bug_table = (void *) sechdrs[i].sh_addr;
> - me->arch.num_bugs = sechdrs[i].sh_size / sizeof(struct bug_entry);
> - break;
> - }
> -
> - /*
> - * Strictly speaking this should have a spinlock to protect against
> - * traversals, but since we only traverse on BUG()s, a spinlock
> - * could potentially lead to deadlock and thus be counter-productive.
> - */
> - list_add(&me->arch.bug_list, &module_bug_list);
> -
> - return 0;
> + return module_bug_finalize(hdr, sechdrs, me);
> }
>
> void module_arch_cleanup(struct module *mod)
> {
> - list_del(&mod->arch.bug_list);
> -}
> -
> -struct bug_entry *module_find_bug(unsigned long bugaddr)
> -{
> - struct mod_arch_specific *mod;
> - unsigned int i;
> - struct bug_entry *bug;
> -
> - list_for_each_entry(mod, &module_bug_list, bug_list) {
> - bug = mod->bug_table;
> - for (i = 0; i < mod->num_bugs; ++i, ++bug)
> - if (bugaddr == bug->bug_addr)
> - return bug;
> - }
> - return NULL;
> + module_bug_cleanup(mod);
> }
> diff -puN arch/powerpc/kernel/traps.c~generic-bug-for-powerpc arch/powerpc/kernel/traps.c
> --- a/arch/powerpc/kernel/traps.c~generic-bug-for-powerpc
> +++ a/arch/powerpc/kernel/traps.c
> @@ -32,6 +32,7 @@
> #include <linux/kprobes.h>
> #include <linux/kexec.h>
> #include <linux/backlight.h>
> +#include <linux/bug.h>
>
> #include <asm/kdebug.h>
> #include <asm/pgtable.h>
> @@ -731,54 +732,9 @@ static int emulate_instruction(struct pt
> return -EINVAL;
> }
>
> -/*
> - * Look through the list of trap instructions that are used for BUG(),
> - * BUG_ON() and WARN_ON() and see if we hit one. At this point we know
> - * that the exception was caused by a trap instruction of some kind.
> - * Returns 1 if we should continue (i.e. it was a WARN_ON) or 0
> - * otherwise.
> - */
> -extern struct bug_entry __start___bug_table[], __stop___bug_table[];
> -
> -#ifndef CONFIG_MODULES
> -#define module_find_bug(x) NULL
> -#endif
> -
> -struct bug_entry *find_bug(unsigned long bugaddr)
> +int is_valid_bugaddr(unsigned long addr)
> {
> - struct bug_entry *bug;
> -
> - for (bug = __start___bug_table; bug < __stop___bug_table; ++bug)
> - if (bugaddr == bug->bug_addr)
> - return bug;
> - return module_find_bug(bugaddr);
> -}
> -
> -static int check_bug_trap(struct pt_regs *regs)
> -{
> - struct bug_entry *bug;
> - unsigned long addr;
> -
> - if (regs->msr & MSR_PR)
> - return 0; /* not in kernel */
> - addr = regs->nip; /* address of trap instruction */
> - if (addr < PAGE_OFFSET)
> - return 0;
> - bug = find_bug(regs->nip);
> - if (bug == NULL)
> - return 0;
> - if (bug->line & BUG_WARNING_TRAP) {
> - /* this is a WARN_ON rather than BUG/BUG_ON */
> - printk(KERN_ERR "Badness in %s at %s:%ld\n",
> - bug->function, bug->file,
> - bug->line & ~BUG_WARNING_TRAP);
> - dump_stack();
> - return 1;
> - }
> - printk(KERN_CRIT "kernel BUG in %s at %s:%ld!\n",
> - bug->function, bug->file, bug->line);
> -
> - return 0;
> + return is_kernel_addr(addr);
> }
>
> void __kprobes program_check_exception(struct pt_regs *regs)
> @@ -812,7 +768,9 @@ void __kprobes program_check_exception(s
> return;
> if (debugger_bpt(regs))
> return;
> - if (check_bug_trap(regs)) {
> +
> + if (!(regs->msr & MSR_PR) && /* not user-mode */
> + report_bug(regs->nip) == BUG_TRAP_TYPE_WARN) {
> regs->nip += 4;
> return;
> }
> diff -puN arch/powerpc/kernel/vmlinux.lds.S~generic-bug-for-powerpc arch/powerpc/kernel/vmlinux.lds.S
> --- a/arch/powerpc/kernel/vmlinux.lds.S~generic-bug-for-powerpc
> +++ a/arch/powerpc/kernel/vmlinux.lds.S
> @@ -62,11 +62,7 @@ SECTIONS
> __stop___ex_table = .;
> }
>
> - __bug_table : {
> - __start___bug_table = .;
> - *(__bug_table)
> - __stop___bug_table = .;
> - }
> + BUG_TABLE
>
> /*
> * Init sections discarded at runtime
> diff -puN arch/powerpc/xmon/xmon.c~generic-bug-for-powerpc arch/powerpc/xmon/xmon.c
> --- a/arch/powerpc/xmon/xmon.c~generic-bug-for-powerpc
> +++ a/arch/powerpc/xmon/xmon.c
> @@ -19,6 +19,7 @@
> #include <linux/module.h>
> #include <linux/sysrq.h>
> #include <linux/interrupt.h>
> +#include <linux/bug.h>
>
> #include <asm/ptrace.h>
> #include <asm/string.h>
> @@ -32,7 +33,6 @@
> #include <asm/cputable.h>
> #include <asm/rtas.h>
> #include <asm/sstep.h>
> -#include <asm/bug.h>
>
> #ifdef CONFIG_PPC64
> #include <asm/hvcall.h>
> @@ -1329,7 +1329,7 @@ static void backtrace(struct pt_regs *ex
>
> static void print_bug_trap(struct pt_regs *regs)
> {
> - struct bug_entry *bug;
> + const struct bug_entry *bug;
> unsigned long addr;
>
> if (regs->msr & MSR_PR)
> @@ -1340,11 +1340,11 @@ static void print_bug_trap(struct pt_reg
> bug = find_bug(regs->nip);
> if (bug == NULL)
> return;
> - if (bug->line & BUG_WARNING_TRAP)
> + if (is_warning_bug(bug))
> return;
>
> - printf("kernel BUG in %s at %s:%d!\n",
> - bug->function, bug->file, (unsigned int)bug->line);
> + printf("kernel BUG at %s:%u!\n",
> + bug->file, bug->line);
> }
>
> void excprint(struct pt_regs *fp)
> diff -puN include/asm-powerpc/bug.h~generic-bug-for-powerpc include/asm-powerpc/bug.h
> --- a/include/asm-powerpc/bug.h~generic-bug-for-powerpc
> +++ a/include/asm-powerpc/bug.h
> @@ -13,37 +13,40 @@
>
> #ifndef __ASSEMBLY__
>
> -struct bug_entry {
> - unsigned long bug_addr;
> - long line;
> - const char *file;
> - const char *function;
> -};
> -
> -struct bug_entry *find_bug(unsigned long bugaddr);
> -
> -/*
> - * If this bit is set in the line number it means that the trap
> - * is for WARN_ON rather than BUG or BUG_ON.
> - */
> -#define BUG_WARNING_TRAP 0x1000000
> -
> #ifdef CONFIG_BUG
>
> +/* _EMIT_BUG_ENTRY expects args %0,%1,%2,%3 to be FILE, LINE, flags and
> + sizeof(struct bug_entry), respectively */
> +#ifdef CONFIG_DEBUG_BUGVERBOSE
> +#define _EMIT_BUG_ENTRY \
> + ".section __bug_table,\"a\"\n" \
> + "2:\t" PPC_LONG "1b, %0\n" \
> + "\t.short %1, %2\n" \
> + ".org 2b+%3\n" \
> + ".previous\n"
> +#else
> +#define _EMIT_BUG_ENTRY \
> + ".section __bug_table,\"a\"\n" \
> + "2:\t" PPC_LONG "1b\n" \
> + "\t.short %2\n" \
> + ".org 2b+%3\n" \
> + ".previous\n"
> +#endif
> +
> /*
> * BUG_ON() and WARN_ON() do their best to cooperate with compile-time
> * optimisations. However depending on the complexity of the condition
> * some compiler versions may not produce optimal results.
> */
>
> -#define BUG() do { \
> - __asm__ __volatile__( \
> - "1: twi 31,0,0\n" \
> - ".section __bug_table,\"a\"\n" \
> - "\t"PPC_LONG" 1b,%0,%1,%2\n" \
> - ".previous" \
> - : : "i" (__LINE__), "i" (__FILE__), "i" (__FUNCTION__)); \
> -} while (0)
> +#define BUG() do { \
> + __asm__ __volatile__( \
> + "1: twi 31,0,0\n" \
> + _EMIT_BUG_ENTRY \
> + : : "i" (__FILE__), "i" (__LINE__), \
> + "i" (0), "i" (sizeof(struct bug_entry))); \
> + for(;;) ; \
> + } while (0)
>
> #define BUG_ON(x) do { \
> if (__builtin_constant_p(x)) { \
> @@ -51,23 +54,22 @@ struct bug_entry *find_bug(unsigned long
> BUG(); \
> } else { \
> __asm__ __volatile__( \
> - "1: "PPC_TLNEI" %0,0\n" \
> - ".section __bug_table,\"a\"\n" \
> - "\t"PPC_LONG" 1b,%1,%2,%3\n" \
> - ".previous" \
> - : : "r" ((long)(x)), "i" (__LINE__), \
> - "i" (__FILE__), "i" (__FUNCTION__)); \
> + "1: "PPC_TLNEI" %4,0\n" \
> + _EMIT_BUG_ENTRY \
> + : : "i" (__FILE__), "i" (__LINE__), "i" (0), \
> + "i" (sizeof(struct bug_entry)), \
> + "r" ((long)(x))); \
> + for(;;) ; \
> } \
> } while (0)
>
> #define __WARN() do { \
> __asm__ __volatile__( \
> "1: twi 31,0,0\n" \
> - ".section __bug_table,\"a\"\n" \
> - "\t"PPC_LONG" 1b,%0,%1,%2\n" \
> - ".previous" \
> - : : "i" (__LINE__ + BUG_WARNING_TRAP), \
> - "i" (__FILE__), "i" (__FUNCTION__)); \
> + _EMIT_BUG_ENTRY \
> + : : "i" (__FILE__), "i" (__LINE__), \
> + "i" (BUGFLAG_WARNING), \
> + "i" (sizeof(struct bug_entry))); \
> } while (0)
>
> #define WARN_ON(x) ({ \
> @@ -77,13 +79,12 @@ struct bug_entry *find_bug(unsigned long
> __WARN(); \
> } else { \
> __asm__ __volatile__( \
> - "1: "PPC_TLNEI" %0,0\n" \
> - ".section __bug_table,\"a\"\n" \
> - "\t"PPC_LONG" 1b,%1,%2,%3\n" \
> - ".previous" \
> - : : "r" (__ret_warn_on), \
> - "i" (__LINE__ + BUG_WARNING_TRAP), \
> - "i" (__FILE__), "i" (__FUNCTION__)); \
> + "1: "PPC_TLNEI" %4,0\n" \
> + _EMIT_BUG_ENTRY \
> + : : "i" (__FILE__), "i" (__LINE__), \
> + "i" (BUGFLAG_WARNING), \
> + "i" (sizeof(struct bug_entry)), \
> + "r" (__ret_warn_on)); \
> } \
> unlikely(__ret_warn_on); \
> })
> diff -puN include/asm-powerpc/module.h~generic-bug-for-powerpc include/asm-powerpc/module.h
> --- a/include/asm-powerpc/module.h~generic-bug-for-powerpc
> +++ a/include/asm-powerpc/module.h
> @@ -46,8 +46,6 @@ struct mod_arch_specific {
> unsigned int num_bugs;
> };
>
> -extern struct bug_entry *module_find_bug(unsigned long bugaddr);
> -
> /*
> * Select ELF headers.
> * Make empty section for module_frob_arch_sections to expand.
> _
>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists