Re: [bisected] kernel panic 2.6.22 -> 2.6.26-rc9+

5 messages, 3 authors, 2008-07-23 · open the first message on its own page

Re: [bisected] kernel panic 2.6.22 -> 2.6.26-rc9+

From: Krzysztof Halasa <khc@pm.waw.pl>
Date: 2008-07-13 21:51:21

Andrew Morton [off-list ref] writes:
quoted hunk
--- a/include/asm-arm/bitops.h~a
+++ a/include/asm-arm/bitops.h
@@ -277,9 +277,16 @@ static inline int constant_fls(int x)
  * the clz instruction for much better code efficiency.
  */
 
-#define fls(x) \
+#define __fls(x) \
 	( __builtin_constant_p(x) ? constant_fls(x) : \
 	  ({ int __r; asm("clz\t%0, %1" : "=r"(__r) : "r"(x) : "cc"); 32-__r; }) )
+
+/* Implement fls() in C so that 64-bit args are suitably truncated */
+static inline int fls(int x)
+{
+	return __fls(x);
+}
+
Well, I like it more as it fixes all possible places instead of only
fls64().

But... can't we just move the #define body into the inline fls(x)?
Will there be other users of __fls(x)? It seems the
__builtin_constant_p(x) works for inline functions.

The above patch fixes the kernel panic, too.
-- 
Krzysztof Halasa

Re: [bisected] kernel panic 2.6.22 -> 2.6.26-rc9+

From: Andrew Morton <akpm@linux-foundation.org>
Date: 2008-07-13 21:56:20

On Sun, 13 Jul 2008 23:51:06 +0200 Krzysztof Halasa [off-list ref] wrote:
Andrew Morton [off-list ref] writes:
quoted
--- a/include/asm-arm/bitops.h~a
+++ a/include/asm-arm/bitops.h
@@ -277,9 +277,16 @@ static inline int constant_fls(int x)
  * the clz instruction for much better code efficiency.
  */
 
-#define fls(x) \
+#define __fls(x) \
 	( __builtin_constant_p(x) ? constant_fls(x) : \
 	  ({ int __r; asm("clz\t%0, %1" : "=r"(__r) : "r"(x) : "cc"); 32-__r; }) )
+
+/* Implement fls() in C so that 64-bit args are suitably truncated */
+static inline int fls(int x)
+{
+	return __fls(x);
+}
+
Well, I like it more as it fixes all possible places instead of only
fls64().

But... can't we just move the #define body into the inline fls(x)?
Will there be other users of __fls(x)? It seems the
__builtin_constant_p(x) works for inline functions.
Could.  That was a minimal&safe thing.
The above patch fixes the kernel panic, too.
OK, thanks.

Re: [bisected] kernel panic 2.6.22 -> 2.6.26-rc9+

From: Willy Tarreau <w@1wt.eu>
Date: 2008-07-23 04:53:09

On Sun, Jul 13, 2008 at 02:55:25PM -0700, Andrew Morton wrote:
On Sun, 13 Jul 2008 23:51:06 +0200 Krzysztof Halasa [off-list ref] wrote:
quoted
Andrew Morton [off-list ref] writes:
quoted
--- a/include/asm-arm/bitops.h~a
+++ a/include/asm-arm/bitops.h
@@ -277,9 +277,16 @@ static inline int constant_fls(int x)
  * the clz instruction for much better code efficiency.
  */
 
-#define fls(x) \
+#define __fls(x) \
 	( __builtin_constant_p(x) ? constant_fls(x) : \
 	  ({ int __r; asm("clz\t%0, %1" : "=r"(__r) : "r"(x) : "cc"); 32-__r; }) )
+
+/* Implement fls() in C so that 64-bit args are suitably truncated */
+static inline int fls(int x)
+{
+	return __fls(x);
+}
+
Well, I like it more as it fixes all possible places instead of only
fls64().

But... can't we just move the #define body into the inline fls(x)?
Will there be other users of __fls(x)? It seems the
__builtin_constant_p(x) works for inline functions.
Could.  That was a minimal&safe thing.
quoted
The above patch fixes the kernel panic, too.
OK, thanks.
Andrew,

have you sent your patch to Linus ? I haven't seen it merged yet, and
I'd like to get it into -stable too.

Thanks,
Willy

Re: [bisected] kernel panic 2.6.22 -> 2.6.26-rc9+

From: Andrew Morton <akpm@linux-foundation.org>
Date: 2008-07-23 05:02:37

On Wed, 23 Jul 2008 06:52:31 +0200 Willy Tarreau [off-list ref] wrote:
On Sun, Jul 13, 2008 at 02:55:25PM -0700, Andrew Morton wrote:
quoted
On Sun, 13 Jul 2008 23:51:06 +0200 Krzysztof Halasa [off-list ref] wrote:
quoted
Andrew Morton [off-list ref] writes:
quoted
--- a/include/asm-arm/bitops.h~a
+++ a/include/asm-arm/bitops.h
@@ -277,9 +277,16 @@ static inline int constant_fls(int x)
  * the clz instruction for much better code efficiency.
  */
 
-#define fls(x) \
+#define __fls(x) \
 	( __builtin_constant_p(x) ? constant_fls(x) : \
 	  ({ int __r; asm("clz\t%0, %1" : "=r"(__r) : "r"(x) : "cc"); 32-__r; }) )
+
+/* Implement fls() in C so that 64-bit args are suitably truncated */
+static inline int fls(int x)
+{
+	return __fls(x);
+}
+
Well, I like it more as it fixes all possible places instead of only
fls64().

But... can't we just move the #define body into the inline fls(x)?
Will there be other users of __fls(x)? It seems the
__builtin_constant_p(x) works for inline functions.
Could.  That was a minimal&safe thing.
quoted
The above patch fixes the kernel panic, too.
OK, thanks.
Andrew,

have you sent your patch to Linus ? I haven't seen it merged yet, and
I'd like to get it into -stable too.
This one?

From: Andrew Morton <akpm@linux-foundation.org>

arm's fls() is implemented as a macro, causing it to misbehave when passed
64-bit arguments.  Fix.

Cc: Nickolay Vinogradov <redacted>
Tested-by: Krzysztof Halasa <khc@pm.waw.pl>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 include/asm-arm/bitops.h |    9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff -puN include/asm-arm/bitops.h~arm-fix-fls-for-64-bit-arguments include/asm-arm/bitops.h
--- a/include/asm-arm/bitops.h~arm-fix-fls-for-64-bit-arguments
+++ a/include/asm-arm/bitops.h
@@ -277,9 +277,16 @@ static inline int constant_fls(int x)
  * the clz instruction for much better code efficiency.
  */
 
-#define fls(x) \
+#define __fls(x) \
 	( __builtin_constant_p(x) ? constant_fls(x) : \
 	  ({ int __r; asm("clz\t%0, %1" : "=r"(__r) : "r"(x) : "cc"); 32-__r; }) )
+
+/* Implement fls() in C so that 64-bit args are suitably truncated */
+static inline int fls(int x)
+{
+	return __fls(x);
+}
+
 #define ffs(x) ({ unsigned long __t = (x); fls(__t & -__t); })
 #define __ffs(x) (ffs(x) - 1)
 #define ffz(x) __ffs( ~(x) )
_


Nope, I'm just kind of sitting on it.  I'll put a stable@kernel.org tag
on it and send it to Russell I guess.

Re: [bisected] kernel panic 2.6.22 -> 2.6.26-rc9+

From: Willy Tarreau <w@1wt.eu>
Date: 2008-07-23 05:06:52

On Tue, Jul 22, 2008 at 10:00:56PM -0700, Andrew Morton wrote:
On Wed, 23 Jul 2008 06:52:31 +0200 Willy Tarreau [off-list ref] wrote:
quoted
On Sun, Jul 13, 2008 at 02:55:25PM -0700, Andrew Morton wrote:
quoted
On Sun, 13 Jul 2008 23:51:06 +0200 Krzysztof Halasa [off-list ref] wrote:
quoted
Andrew Morton [off-list ref] writes:
quoted
--- a/include/asm-arm/bitops.h~a
+++ a/include/asm-arm/bitops.h
@@ -277,9 +277,16 @@ static inline int constant_fls(int x)
  * the clz instruction for much better code efficiency.
  */
 
-#define fls(x) \
+#define __fls(x) \
 	( __builtin_constant_p(x) ? constant_fls(x) : \
 	  ({ int __r; asm("clz\t%0, %1" : "=r"(__r) : "r"(x) : "cc"); 32-__r; }) )
+
+/* Implement fls() in C so that 64-bit args are suitably truncated */
+static inline int fls(int x)
+{
+	return __fls(x);
+}
+
Well, I like it more as it fixes all possible places instead of only
fls64().

But... can't we just move the #define body into the inline fls(x)?
Will there be other users of __fls(x)? It seems the
__builtin_constant_p(x) works for inline functions.
Could.  That was a minimal&safe thing.
quoted
The above patch fixes the kernel panic, too.
OK, thanks.
Andrew,

41 more quoted lines

yes, precisely this one.
Nope, I'm just kind of sitting on it.  I'll put a stable@kernel.org tag
on it and send it to Russell I guess.
Perfect, thanks!
Willy
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help