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

From: Andrew Morton <[email-masked]>
Date: Sun, 13 Jul 2008 11:22:30 -0700

On Sun, 13 Jul 2008 12:48:15 +0200 Krzysztof Halasa <[email-masked]> wrote:

> Andrew Morton <[email-masked]> writes:
> 
> > (cc's added)
> 
> (cc added) :-)
> 
> > I guess you're referring to this:
> > http://linux.derkeiler.com/Mailing-Lists/Kernel/2008-07/msg04754.html
> 
> Right.
> 
> >> PPP over ATM connection (Thomson/Alcatel
> >> Speedtouch). It doesn't seem to occur on the same IXP4xx with Ethernet
> >> or V.35 WAN, and it doesn't occur on i386 + the same Speedtouch ADSL.
> >> The kernel is basically unpatched, the only extra patch applied is the
> >> platform support (nothing magic).
> >> 
> >> Generally to trigger the panic one has to request a TCP data stream
> >> over that PPPoATM connection.
> 
> I see what's wrong now: that's the ARM fls() problem, the call in
> fls64() to be precise. It seems it was already discussed: the patch in
> http://lkml.org/lkml/2008/5/4/233 makes the problem disappear.

Ah.

> Perhaps it's time to fix it definitely?

I'd sugget something like this.  Can you test, please?

--- 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);
+}
+
 #define ffs(x) ({ unsigned long __t = (x); fls(__t & -__t); })
 #define __ffs(x) (ffs(x) - 1)
 #define ffz(x) __ffs( ~(x) )
_