Messages in this thread Patch in this message | | | From | Michael Ellerman <> | Subject | Re: [PATCH v2 2/2] powerpc/bug: Provide better flexibility to WARN_ON/__WARN_FLAGS() with asm goto | Date | Fri, 27 Aug 2021 00:45:06 +1000 |
| |
Christophe Leroy <christophe.leroy@csgroup.eu> writes: > Le 26/08/2021 à 05:21, Michael Ellerman a écrit : >> Nathan Chancellor <nathan@kernel.org> writes: >>> On Tue, Apr 13, 2021 at 04:38:10PM +0000, Christophe Leroy wrote: >>>> Using asm goto in __WARN_FLAGS() and WARN_ON() allows more >>>> flexibility to GCC. >> ... >>> >>> This patch as commit 1e688dd2a3d6 ("powerpc/bug: Provide better >>> flexibility to WARN_ON/__WARN_FLAGS() with asm goto") cause a WARN_ON in >>> klist_add_tail to trigger over and over on boot when compiling with >>> clang: > > ... > >> >> This patch seems to fix it. Not sure if that's just papering over it though. >> >> diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h >> index 1ee0f22313ee..75fcb4370d96 100644 >> --- a/arch/powerpc/include/asm/bug.h >> +++ b/arch/powerpc/include/asm/bug.h >> @@ -119,7 +119,7 @@ __label_warn_on: \ >> \ >> WARN_ENTRY(PPC_TLNEI " %4, 0", \ >> BUGFLAG_WARNING | BUGFLAG_TAINT(TAINT_WARN), \ >> - __label_warn_on, "r" (x)); \ >> + __label_warn_on, "r" (!!(x))); \ >> break; \ >> __label_warn_on: \ >> __ret_warn_on = true; \ > > But for a simple WARN_ON() call: > > void test(unsigned long b) > { > WARN_ON(b); > } > > Without your change with GCC you get: > > 00000000000012d0 <.test>: > 12d0: 0b 03 00 00 tdnei r3,0 > 12d4: 4e 80 00 20 blr > > > With the !! change you get: > > 00000000000012d0 <.test>: > 12d0: 31 23 ff ff addic r9,r3,-1 > 12d4: 7d 29 19 10 subfe r9,r9,r3 > 12d8: 0b 09 00 00 tdnei r9,0 > 12dc: 4e 80 00 20 blr
Yeah that's a pity.
We could do something like below, which is ugly, but would be better than having to revert the whole thing.
Although this doesn't fix the strange warning in drivers/net/ethernet/sfc.
So possibly we need a CLANG ifdef around the whole thing, and use the old style warn for clang.
cheers
diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h index 1ee0f22313ee..d978d9004d0d 100644 --- a/arch/powerpc/include/asm/bug.h +++ b/arch/powerpc/include/asm/bug.h @@ -106,6 +106,12 @@ __label_warn_on: \ } \ } while (0) +#ifdef CONFIG_CC_IS_CLANG +#define __clang_warn_hack(x) (!!(x)) +#else +#define __clang_warn_hack(x) (x) +#endif + #define WARN_ON(x) ({ \ bool __ret_warn_on = false; \ do { \ @@ -119,7 +125,8 @@ __label_warn_on: \ \ WARN_ENTRY(PPC_TLNEI " %4, 0", \ BUGFLAG_WARNING | BUGFLAG_TAINT(TAINT_WARN), \ - __label_warn_on, "r" (x)); \ + __label_warn_on, \ + "r" __clang_warn_hack(x)); \ break; \ __label_warn_on: \ __ret_warn_on = true; \
| |