Support resetting WARN_ONCE for all architectures

PATCH In review Andi Kleen 3 participants 6 messages Oct 19, 2017 – Nov 8, 2017 Library code

lore

Share & tools

lore

Andi Kleen

author

2017-10-19 20:46 UTC #0 lore raw

PATCH 1/1 Support resetting WARN_ONCE for all architectures

From: Andi Kleen <[email protected]>

Some architectures store the WARN_ONCE state in the flags field of the bug_entry. Clear that one too when resetting once state through /sys/kernel/debug/clear_warn_once

Pointed out by Michael Ellerman

Improves the earlier patch that add clear_warn_once.

Cc [email protected]

Signed-off-by Andi Kleen <[email protected]>

---
 include/linux/bug.h |  5 +++++
 kernel/panic.c      |  1 +
 lib/bug.c           | 21 +++++++++++++++++++++
 3 files changed, 27 insertions(+)

include/linux/bug.h +5 -0

index 5d5554c874fd..b8666ee88086 100644
@@ -42,6 +42,8 @@ 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);
+
 #else	/* !CONFIG_GENERIC_BUG */
 
 static inline enum bug_trap_type report_bug(unsigned long bug_addr,

@@ -50,6 +52,9 @@ static inline enum bug_trap_type report_bug(unsigned long bug_addr,
 	return BUG_TRAP_TYPE_BUG;
 }
 
+
+static inline void generic_bug_clear_once(void) {}
+
 #endif	/* CONFIG_GENERIC_BUG */
 
 /*

kernel/panic.c +1 -0

index 49dbdb231a77..840a98fa4963 100644
@@ -595,6 +595,7 @@ EXPORT_SYMBOL(warn_slowpath_null);
 
 static int clear_warn_once_set(void *data, u64 val)
 {
+	generic_bug_clear_once();
 	memset(__start_once, 0, __end_once - __start_once);
 	return 0;
 }

lib/bug.c +21 -0

index a6a1137d06db..7cb2d41845f7 100644
@@ -195,3 +195,24 @@ enum bug_trap_type report_bug(unsigned long bugaddr, struct pt_regs *regs)
 
 	return BUG_TRAP_TYPE_BUG;
 }
+
+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_ONCE;
+}
+
+void generic_bug_clear_once(void)
+{
+	struct module *mod;
+
+	rcu_read_lock_sched();
+	list_for_each_entry_rcu(mod, &module_bug_list, bug_list)
+		clear_once_table(mod->bug_table,
+				 mod->bug_table + mod->num_bugs);
+	rcu_read_unlock_sched();
+
+	clear_once_table(__start___bug_table, __stop___bug_table);
+}

Signature · 1 line

2.13.6

Michael Ellerman

2017-11-03 09:38 UTC #1 lore raw

Hi Andi,

Thanks for making it work with the flag, but ...

Quoted 5 lines · Andi Kleen

lib/bug.c +0 -0

index a6a1137d06db..7cb2d41845f7 100644

lib/bug.c enum bug_trap_type report_bug(unsigned long bugaddr, struct pt_regs *regs) · line 204

Quoted 10 lines

lib/bug.c +7 -0

@@ -195,3 +195,24 @@ enum bug_trap_type report_bug(unsigned long bugaddr, struct pt_regs *regs)
 
 	return BUG_TRAP_TYPE_BUG;
 }
+
+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_ONCE;

Clearing BUGFLAG_ONCE removes the once-ness permanently. ie. it becomes a WARN().

You should be clearing BUGFLAG_DONE, which is the flag that says this WARN has already triggered.

cheers

support-resetting-warn_once.patch support-resetting-warn_once-checkpatch-fixes.patch support-resetting-warn_once-for-all-architectures.patch support-resetting-warn_once-for-all-architectures-v2.patch support-resetting-warn_once-for-all-architectures-v2-fix.patch support-resetting-warn_once-for-all-architectures-v2-fix-fix.patch support-resetting-warn_once-for-all-architectures-v2-fix-fix-fix.patch

I'll push all this at Stephen later today. Someone please tell me whether this code actually works!

Michael Ellerman

2017-11-06 00:18 UTC #3 lore raw

Quoted 21 lines · Andrew Morton

Quoted 20 lines · On Fri, 03 Nov 2017 20:38:03 +1100 Michael Ellerman <[email protected]> wrote:
Quoted 5 lines · Andi Kleen
lib/bug.c +0 -0
index a6a1137d06db..7cb2d41845f7 100644
lib/bug.c enum bug_trap_type report_bug(unsigned long bugaddr, struct pt_regs *regs) · line 204
Quoted 10 lines
lib/bug.c +7 -0
@@ -195,3 +195,24 @@ enum bug_trap_type report_bug(unsigned long bugaddr, struct pt_regs *regs)
 
 	return BUG_TRAP_TYPE_BUG;
 }
+
+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_ONCE;
Clearing BUGFLAG_ONCE removes the once-ness permanently. ie. it becomes a WARN().

Quoted 4 lines

Quoted 2 lines
You should be clearing BUGFLAG_DONE, which is the flag that says this WARN has already triggered.
This?

Yes.

Quoted 15 lines

lib/bug.c renamed from lib/bug.c~support-resetting-warn_once-for-all-architectures-v2-fix-fix-fix +1 -1
@@ -202,7 +202,7 @@ static void clear_once_table(struct bug_
 	struct bug_entry *bug;
 
 	for (bug = start; bug < end; bug++)
-		bug->flags &= ~BUGFLAG_ONCE;
+		bug->flags &= ~BUGFLAG_DONE;
 }
 
 void generic_bug_clear_once(void)
And this is getting a bit nutty:

Quoted 7 lines

support-resetting-warn_once.patch support-resetting-warn_once-checkpatch-fixes.patch support-resetting-warn_once-for-all-architectures.patch support-resetting-warn_once-for-all-architectures-v2.patch support-resetting-warn_once-for-all-architectures-v2-fix.patch support-resetting-warn_once-for-all-architectures-v2-fix-fix.patch support-resetting-warn_once-for-all-architectures-v2-fix-fix-fix.patch

Quite. They should probably all just be squashed together.

Quoted 2 lines

I'll push all this at Stephen later today. Someone please tell me whether this code actually works!

I've tested Andi's two patches with the above one-liner and that worked for me. I'll test again on linux-next once it's in there to confirm.

cheers

Andi Kleen

author

2017-11-06 21:36 UTC #4 lore raw

In reply to Andrew Morton

Quoted 11 lines

And this is getting a bit nutty:
support-resetting-warn_once.patch support-resetting-warn_once-checkpatch-fixes.patch support-resetting-warn_once-for-all-architectures.patch support-resetting-warn_once-for-all-architectures-v2.patch support-resetting-warn_once-for-all-architectures-v2-fix.patch support-resetting-warn_once-for-all-architectures-v2-fix-fix.patch support-resetting-warn_once-for-all-architectures-v2-fix-fix-fix.patch

Quoted 2 lines

I'll push all this at Stephen later today. Someone please tell me whether this code actually works!

Sorry for the trash! Yes this was harder than originally expected.

The original x86 patch was tested. I see Michael now tested the PPC version too. So everything should be good and hopefully don't need any more fixes.

Thanks,

-Andi

Michael Ellerman

Tested-by

2017-11-08 09:41 UTC #5 lore raw

Quoted 11 lines · Andi Kleen

Quoted 11 lines
And this is getting a bit nutty:
support-resetting-warn_once.patch support-resetting-warn_once-checkpatch-fixes.patch support-resetting-warn_once-for-all-architectures.patch support-resetting-warn_once-for-all-architectures-v2.patch support-resetting-warn_once-for-all-architectures-v2-fix.patch support-resetting-warn_once-for-all-architectures-v2-fix-fix.patch support-resetting-warn_once-for-all-architectures-v2-fix-fix-fix.patch

Quoted 8 lines

Quoted 2 lines
I'll push all this at Stephen later today. Someone please tell me whether this code actually works!
Sorry for the trash! Yes this was harder than originally expected.
The original x86 patch was tested. I see Michael now tested the PPC version too. So everything should be good and hopefully don't need any more fixes.

Yep, I tested next-20171107 and that works for me, if you like:

Tested-by Michael Ellerman <[email protected]>

cheers