From: Christophe Leroy <christophe.leroy@c-s.fr>
To: Benjamin Herrenschmidt <benh@kernel.crashing.org>,
Paul Mackerras <paulus@samba.org>,
Michael Ellerman <mpe@ellerman.id.au>,
segher@kernel.crashing.org
Cc: linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
Subject: [PATCH 3/3] powerpc: use __builtin_trap() in BUG/WARN macros.
Date: Mon, 19 Aug 2019 13:06:31 +0000 (UTC) [thread overview]
Message-ID: <20510ce03cc9463f1c9e743c1d93b939de501b53.1566219503.git.christophe.leroy@c-s.fr> (raw)
In-Reply-To: <a6781075192afe0c909ce7d091de7931183a5d93.1566219503.git.christophe.leroy@c-s.fr>
The below exemples of use of WARN_ON() show that the result
is sub-optimal in regard of the capabilities of powerpc.
void test_warn1(unsigned long long a)
{
WARN_ON(a);
}
void test_warn2(unsigned long a)
{
WARN_ON(a);
}
void test_warn3(unsigned long a, unsigned long b)
{
WARN_ON(a < b);
}
void test_warn4(unsigned long a, unsigned long b)
{
WARN_ON(!a);
}
void test_warn5(unsigned long a, unsigned long b)
{
WARN_ON(!a && b);
}
00000000 <test_warn1>:
0: 7c 64 23 78 or r4,r3,r4
4: 31 24 ff ff addic r9,r4,-1
8: 7c 89 21 10 subfe r4,r9,r4
c: 0f 04 00 00 twnei r4,0
10: 4e 80 00 20 blr
00000014 <test_warn2>:
14: 31 23 ff ff addic r9,r3,-1
18: 7c 69 19 10 subfe r3,r9,r3
1c: 0f 03 00 00 twnei r3,0
20: 4e 80 00 20 blr
00000024 <test_warn3>:
24: 7c 84 18 10 subfc r4,r4,r3
28: 7d 29 49 10 subfe r9,r9,r9
2c: 7d 29 00 d0 neg r9,r9
30: 0f 09 00 00 twnei r9,0
34: 4e 80 00 20 blr
00000038 <test_warn4>:
38: 7c 63 00 34 cntlzw r3,r3
3c: 54 63 d9 7e rlwinm r3,r3,27,5,31
40: 0f 03 00 00 twnei r3,0
44: 4e 80 00 20 blr
00000048 <test_warn5>:
48: 2f 83 00 00 cmpwi cr7,r3,0
4c: 39 20 00 00 li r9,0
50: 41 9e 00 0c beq cr7,5c <test_warn5+0x14>
54: 7c 84 00 34 cntlzw r4,r4
58: 54 89 d9 7e rlwinm r9,r4,27,5,31
5c: 0f 09 00 00 twnei r9,0
60: 4e 80 00 20 blr
RELOCATION RECORDS FOR [__bug_table]:
OFFSET TYPE VALUE
00000000 R_PPC_ADDR32 .text+0x0000000c
0000000c R_PPC_ADDR32 .text+0x0000001c
00000018 R_PPC_ADDR32 .text+0x00000030
00000018 R_PPC_ADDR32 .text+0x00000030
00000024 R_PPC_ADDR32 .text+0x00000040
00000030 R_PPC_ADDR32 .text+0x0000005c
Using __builtin_trap() instead of inline assembly of twnei/tdnei
provides a far better result:
00000000 <test_warn1>:
0: 7c 64 23 78 or r4,r3,r4
4: 0f 04 00 00 twnei r4,0
8: 4e 80 00 20 blr
0000000c <test_warn2>:
c: 0f 03 00 00 twnei r3,0
10: 4e 80 00 20 blr
00000014 <test_warn3>:
14: 7c 43 20 08 twllt r3,r4
18: 4e 80 00 20 blr
0000001c <test_warn4>:
1c: 0c 83 00 00 tweqi r3,0
20: 4e 80 00 20 blr
00000024 <test_warn5>:
24: 2f 83 00 00 cmpwi cr7,r3,0
28: 41 9e 00 08 beq cr7,30 <test_warn5+0xc>
2c: 0c 84 00 00 tweqi r4,0
30: 4e 80 00 20 blr
RELOCATION RECORDS FOR [__bug_table]:
OFFSET TYPE VALUE
00000000 R_PPC_ADDR32 .text+0x00000004
0000000c R_PPC_ADDR32 .text+0x0000000c
00000018 R_PPC_ADDR32 .text+0x00000014
00000024 R_PPC_ADDR32 .text+0x0000001c
00000030 R_PPC_ADDR32 .text+0x0000002c
Note that we keep using an assembly text using "twi 31, 0, 0" for
inconditional traps because GCC drops all code after
__builtin_trap() when the condition is always true at build time.
In addition, this patch also fixes bugs in the BUG_ON(x) macro
which unlike WARN_ON(x) uses (x) directly as the condition by
forcing it to long instead of using !!(x). This leads to
upper part of an unsigned long long being ignored on PPC32 and
may produce bugs on PPC64 if (x) is a smaller type like an int.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/include/asm/bug.h | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h
index dbf7da90f507..a229130ffcf9 100644
--- a/arch/powerpc/include/asm/bug.h
+++ b/arch/powerpc/include/asm/bug.h
@@ -44,14 +44,14 @@
#ifdef CONFIG_DEBUG_BUGVERBOSE
#define _EMIT_BUG_ENTRY \
".section __bug_table,\"aw\"\n" \
- "2:\t" PPC_LONG "1b, %0\n" \
+ "2:\t" PPC_LONG "1b - 4, %0\n" \
"\t.short %1, %2\n" \
".org 2b+%3\n" \
".previous\n"
#else
#define _EMIT_BUG_ENTRY \
".section __bug_table,\"aw\"\n" \
- "2:\t" PPC_LONG "1b\n" \
+ "2:\t" PPC_LONG "1b - 4\n" \
"\t.short %2\n" \
".org 2b+%3\n" \
".previous\n"
@@ -59,7 +59,8 @@
#define BUG_ENTRY(insn, flags, ...) \
__asm__ __volatile__( \
- "1: " insn "\n" \
+ insn "\n" \
+ "1:\n" \
_EMIT_BUG_ENTRY \
: : "i" (__FILE__), "i" (__LINE__), \
"i" (flags), \
@@ -82,7 +83,9 @@
if (x) \
BUG(); \
} else { \
- BUG_ENTRY(PPC_TLNEI " %4, 0", 0, "r" ((__force long)(x))); \
+ if (x) \
+ __builtin_trap(); \
+ BUG_ENTRY("", 0); \
} \
} while (0)
@@ -94,9 +97,9 @@
if (__ret_warn_on) \
__WARN_TAINT(TAINT_WARN); \
} else { \
- BUG_ENTRY(PPC_TLNEI " %4, 0", \
- BUGFLAG_WARNING | BUGFLAG_TAINT(TAINT_WARN), \
- "r" (__ret_warn_on)); \
+ if (__ret_warn_on) \
+ __builtin_trap(); \
+ BUG_ENTRY("", BUGFLAG_WARNING | BUGFLAG_TAINT(TAINT_WARN)); \
} \
unlikely(__ret_warn_on); \
})
--
2.13.3
next prev parent reply other threads:[~2019-08-19 13:06 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-19 13:06 [PATCH 1/3] powerpc: don't use __WARN() for WARN_ON() Christophe Leroy
2019-08-19 13:06 ` [PATCH 2/3] powerpc: refactoring BUG/WARN macros Christophe Leroy
2019-11-25 10:46 ` Michael Ellerman
2019-08-19 13:06 ` Christophe Leroy [this message]
2019-08-19 13:23 ` [PATCH 3/3] powerpc: use __builtin_trap() in " Segher Boessenkool
2019-08-19 14:08 ` Christophe Leroy
2019-08-19 14:37 ` Segher Boessenkool
2019-08-19 15:05 ` Christophe Leroy
2019-08-19 15:45 ` Segher Boessenkool
2019-08-23 15:35 ` Christophe Leroy
2019-08-19 16:28 ` [PATCH 1/3] powerpc: don't use __WARN() for WARN_ON() Kees Cook
2019-08-19 17:29 ` Clean up cut-here even harder (was Re: [PATCH 1/3] powerpc: don't use __WARN() for WARN_ON()) Kees Cook
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20510ce03cc9463f1c9e743c1d93b939de501b53.1566219503.git.christophe.leroy@c-s.fr \
--to=christophe.leroy@c-s.fr \
--cc=benh@kernel.crashing.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=mpe@ellerman.id.au \
--cc=paulus@samba.org \
--cc=segher@kernel.crashing.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).