Le 12/07/2023 à 15:45, Michael Ellerman a écrit : > From: Christophe Leroy <christophe.le...@csgroup.eu> > > This partly reverts commit 1e688dd2a3d6759d416616ff07afc4bb836c4213. > > That commit aimed at optimising the code around generation of > WARN_ON/BUG_ON but this leads to a lot of dead code erroneously > generated by GCC. > > That dead code becomes a problem when we start using objtool validation > because objtool will abort validation with a warning as soon as it > detects unreachable code. This is because unreachable code might > be the indication that objtool doesn't properly decode object text. > > text data bss dec hex filename > 9551585 3627834 224376 13403795 cc8693 vmlinux.before > 9535281 3628358 224376 13388015 cc48ef vmlinux.after > > Once this change is reverted, in a standard configuration (pmac32 + > function tracer) the text is reduced by 16k which is around 1.7% > > We already had problem with it when starting to use objtool on powerpc > as a replacement for recordmcount, see commit 93e3f45a2631 ("powerpc: > Fix __WARN_FLAGS() for use with Objtool") > > There is also a problem with at least GCC 12, on ppc64_defconfig + > CONFIG_CC_OPTIMIZE_FOR_SIZE=y + CONFIG_DEBUG_SECTION_MISMATCH=y : > > LD .tmp_vmlinux.kallsyms1 > powerpc64-linux-ld: net/ipv4/tcp_input.o:(__ex_table+0xc4): undefined > reference to `.L2136' > make[2]: *** [scripts/Makefile.vmlinux:36: vmlinux] Error 1 > make[1]: *** [/home/chleroy/linux-powerpc/Makefile:1238: vmlinux] Error 2 > > Taking into account that other problems are encountered with that > 'asm goto' in WARN_ON(), including build failures, keeping that > change is not worth it allthough it is primarily a compiler bug. > > Revert it for now. > > mpe: Retain EMIT_WARN_ENTRY as a synonym for EMIT_BUG_ENTRY to reduce > churn, as there are now nearly as many uses of EMIT_WARN_ENTRY as > EMIT_BUG_ENTRY.
In that case, should we keep __EMIT_BUG_ENTRY and also keep the check that makes sure nobody uses EMIT_BUG_ENTRY with BUGFLAG_WARNING ? > -.macro EMIT_BUG_ENTRY addr,file,line,flags > - .if \flags & 1 /* BUGFLAG_WARNING */ > - .err /* Use EMIT_WARN_ENTRY for warnings */ > - .endif > - __EMIT_BUG_ENTRY \addr,\file,\line,\flags > -.endm > > Signed-off-by: Christophe Leroy <christophe.le...@csgroup.eu> > Acked-by: Naveen N Rao <nav...@kernel.org> > Signed-off-by: Michael Ellerman <m...@ellerman.id.au> > Link: > https://msgid.link/a7d6d0c20deaccfcbc74c3149e782538461fd6fe.1689091394.git.christophe.le...@csgroup.eu > --- > arch/powerpc/include/asm/bug.h | 69 +++++++--------------------------- > arch/powerpc/kernel/traps.c | 9 +---- > 2 files changed, 15 insertions(+), 63 deletions(-) > > v5: Keep EMIT_WARN_ENTRY. > I'll plan to take this as a fix. > > diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h > index ef42adb44aa3..00c6b0b4ede4 100644 > --- a/arch/powerpc/include/asm/bug.h > +++ b/arch/powerpc/include/asm/bug.h > @@ -4,14 +4,13 @@ > #ifdef __KERNEL__ > > #include <asm/asm-compat.h> > -#include <asm/extable.h> > > #ifdef CONFIG_BUG > > #ifdef __ASSEMBLY__ > #include <asm/asm-offsets.h> > #ifdef CONFIG_DEBUG_BUGVERBOSE > -.macro __EMIT_BUG_ENTRY addr,file,line,flags > +.macro EMIT_BUG_ENTRY addr,file,line,flags > .section __bug_table,"aw" > 5001: .4byte \addr - . > .4byte 5002f - . > @@ -23,7 +22,7 @@ > .previous > .endm > #else > -.macro __EMIT_BUG_ENTRY addr,file,line,flags > +.macro EMIT_BUG_ENTRY addr,file,line,flags > .section __bug_table,"aw" > 5001: .4byte \addr - . > .short \flags > @@ -32,18 +31,6 @@ > .endm > #endif /* verbose */ > > -.macro EMIT_WARN_ENTRY addr,file,line,flags > - EX_TABLE(\addr,\addr+4) > - __EMIT_BUG_ENTRY \addr,\file,\line,\flags > -.endm > - > -.macro EMIT_BUG_ENTRY addr,file,line,flags > - .if \flags & 1 /* BUGFLAG_WARNING */ > - .err /* Use EMIT_WARN_ENTRY for warnings */ > - .endif > - __EMIT_BUG_ENTRY \addr,\file,\line,\flags > -.endm > - > #else /* !__ASSEMBLY__ */ > /* _EMIT_BUG_ENTRY expects args %0,%1,%2,%3 to be FILE, LINE, flags and > sizeof(struct bug_entry), respectively */ > @@ -73,16 +60,6 @@ > "i" (sizeof(struct bug_entry)), \ > ##__VA_ARGS__) > > -#define WARN_ENTRY(insn, flags, label, ...) \ > - asm_volatile_goto( \ > - "1: " insn "\n" \ > - EX_TABLE(1b, %l[label]) \ > - _EMIT_BUG_ENTRY \ > - : : "i" (__FILE__), "i" (__LINE__), \ > - "i" (flags), \ > - "i" (sizeof(struct bug_entry)), \ > - ##__VA_ARGS__ : : label) > - > /* > * BUG_ON() and WARN_ON() do their best to cooperate with compile-time > * optimisations. However depending on the complexity of the condition > @@ -95,16 +72,7 @@ > } while (0) > #define HAVE_ARCH_BUG > > -#define __WARN_FLAGS(flags) do { \ > - __label__ __label_warn_on; \ > - \ > - WARN_ENTRY("twi 31, 0, 0", BUGFLAG_WARNING | (flags), __label_warn_on); > \ > - barrier_before_unreachable(); \ > - __builtin_unreachable(); \ > - \ > -__label_warn_on: \ > - break; \ > -} while (0) > +#define __WARN_FLAGS(flags) BUG_ENTRY("twi 31, 0, 0", BUGFLAG_WARNING | > (flags)) > > #ifdef CONFIG_PPC64 > #define BUG_ON(x) do { \ > @@ -117,25 +85,15 @@ __label_warn_on: > \ > } while (0) > > #define WARN_ON(x) ({ \ > - bool __ret_warn_on = false; \ > - do { \ > - if (__builtin_constant_p((x))) { \ > - if (!(x)) \ > - break; \ > + int __ret_warn_on = !!(x); \ > + if (__builtin_constant_p(__ret_warn_on)) { \ > + if (__ret_warn_on) \ > __WARN(); \ > - __ret_warn_on = true; \ > - } else { \ > - __label__ __label_warn_on; \ > - \ > - WARN_ENTRY(PPC_TLNEI " %4, 0", \ > - BUGFLAG_WARNING | BUGFLAG_TAINT(TAINT_WARN), > \ > - __label_warn_on, \ > - "r" ((__force long)(x))); \ > - break; \ > -__label_warn_on: \ > - __ret_warn_on = true; \ > - } \ > - } while (0); \ > + } else { \ > + BUG_ENTRY(PPC_TLNEI " %4, 0", \ > + BUGFLAG_WARNING | BUGFLAG_TAINT(TAINT_WARN), \ > + "r" (__ret_warn_on)); \ > + } \ > unlikely(__ret_warn_on); \ > }) > > @@ -148,14 +106,13 @@ __label_warn_on: > \ > #ifdef __ASSEMBLY__ > .macro EMIT_BUG_ENTRY addr,file,line,flags > .endm > -.macro EMIT_WARN_ENTRY addr,file,line,flags > -.endm > #else /* !__ASSEMBLY__ */ > #define _EMIT_BUG_ENTRY > -#define _EMIT_WARN_ENTRY > #endif > #endif /* CONFIG_BUG */ > > +#define EMIT_WARN_ENTRY EMIT_BUG_ENTRY > + > #include <asm-generic/bug.h> > > #ifndef __ASSEMBLY__ > diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c > index e59ec6d32d37..7ef147e2a20d 100644 > --- a/arch/powerpc/kernel/traps.c > +++ b/arch/powerpc/kernel/traps.c > @@ -1508,13 +1508,8 @@ static void do_program_check(struct pt_regs *regs) > > if (!(regs->msr & MSR_PR) && /* not user-mode */ > report_bug(bugaddr, regs) == BUG_TRAP_TYPE_WARN) { > - const struct exception_table_entry *entry; > - > - entry = search_exception_tables(bugaddr); > - if (entry) { > - regs_set_return_ip(regs, extable_fixup(entry) + > regs->nip - bugaddr); > - return; > - } > + regs_add_return_ip(regs, 4); > + return; > } > > if (cpu_has_feature(CPU_FTR_DEXCR_NPHIE) && user_mode(regs)) {