Re: [PATCH v2 2/2] powerpc/bug: Provide better flexibility to WARN_ON/__WARN_FLAGS() with asm goto

From: Christophe Leroy
Date: Thu Aug 26 2021 - 10:53:06 EST




Le 26/08/2021 à 16:45, Michael Ellerman a écrit :
Christophe Leroy <christophe.leroy@xxxxxxxxxx> writes:
Le 26/08/2021 à 05:21, Michael Ellerman a écrit :
Nathan Chancellor <nathan@xxxxxxxxxx> 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.

Yes looks nice, we already had that kind of stuff in the past, even more ugly.


Although this doesn't fix the strange warning in drivers/net/ethernet/sfc.

What's the warning ?



So possibly we need a CLANG ifdef around the whole thing, and use the
old style warn for clang.


Christophe