[RFC PATCH 2/2] powerpc: nop trap instruction after WARN_ONCE fires
Nicholas Piggin
npiggin at gmail.com
Sat Sep 24 01:41:43 AEST 2022
WARN_ONCE and similar are often used in frequently executed code, and
should not crash the system. The program check interrupt caused by
WARN_ON_ONCE can be a significant overhead even when nothing is being
printed. This can cause performance to become unacceptable, having the
same effective impact to the user as a BUG_ON().
Avoid this overhead by patching the trap with a nop instruction after a
"once" trap fires. Conditional warnings that return a result must have
equivalent compare and branch instructions after the trap, so when it is
nopped the statement will behave the same way. It's possible the asm
goto should be removed entirely and this comparison just done in C now.
XXX: possibly this should schedule the patching to run in a different
context than the program check.
XXX: patching process could allocate memory for patched bugs rather
than keeping a word in the bug entry for all of them. Very few will
ever fire.
XXX: is concurrency okay?
XXX: could avoid clobbering cc in some cases, possibly optimise some
variants.
---
arch/powerpc/include/asm/bug.h | 14 ++++++++++--
arch/powerpc/kernel/traps.c | 42 +++++++++++++++++++++++++++++++++-
include/asm-generic/bug.h | 9 ++++++++
include/linux/bug.h | 1 +
lib/bug.c | 22 +++++++++---------
5 files changed, 74 insertions(+), 14 deletions(-)
diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h
index dec73137fea0..0ea7b9fe0305 100644
--- a/arch/powerpc/include/asm/bug.h
+++ b/arch/powerpc/include/asm/bug.h
@@ -14,6 +14,7 @@
.macro __EMIT_BUG_ENTRY addr,file,line,flags
.section __bug_table,"aw"
5001: .4byte \addr - .
+ .4byte 0
.4byte 0
.4byte 5002f - .
.short \line, \flags
@@ -27,6 +28,7 @@
.macro __EMIT_BUG_ENTRY addr,file,line,flags
.section __bug_table,"aw"
5001: .4byte \addr - .
+ .4byte 0
.4byte 0
.short \flags
.org 5001b+BUG_ENTRY_SIZE
@@ -48,6 +50,7 @@
#else /* !__ASSEMBLY__ */
struct arch_bug_entry {
signed int recovery_addr_disp;
+ unsigned int insn;
};
/* _EMIT_BUG_ENTRY expects args %0,%1,%2,%3 to be FILE, LINE, flags and
@@ -57,6 +60,7 @@ struct arch_bug_entry {
".section __bug_table,\"aw\"\n" \
"2: .4byte 1b - .\n" \
" .4byte 0\n" \
+ " .4byte 0\n" \
" .4byte %0 - .\n" \
" .short %1, %2\n" \
".org 2b+%3\n" \
@@ -66,6 +70,7 @@ struct arch_bug_entry {
".section __bug_table,\"aw\"\n" \
"2: .4byte 1b - .\n" \
" .4byte 0\n" \
+ " .4byte 0\n" \
" .short %2\n" \
".org 2b+%3\n" \
".previous\n"
@@ -76,6 +81,7 @@ struct arch_bug_entry {
".section __bug_table,\"aw\"\n" \
"2: .4byte 1b - .\n" \
" .4byte 1b - %l[" #label "]\n" \
+ " .4byte 0\n" \
" .4byte %0 - .\n" \
" .short %1, %2\n" \
".org 2b+%3\n" \
@@ -85,6 +91,7 @@ struct arch_bug_entry {
".section __bug_table,\"aw\"\n" \
"2: .4byte 1b - .\n" \
" .4byte 1b - %l[" #label "]\n" \
+ " .4byte 0\n" \
" .short %2\n" \
".org 2b+%3\n" \
".previous\n"
@@ -106,7 +113,7 @@ struct arch_bug_entry {
: : "i" (__FILE__), "i" (__LINE__), \
"i" (flags), \
"i" (sizeof(struct bug_entry)), \
- ##__VA_ARGS__ : : label)
+ ##__VA_ARGS__ : "cr0" : label)
/*
* BUG_ON() and WARN_ON() do their best to cooperate with compile-time
@@ -151,7 +158,7 @@ __label_warn_on: \
} else { \
__label__ __label_warn_on; \
\
- WARN_ENTRY(PPC_TLNEI " %4, 0", \
+ WARN_ENTRY(PPC_TLNEI " %4, 0 ; cmpdi %4, 0 ; bne %l[__label_warn_on]", \
BUGFLAG_WARNING | BUGFLAG_TAINT(TAINT_WARN), \
__label_warn_on, \
"r" ((__force long)(x))); \
@@ -178,6 +185,9 @@ __label_warn_on: \
#define _EMIT_BUG_ENTRY
#define _EMIT_WARN_ENTRY
#endif
+#define arch_generic_bug_entry_clear_once
+void arch_generic_bug_entry_clear_once(struct bug_entry *bug);
+
#endif /* CONFIG_BUG */
#include <asm-generic/bug.h>
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index 1e521a432bd0..14137c17b9cd 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -1458,6 +1458,44 @@ static int emulate_math(struct pt_regs *regs)
static inline int emulate_math(struct pt_regs *regs) { return -1; }
#endif
+static DEFINE_MUTEX(bug_lock);
+
+void arch_generic_bug_entry_clear_once(struct bug_entry *bug)
+{
+ unsigned long bugaddr = bug_addr(bug);
+
+ BUG_ON(!bug->arch.insn);
+
+ mutex_lock(&bug_lock);
+ if (bug->arch.insn == 0) {
+ mutex_unlock(&bug_lock);
+ return;
+ }
+ patch_instruction((u32 *)bugaddr, ppc_inst(bug->arch.insn));
+ bug->arch.insn = 0;
+ mutex_unlock(&bug_lock);
+}
+
+static void bug_entry_done_once(struct bug_entry *bug)
+{
+ unsigned long bugaddr = bug_addr(bug);
+ unsigned int insn;
+
+ if (!mutex_trylock(&bug_lock))
+ return;
+
+ if (bug->arch.insn != 0)
+ goto out;
+
+ if (__get_user(insn, (unsigned int __user *)bugaddr))
+ goto out;
+
+ patch_instruction((u32 *)bugaddr, ppc_inst(PPC_RAW_NOP()));
+
+out:
+ mutex_unlock(&bug_lock);
+}
+
static void do_program_check(struct pt_regs *regs)
{
unsigned int reason = get_reason(regs);
@@ -1494,7 +1532,7 @@ 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 bug_entry *bug;
+ struct bug_entry *bug;
unsigned long recov;
bug = find_bug(bugaddr);
@@ -1502,6 +1540,8 @@ static void do_program_check(struct pt_regs *regs)
recov = regs->nip + 4;
else
recov = bugaddr - bug->arch.recovery_addr_disp;
+ if (bug && (bug->flags & BUGFLAG_ONCE))
+ bug_entry_done_once(bug);
regs_set_return_ip(regs, recov);
return;
diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index 1ce8431bdf02..a0cb717998c1 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -49,6 +49,15 @@ struct bug_entry {
#endif
unsigned short flags;
};
+
+static inline unsigned long bug_addr(const struct bug_entry *bug)
+{
+#ifdef CONFIG_GENERIC_BUG_RELATIVE_POINTERS
+ return (unsigned long)&bug->bug_addr_disp + bug->bug_addr_disp;
+#else
+ return bug->bug_addr;
+#endif
+}
#endif /* CONFIG_GENERIC_BUG */
/*
diff --git a/include/linux/bug.h b/include/linux/bug.h
index 348acf2558f3..a207445e5b5f 100644
--- a/include/linux/bug.h
+++ b/include/linux/bug.h
@@ -46,6 +46,7 @@ enum bug_trap_type report_bug(unsigned long bug_addr, struct pt_regs *regs);
/* These are defined by the architecture */
int is_valid_bugaddr(unsigned long addr);
+void __generic_bug_clear_once(void);
void generic_bug_clear_once(void);
#else /* !CONFIG_GENERIC_BUG */
diff --git a/lib/bug.c b/lib/bug.c
index c223a2575b72..d1cc4f763679 100644
--- a/lib/bug.c
+++ b/lib/bug.c
@@ -50,15 +50,6 @@
extern struct bug_entry __start___bug_table[], __stop___bug_table[];
-static inline unsigned long bug_addr(const struct bug_entry *bug)
-{
-#ifdef CONFIG_GENERIC_BUG_RELATIVE_POINTERS
- return (unsigned long)&bug->bug_addr_disp + bug->bug_addr_disp;
-#else
- return bug->bug_addr;
-#endif
-}
-
#ifdef CONFIG_MODULES
/* Updates are protected by module mutex */
static LIST_HEAD(module_bug_list);
@@ -209,12 +200,21 @@ enum bug_trap_type report_bug(unsigned long bugaddr, struct pt_regs *regs)
return BUG_TRAP_TYPE_BUG;
}
+#ifndef arch_generic_bug_entry_clear_once
+#define arch_generic_bug_entry_clear_once(bug)
+#endif
+
static void clear_once_table(struct bug_entry *start, struct bug_entry *end)
{
struct bug_entry *bug;
- for (bug = start; bug < end; bug++)
- bug->flags &= ~BUGFLAG_DONE;
+ for (bug = start; bug < end; bug++) {
+ bool triggered = bug->flags & BUGFLAG_DONE;
+ if (triggered) {
+ bug->flags &= ~BUGFLAG_DONE;
+ arch_generic_bug_entry_clear_once(bug);
+ }
+ }
}
void generic_bug_clear_once(void)
--
2.37.2
More information about the Linuxppc-dev
mailing list