* [PATCH] bug: Recursion avoidance for WARN_ONCE() and friends
@ 2015-03-18 16:51 Daniel Thompson
2015-04-22 12:54 ` [RESEND PATCH] " Daniel Thompson
0 siblings, 1 reply; 2+ messages in thread
From: Daniel Thompson @ 2015-03-18 16:51 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Daniel Thompson, linux-arch, linux-kernel, patches, linaro-kernel,
John Stultz, Sumit Semwal
Currently WARN_ONCE() and similar macros set __warned *after* calling
the underlying macro. This risks infinite recursion if WARN_ONCE() is
used to implement sanity tests in any code that can be called by printk.
This can be fixed by restructuring the macros to set __warned before
calling further macros.
Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
---
Notes:
I discovered this problem when I temporarily added sanity tests to the
irqflags macros during some of my development work but I suspect the
scope is a little wider. I admit I was tempted to throw this change
away after I had finished debugging but for two things prompted me to
post it.
1. It did cost me a few minutes head scratching and I'd like to spare
others the pain.
2. I realized the new code is potentially (and very fractionally) more
efficient: the register containing address of __warned can be reused
and a cache hit is a near certainty for the write.
Don't get too excited about the efficiency gains though they are
extremely modest. Measures as code size benefit and using v4.0-rc4 the
results are:
Kernel GCC version Code size reduction
arm multi_v7_defconfig Linaro 4.8-2014.01 224 bytes
arm64 defconfig Linaro 4.9-2014.09 32 bytes
i386_defconfig Redhat 4.9.2-6 62 bytes
x86_64_defconfig Redhat 4.9.2-6 380 bytes
include/asm-generic/bug.h | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index 630dd2372238..f8c8a819c563 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(true); \
+ } \
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(true, 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(true, taint, format); \
+ } \
unlikely(__ret_warn_once); \
})
--
2.1.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* [RESEND PATCH] bug: Recursion avoidance for WARN_ONCE() and friends
2015-03-18 16:51 [PATCH] bug: Recursion avoidance for WARN_ONCE() and friends Daniel Thompson
@ 2015-04-22 12:54 ` Daniel Thompson
0 siblings, 0 replies; 2+ messages in thread
From: Daniel Thompson @ 2015-04-22 12:54 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Daniel Thompson, linux-arch, linux-kernel, patches, linaro-kernel,
John Stultz, Sumit Semwal
Currently WARN_ONCE() and similar macros set __warned *after* calling
the underlying macro. This risks infinite recursion if WARN_ONCE() is
used to implement sanity tests in any code that can be called by printk.
This can be fixed by restructuring the macros to set __warned before
calling further macros.
Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
---
Notes:
I discovered this problem when I temporarily added sanity tests to the
irqflags macros during some of my development work but I suspect the
scope is a little wider. I admit I was tempted to throw this change
away after I had finished debugging but for two things prompted me to
post it.
1. It did cost me a few minutes head scratching and I'd like to spare
others the pain.
2. I realized the new code is potentially (and very fractionally) more
efficient: the register containing address of __warned can be reused
and a cache hit is a near certainty for the write.
Don't get too excited about the efficiency gains though they are
extremely modest. Measures as code size benefit and using v4.0-rc4 the
results are:
Kernel GCC version Code size reduction
arm multi_v7_defconfig Linaro 4.8-2014.01 224 bytes
arm64 defconfig Linaro 4.9-2014.09 32 bytes
i386_defconfig Redhat 4.9.2-6 62 bytes
x86_64_defconfig Redhat 4.9.2-6 380 bytes
include/asm-generic/bug.h | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index 630dd2372238..f8c8a819c563 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(true); \
+ } \
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(true, 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(true, taint, format); \
+ } \
unlikely(__ret_warn_once); \
})
--
2.1.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2015-04-22 12:54 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-03-18 16:51 [PATCH] bug: Recursion avoidance for WARN_ONCE() and friends Daniel Thompson
2015-04-22 12:54 ` [RESEND PATCH] " Daniel Thompson
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).