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.thomp...@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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to