[bisected] kernel panic 2.6.22 -> 2.6.26-rc9+ - Kernel

This is a discussion on [bisected] kernel panic 2.6.22 -> 2.6.26-rc9+ - Kernel ; Hi, Bisected the panic. It seems it occurs on my ARM IXP4xx big-endian system with USB ADSL 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 ...

+ Reply to Thread
Results 1 to 9 of 9

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

  1. [bisected] kernel panic 2.6.22 -> 2.6.26-rc9+

    Hi,

    Bisected the panic. It seems it occurs on my ARM IXP4xx big-endian
    system with USB ADSL 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.

    Reverting the commit on top of 2.6.25.10 fixes the problem.

    Now what's wrong with that?

    7e58886b45bc4a309aeaa8178ef89ff767daaf7f:
    author Stephen Hemminger
    [TCP]: cubic optimization

    Use willy's work in optimizing cube root by having table for small values.

    --- a/net/ipv4/tcp_cubic.c
    +++ b/net/ipv4/tcp_cubic.c
    @@ -91,23 +91,51 @@ static void bictcp_init(struct sock *sk)
    tcp_sk(sk)->snd_ssthresh = initial_ssthresh;
    }

    -/*
    - * calculate the cubic root of x using Newton-Raphson
    +/* calculate the cubic root of x using a table lookup followed by one
    + * Newton-Raphson iteration.
    + * Avg err ~= 0.195%
    */
    static u32 cubic_root(u64 a)
    {
    - u32 x;
    -
    - /* Initial estimate is based on:
    - * cbrt(x) = exp(log(x) / 3)
    + u32 x, b, shift;
    + /*
    + * cbrt(x) MSB values for x MSB values in [0..63].
    + * Precomputed then refined by hand - Willy Tarreau
    + *
    + * For x in [0..63],
    + * v = cbrt(x << 18) - 1
    + * cbrt(x) = (v[x] + 10) >> 6
    */
    - x = 1u << (fls64(a)/3);
    + static const u8 v[] = {
    + /* 0x00 */ 0, 54, 54, 54, 118, 118, 118, 118,
    + /* 0x08 */ 123, 129, 134, 138, 143, 147, 151, 156,
    + /* 0x10 */ 157, 161, 164, 168, 170, 173, 176, 179,
    + /* 0x18 */ 181, 185, 187, 190, 192, 194, 197, 199,
    + /* 0x20 */ 200, 202, 204, 206, 209, 211, 213, 215,
    + /* 0x28 */ 217, 219, 221, 222, 224, 225, 227, 229,
    + /* 0x30 */ 231, 232, 234, 236, 237, 239, 240, 242,
    + /* 0x38 */ 244, 245, 246, 248, 250, 251, 252, 254,
    + };
    +
    + b = fls64(a);
    + if (b < 7) {
    + /* a in [0..63] */
    + return ((u32)v[(u32)a] + 35) >> 6;
    + }
    +
    + b = ((b * 84) >> 8) - 1;
    + shift = (a >> (b * 3));

    - /* converges to 32 bits in 3 iterations */
    - x = (2 * x + (u32)div64_64(a, (u64)x*(u64)x)) / 3;
    - x = (2 * x + (u32)div64_64(a, (u64)x*(u64)x)) / 3;
    - x = (2 * x + (u32)div64_64(a, (u64)x*(u64)x)) / 3;
    + x = ((u32)(((u32)v[shift] + 10) << b)) >> 6;

    + /*
    + * Newton-Raphson iteration
    + * 2
    + * x = ( 2 * x + a / x ) / 3
    + * k+1 k k
    + */
    + x = (2 * x + (u32)div64_64(a, (u64)x * (u64)(x - 1)));
    + x = ((x * 341) >> 10);
    return x;
    }

    PC is at bictcp_cong_avoid+0x33c/0x454

    Backtrace:

    bictcp_cong_avoid+0x0/0x454 from tcp_cong_avoid+0x1c/0x30
    tcp_cong_avoid+0x0/0x30 from tcp_ack+0x156c/0x1c6c
    r4:00000000
    tcp_ack+0x0/0x1c6c from tcp_rcv_established+0x4a8/0x76c
    tcp_rcv_established+0x0/0x76c from tcp_v4_do_rcv+0x25c/0x458
    tcp_v4_do_rcv+0x0/0x458 from tcp_v4_rcv+0x6c4/0x7ec
    tcp_v4_rcv+0x0/0x7ec from ip_local_deliver_finish+0xe4/0x24c
    ip_local_deliver_finish+0x0/0x24c from ip_local_deliver+0x4c/0xac
    r7:c043817c r6:c6818612 r5:c6a6a3c0 r4:c6a6a3c0
    ip_local_deliver+0x0/0xac from ip_rcv_finish+0x114/0x378
    r4:80000000
    ip_rcv_finish+0x0/0x378 from ip_rcv+0x1f8/0x2a8
    ip_rcv+0x0/0x2a8 from netif_receive_skb+0x36c/0x4a4
    r7:00000800 r6:c0405e80 r5:c6a6a3c0 r4:c0436d54
    netif_receive_skb+0x0/0x4a4 from process_backlog+0x88/0x14c
    r8:00000040 r7:c0436d24 r6:00000000 r5:c0436d0c r4:c6912c00
    process_backlog+0x0/0x14c from net_rx_action+0xb4/0x1c4
    net_rx_action+0x0/0x1c4 from __do_softirq+0x68/0xe0
    _do_softirq+0x0/0xe0 from irq_exit+0x48/0x50
    r7:00000000 r6:c04344d0 r5:c040ae24 r4:00000014
    irq_exit+0x0/0x50 from asm_do_IRQ+0x48/0x5c
    asm_do_IRQ+0x0/0x5c from __irq_svc+0x24/0x60
    ....
    --
    Krzysztof Halasa
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

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


    (cc's added)

    On Sun, 13 Jul 2008 02:28:13 +0200 Krzysztof Halasa wrote:

    > Hi,
    >
    > Bisected the panic. It seems it occurs on my ARM IXP4xx big-endian
    > system with USB ADS


    I guess you're referring to this:
    http://linux.derkeiler.com/Mailing-L.../msg04754.html

    > 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.
    >
    > Reverting the commit on top of 2.6.25.10 fixes the problem.
    >
    > Now what's wrong with that?
    >
    > 7e58886b45bc4a309aeaa8178ef89ff767daaf7f:
    > author Stephen Hemminger
    > [TCP]: cubic optimization
    >
    > Use willy's work in optimizing cube root by having table for small values.
    >
    > --- a/net/ipv4/tcp_cubic.c
    > +++ b/net/ipv4/tcp_cubic.c
    > @@ -91,23 +91,51 @@ static void bictcp_init(struct sock *sk)
    > tcp_sk(sk)->snd_ssthresh = initial_ssthresh;
    > }
    >
    > -/*
    > - * calculate the cubic root of x using Newton-Raphson
    > +/* calculate the cubic root of x using a table lookup followed by one
    > + * Newton-Raphson iteration.
    > + * Avg err ~= 0.195%
    > */
    > static u32 cubic_root(u64 a)
    > {
    > - u32 x;
    > -
    > - /* Initial estimate is based on:
    > - * cbrt(x) = exp(log(x) / 3)
    > + u32 x, b, shift;
    > + /*
    > + * cbrt(x) MSB values for x MSB values in [0..63].
    > + * Precomputed then refined by hand - Willy Tarreau
    > + *
    > + * For x in [0..63],
    > + * v = cbrt(x << 18) - 1
    > + * cbrt(x) = (v[x] + 10) >> 6
    > */
    > - x = 1u << (fls64(a)/3);
    > + static const u8 v[] = {
    > + /* 0x00 */ 0, 54, 54, 54, 118, 118, 118, 118,
    > + /* 0x08 */ 123, 129, 134, 138, 143, 147, 151, 156,
    > + /* 0x10 */ 157, 161, 164, 168, 170, 173, 176, 179,
    > + /* 0x18 */ 181, 185, 187, 190, 192, 194, 197, 199,
    > + /* 0x20 */ 200, 202, 204, 206, 209, 211, 213, 215,
    > + /* 0x28 */ 217, 219, 221, 222, 224, 225, 227, 229,
    > + /* 0x30 */ 231, 232, 234, 236, 237, 239, 240, 242,
    > + /* 0x38 */ 244, 245, 246, 248, 250, 251, 252, 254,
    > + };
    > +
    > + b = fls64(a);
    > + if (b < 7) {
    > + /* a in [0..63] */
    > + return ((u32)v[(u32)a] + 35) >> 6;
    > + }
    > +
    > + b = ((b * 84) >> 8) - 1;
    > + shift = (a >> (b * 3));
    >
    > - /* converges to 32 bits in 3 iterations */
    > - x = (2 * x + (u32)div64_64(a, (u64)x*(u64)x)) / 3;
    > - x = (2 * x + (u32)div64_64(a, (u64)x*(u64)x)) / 3;
    > - x = (2 * x + (u32)div64_64(a, (u64)x*(u64)x)) / 3;
    > + x = ((u32)(((u32)v[shift] + 10) << b)) >> 6;
    >
    > + /*
    > + * Newton-Raphson iteration
    > + * 2
    > + * x = ( 2 * x + a / x ) / 3
    > + * k+1 k k
    > + */
    > + x = (2 * x + (u32)div64_64(a, (u64)x * (u64)(x - 1)));
    > + x = ((x * 341) >> 10);
    > return x;
    > }
    >
    > PC is at bictcp_cong_avoid+0x33c/0x454
    >
    > Backtrace:
    >
    > bictcp_cong_avoid+0x0/0x454 from tcp_cong_avoid+0x1c/0x30
    > tcp_cong_avoid+0x0/0x30 from tcp_ack+0x156c/0x1c6c
    > r4:00000000
    > tcp_ack+0x0/0x1c6c from tcp_rcv_established+0x4a8/0x76c
    > tcp_rcv_established+0x0/0x76c from tcp_v4_do_rcv+0x25c/0x458
    > tcp_v4_do_rcv+0x0/0x458 from tcp_v4_rcv+0x6c4/0x7ec
    > tcp_v4_rcv+0x0/0x7ec from ip_local_deliver_finish+0xe4/0x24c
    > ip_local_deliver_finish+0x0/0x24c from ip_local_deliver+0x4c/0xac
    > r7:c043817c r6:c6818612 r5:c6a6a3c0 r4:c6a6a3c0
    > ip_local_deliver+0x0/0xac from ip_rcv_finish+0x114/0x378
    > r4:80000000
    > ip_rcv_finish+0x0/0x378 from ip_rcv+0x1f8/0x2a8
    > ip_rcv+0x0/0x2a8 from netif_receive_skb+0x36c/0x4a4
    > r7:00000800 r6:c0405e80 r5:c6a6a3c0 r4:c0436d54
    > netif_receive_skb+0x0/0x4a4 from process_backlog+0x88/0x14c
    > r8:00000040 r7:c0436d24 r6:00000000 r5:c0436d0c r4:c6912c00
    > process_backlog+0x0/0x14c from net_rx_action+0xb4/0x1c4
    > net_rx_action+0x0/0x1c4 from __do_softirq+0x68/0xe0
    > _do_softirq+0x0/0xe0 from irq_exit+0x48/0x50
    > r7:00000000 r6:c04344d0 r5:c040ae24 r4:00000014
    > irq_exit+0x0/0x50 from asm_do_IRQ+0x48/0x5c
    > asm_do_IRQ+0x0/0x5c from __irq_svc+0x24/0x60
    > ...
    > --
    > Krzysztof Halasa
    > --
    > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    > the body of a message to majordomo@vger.kernel.org
    > More majordomo info at http://vger.kernel.org/majordomo-info.html
    > Please read the FAQ at http://www.tux.org/lkml/

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

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

    Andrew Morton writes:

    > (cc's added)


    (cc added) :-)

    > I guess you're referring to this:
    > http://linux.derkeiler.com/Mailing-L.../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.

    Perhaps it's time to fix it definitely?
    --
    Krzysztof Halasa
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

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

    On Sun, 13 Jul 2008 12:48:15 +0200 Krzysztof Halasa wrote:

    > Andrew Morton writes:
    >
    > > (cc's added)

    >
    > (cc added) :-)
    >
    > > I guess you're referring to this:
    > > http://linux.derkeiler.com/Mailing-L.../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) )
    _

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

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

    Andrew Morton writes:

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

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

    On Sun, 13 Jul 2008 23:51:06 +0200 Krzysztof Halasa wrote:

    > Andrew Morton writes:
    >
    > > --- 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.
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

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

    On Sun, Jul 13, 2008 at 02:55:25PM -0700, Andrew Morton wrote:
    > On Sun, 13 Jul 2008 23:51:06 +0200 Krzysztof Halasa wrote:
    >
    > > Andrew Morton writes:
    > >
    > > > --- 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.


    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

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

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

    On Tue, Jul 22, 2008 at 10:00:56PM -0700, Andrew Morton wrote:
    > On Wed, 23 Jul 2008 06:52:31 +0200 Willy Tarreau wrote:
    >
    > > On Sun, Jul 13, 2008 at 02:55:25PM -0700, Andrew Morton wrote:
    > > > On Sun, 13 Jul 2008 23:51:06 +0200 Krzysztof Halasa wrote:
    > > >
    > > > > Andrew Morton writes:
    > > > >
    > > > > > --- 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.

    > >
    > > 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
    >
    > arm's fls() is implemented as a macro, causing it to misbehave when passed
    > 64-bit arguments. Fix.
    >
    > Cc: Nickolay Vinogradov
    > Tested-by: Krzysztof Halasa
    > Signed-off-by: Andrew Morton
    > ---
    >
    > 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) )
    > _
    >


    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

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

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

    On Wed, 23 Jul 2008 06:52:31 +0200 Willy Tarreau wrote:

    > On Sun, Jul 13, 2008 at 02:55:25PM -0700, Andrew Morton wrote:
    > > On Sun, 13 Jul 2008 23:51:06 +0200 Krzysztof Halasa wrote:
    > >
    > > > Andrew Morton writes:
    > > >
    > > > > --- 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.

    >
    > 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

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

    Cc: Nickolay Vinogradov
    Tested-by: Krzysztof Halasa
    Signed-off-by: Andrew Morton
    ---

    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.

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@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 to Thread