Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1346723 > unrolled thread

[PATCH] bug: Set warn variable before calling WARN()

Back to article view | Back to linux.kernel

Contents

  [PATCH] bug: Set warn variable before calling WARN() Steven Rostedt <rostedt@goodmis.org> - 2016-03-01 17:10 +0100
    Re: [PATCH] bug: Set warn variable before calling WARN() Peter Zijlstra <peterz@infradead.org> - 2016-03-01 17:40 +0100
      Re: [PATCH] bug: Set warn variable before calling WARN() Steven Rostedt <rostedt@goodmis.org> - 2016-03-01 17:40 +0100

#1346723 — [PATCH] bug: Set warn variable before calling WARN()

This has hit me a couple of times already. I would be debugging code
and the system would simply hang and then reboot. Finally, I found that
the problem was caused by WARN_ON_ONCE() and friends.

The macro WARN_ON_ONCE(condition) is defined as:

	static bool __section(.data.unlikely) __warned;
	int __ret_warn_once = !!(condition);

	if (unlikely(__ret_warn_once))
		if (WARN_ON(!__warned))
			__warned = true;

	unlikely(__ret_warn_once);

Which looks great and all. But what I have hit, is an issue when
WARN_ON() itself hits the same WARN_ON_ONCE() code. Because, the
variable __warned is not yet set. Then it too calls WARN_ON() and that
triggers the warning again. It keeps doing this until the stack is
overflowed and the system crashes.

By setting __warned first before calling WARN_ON() makes the original
WARN_ON_ONCE() really only warn once, and not an infinite amount of
times if the WARN_ON() also triggers the warning.

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index 630dd2372238..fdf6fa078422 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -110,9 +110,10 @@ extern void warn_slowpath_null(const char *file, const int line);
 	static bool __section(.data.unlikely) __warned;		\
 	int __ret_warn_once = !!(condition);			\
 								\
-	if (unlikely(__ret_warn_once))				\
-		if (WARN_ON(!__warned)) 			\
-			__warned = true;			\
+	if (unlikely(__ret_warn_once && !__warned)) {		\
+		__warned = true;				\
+		WARN_ON(1);					\
+	}							\
 	unlikely(__ret_warn_once);				\
 })
 
@@ -120,9 +121,10 @@ extern void warn_slowpath_null(const char *file, const int line);
 	static bool __section(.data.unlikely) __warned;		\
 	int __ret_warn_once = !!(condition);			\
 								\
-	if (unlikely(__ret_warn_once))				\
-		if (WARN(!__warned, format)) 			\
-			__warned = true;			\
+	if (unlikely(__ret_warn_once && !__warned)) {		\
+		__warned = true;				\
+		WARN(1, format);				\
+	}							\
 	unlikely(__ret_warn_once);				\
 })
 
@@ -130,9 +132,10 @@ extern void warn_slowpath_null(const char *file, const int line);
 	static bool __section(.data.unlikely) __warned;		\
 	int __ret_warn_once = !!(condition);			\
 								\
-	if (unlikely(__ret_warn_once))				\
-		if (WARN_TAINT(!__warned, taint, format))	\
-			__warned = true;			\
+	if (unlikely(__ret_warn_once && !__warned)) {		\
+		__warned = true;				\
+		WARN_TAINT(1, taint, format);			\
+	}							\
 	unlikely(__ret_warn_once);				\
 })
 

[toc] | [next] | [standalone]

#1346735

On Tue, Mar 01, 2016 at 11:09:39AM -0500, Steven Rostedt wrote:
> This has hit me a couple of times already. I would be debugging code
> and the system would simply hang and then reboot. Finally, I found that
> the problem was caused by WARN_ON_ONCE() and friends.
> 
> The macro WARN_ON_ONCE(condition) is defined as:
> 
> 	static bool __section(.data.unlikely) __warned;
> 	int __ret_warn_once = !!(condition);
> 
> 	if (unlikely(__ret_warn_once))
> 		if (WARN_ON(!__warned))
> 			__warned = true;
> 
> 	unlikely(__ret_warn_once);
> 
> Which looks great and all. But what I have hit, is an issue when
> WARN_ON() itself hits the same WARN_ON_ONCE() code. Because, the
> variable __warned is not yet set. Then it too calls WARN_ON() and that
> triggers the warning again. It keeps doing this until the stack is
> overflowed and the system crashes.
> 
> By setting __warned first before calling WARN_ON() makes the original
> WARN_ON_ONCE() really only warn once, and not an infinite amount of
> times if the WARN_ON() also triggers the warning.
> 
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>

[toc] | [prev] | [next] | [standalone]

#1346742

On Tue, 1 Mar 2016 17:35:31 +0100
Peter Zijlstra <peterz@infradead.org> wrote:


> > Signed-off-by: Steven Rostedt <rostedt@goodmis.org>  
> 
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>

Andrew, can you take this in your tree?

-- Steve

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web