[PATCH] asm-generic/bitops/fls64.h - Kernel

This is a discussion on [PATCH] asm-generic/bitops/fls64.h - Kernel ; bugfix in fls64 on a big endian systems(against 2.6.25). Signed-off-by: Nickolay Vinogradov -- diff --git a/include/asm-generic/bitops/fls64.h b/include/asm-generic/bitops/fls64.h index 1b6b17c..2eedb6f 100644 --- a/include/asm-generic/bitops/fls64.h +++ b/include/asm-generic/bitops/fls64.h @@ -8,7 +8,7 @@ static inline int fls64(__u64 x) __u32 h = x >> 32; if ...

+ Reply to Thread
Results 1 to 13 of 13

Thread: [PATCH] asm-generic/bitops/fls64.h

  1. [PATCH] asm-generic/bitops/fls64.h

    bugfix in fls64 on a big endian systems(against 2.6.25).

    Signed-off-by: Nickolay Vinogradov

    --

    diff --git a/include/asm-generic/bitops/fls64.h
    b/include/asm-generic/bitops/fls64.h
    index 1b6b17c..2eedb6f 100644
    --- a/include/asm-generic/bitops/fls64.h
    +++ b/include/asm-generic/bitops/fls64.h
    @@ -8,7 +8,7 @@ static inline int fls64(__u64 x)
    __u32 h = x >> 32;
    if (h)
    return fls(h) + 32;
    - return fls(x);
    + return fls((__u32)x);
    }

    #endif /* _ASM_GENERIC_BITOPS_FLS64_H_ */

    --
    Nickolay Vinogradov
    Protei Research and Development Center
    St.Petersburg, 194044, Russia
    Tel.: +7 812 449 47 27
    --
    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: [PATCH] asm-generic/bitops/fls64.h

    On Sun, 04 May 2008 22:58:50 +0400
    ____________________ ______________ ____________________ wrote:

    > bugfix in fls64 on a big endian systems(against 2.6.25).
    >
    > Signed-off-by: Nickolay Vinogradov
    >
    > --
    >
    > diff --git a/include/asm-generic/bitops/fls64.h
    > b/include/asm-generic/bitops/fls64.h
    > index 1b6b17c..2eedb6f 100644
    > --- a/include/asm-generic/bitops/fls64.h
    > +++ b/include/asm-generic/bitops/fls64.h
    > @@ -8,7 +8,7 @@ static inline int fls64(__u64 x)
    > __u32 h = x >> 32;
    > if (h)
    > return fls(h) + 32;
    > - return fls(x);
    > + return fls((__u32)x);
    > }
    >
    > #endif /* _ASM_GENERIC_BITOPS_FLS64_H_ */


    Please describe the bug which you are fixing?

    Perhaps a more robust fix to would be to
    repair fls() so that it works correctly when passed a u64. Perhaps.
    --
    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: [PATCH] asm-generic/bitops/fls64.h

    Andrew Morton пишет:
    > On Sun, 04 May 2008 22:58:50 +0400
    > ____________________ ______________ ____________________ wrote:
    >
    >> bugfix in fls64 on a big endian systems(against 2.6.25).
    >>
    >> Signed-off-by: Nickolay Vinogradov
    >>
    >> --
    >>
    >> diff --git a/include/asm-generic/bitops/fls64.h
    >> b/include/asm-generic/bitops/fls64.h
    >> index 1b6b17c..2eedb6f 100644
    >> --- a/include/asm-generic/bitops/fls64.h
    >> +++ b/include/asm-generic/bitops/fls64.h
    >> @@ -8,7 +8,7 @@ static inline int fls64(__u64 x)
    >> __u32 h = x >> 32;
    >> if (h)
    >> return fls(h) + 32;
    >> - return fls(x);
    >> + return fls((__u32)x);
    >> }
    >>
    >> #endif /* _ASM_GENERIC_BITOPS_FLS64_H_ */

    >
    > Please describe the bug which you are fixing?
    >
    > Perhaps a more robust fix to would be to
    > repair fls() so that it works correctly when passed a u64. Perhaps.


    Repair fls64() so that it works correctly when passed a u64.

    --
    Nickolay Vinogradov
    Protei Research and Development Center
    St.Petersburg, 194044, Russia
    Tel.: +7 812 449 47 27
    --
    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: [PATCH] asm-generic/bitops/fls64.h

    > bugfix in fls64 on a big endian systems(against 2.6.25).
    >
    > Signed-off-by: Nickolay Vinogradov
    >
    > --
    >
    > diff --git a/include/asm-generic/bitops/fls64.h
    > b/include/asm-generic/bitops/fls64.h
    > index 1b6b17c..2eedb6f 100644
    > --- a/include/asm-generic/bitops/fls64.h
    > +++ b/include/asm-generic/bitops/fls64.h
    > @@ -8,7 +8,7 @@ static inline int fls64(__u64 x)
    > __u32 h = x >> 32;
    > if (h)
    > return fls(h) + 32;
    > - return fls(x);
    > + return fls((__u32)x);
    > }


    Hi Nickolay,

    The change is ok, I guess, but the cast should be a no-op (fls
    takes an int, which is always 32 bit in linux). What is the problem
    you are seeing? Does fls64() return a wrong value in some cases? If
    so, what cpu? Which values?

    Why would this be a bug on big endian systems only? There is no
    pointer magic involved, so the compiler should take care of the
    casts in a correct way.

    Maybe you see a compiler warning? Which compiler version?

    (also note that current (development) kernels now have separate
    versions for 32-bit and 64-bit environments.)

    Greetings,
    Alexander

    > #endif /* _ASM_GENERIC_BITOPS_FLS64_H_ */
    >
    > --
    > Nickolay Vinogradov
    > Protei Research and Development Center
    > St.Petersburg, 194044, Russia
    > Tel.: +7 812 449 47 27
    > --
    > 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/
    >
    >

    --
    Alexander van Heukelum
    heukelum@fastmail.fm

    --
    http://www.fastmail.fm - Or how I learned to stop worrying and
    love email again

    --
    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: [PATCH] asm-generic/bitops/fls64.h

    Alexander van Heukelum пишет:

    > Hi Nickolay,
    >
    > The change is ok, I guess, but the cast should be a no-op (fls
    > takes an int, which is always 32 bit in linux). What is the problem
    > you are seeing? Does fls64() return a wrong value in some cases? If
    > so, what cpu? Which values?
    >
    > Why would this be a bug on big endian systems only? There is no
    > pointer magic involved, so the compiler should take care of the
    > casts in a correct way.
    >
    > Maybe you see a compiler warning? Which compiler version?
    >
    > (also note that current (development) kernels now have separate
    > versions for 32-bit and 64-bit environments.)


    Because fls() is a macro for asm-arm:

    #define fls(x) \
    ( __builtin_constant_p(x) ? constant_fls(x) : \
    ({ int __r; asm("clz\t%0, %1" : "=r"(__r) : "r"(x) : "cc");
    32-__r; }) )

    We can fix it right here:

    diff --git a/include/asm-arm/bitops.h b/include/asm-arm/bitops.h
    index 5c60bfc..ce3fb6f 100644
    --- a/include/asm-arm/bitops.h
    +++ b/include/asm-arm/bitops.h
    @@ -279,7 +279,7 @@ static inline int constant_fls(int 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; }) )
    + ({ int __r; asm("clz\t%0, %1" : "=r"(__r) : "r"((__u32)x) :
    "cc"); 32-__r; }) )
    #define ffs(x) ({ unsigned long __t = (x); fls(__t & -__t); })
    #define __ffs(x) (ffs(x) - 1)
    #define ffz(x) __ffs( ~(x) )


    --
    Nickolay Vinogradov
    Protei Research and Development Center
    St.Petersburg, 194044, Russia
    Tel.: +7 812 449 47 27
    --
    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: [PATCH] asm-generic/bitops/fls64.h

    On Tue, 13 May 2008 16:29:04 +0400, "Nickolay Vinogradov"
    said:
    > Alexander van Heukelum пишет:
    >
    > > Hi Nickolay,
    > >
    > > The change is ok, I guess, but the cast should be a no-op (fls
    > > takes an int, which is always 32 bit in linux). What is the problem
    > > you are seeing? Does fls64() return a wrong value in some cases? If
    > > so, what cpu? Which values?
    > >
    > > Why would this be a bug on big endian systems only? There is no
    > > pointer magic involved, so the compiler should take care of the
    > > casts in a correct way.
    > >
    > > Maybe you see a compiler warning? Which compiler version?
    > >
    > > (also note that current (development) kernels now have separate
    > > versions for 32-bit and 64-bit environments.)

    >
    > Because fls() is a macro for asm-arm:
    >
    > #define fls(x) \
    > ( __builtin_constant_p(x) ? constant_fls(x) : \
    > ({ int __r; asm("clz\t%0, %1" : "=r"(__r) : "r"(x) : "cc");
    > 32-__r; }) )
    >
    > We can fix it right here:
    >
    > diff --git a/include/asm-arm/bitops.h b/include/asm-arm/bitops.h
    > index 5c60bfc..ce3fb6f 100644
    > --- a/include/asm-arm/bitops.h
    > +++ b/include/asm-arm/bitops.h
    > @@ -279,7 +279,7 @@ static inline int constant_fls(int 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; }) )
    > + ({ int __r; asm("clz\t%0, %1" : "=r"(__r) : "r"((__u32)x) :
    > "cc"); 32-__r; }) )
    > #define ffs(x) ({ unsigned long __t = (x); fls(__t & -__t); })
    > #define __ffs(x) (ffs(x) - 1)
    > #define ffz(x) __ffs( ~(x) )


    Hmm, indeed.

    Maybe: ({ int __r, __x = (x); asm("clz\t%0, %1" : "=r"(__r) : "r"(__x) :
    "cc"); 32-__r; }) ?

    This is a 32-bit machine, right? Doesn't the compiler complain about
    feeding a long long into a 32-bit register?

    Greetings,
    Alexander
    --
    Alexander van Heukelum
    heukelum@fastmail.fm

    --
    http://www.fastmail.fm - I mean, what is it about a decent email service?

    --
    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: [PATCH] asm-generic/bitops/fls64.h

    On Tue, May 13, 2008 at 03:24:13PM +0200, Alexander van Heukelum wrote:
    > On Tue, 13 May 2008 16:29:04 +0400, "Nickolay Vinogradov"
    > said:
    > > Alexander van Heukelum пишет:
    > >
    > > > Hi Nickolay,
    > > >
    > > > The change is ok, I guess, but the cast should be a no-op (fls
    > > > takes an int, which is always 32 bit in linux). What is the problem
    > > > you are seeing? Does fls64() return a wrong value in some cases? If
    > > > so, what cpu? Which values?
    > > >
    > > > Why would this be a bug on big endian systems only? There is no
    > > > pointer magic involved, so the compiler should take care of the
    > > > casts in a correct way.
    > > >
    > > > Maybe you see a compiler warning? Which compiler version?
    > > >
    > > > (also note that current (development) kernels now have separate
    > > > versions for 32-bit and 64-bit environments.)

    > >
    > > Because fls() is a macro for asm-arm:
    > >
    > > #define fls(x) \
    > > ( __builtin_constant_p(x) ? constant_fls(x) : \
    > > ({ int __r; asm("clz\t%0, %1" : "=r"(__r) : "r"(x) : "cc");
    > > 32-__r; }) )
    > >
    > > We can fix it right here:


    No. "fls" is for finding the last set bit in an _int_. It is not
    supposed to have random crap passed to it, such as types longer than
    sizeof(int).

    If you're going to pass long long (64-bit) arguments to fls, and then
    cast them to a u32, you're truncating the value, and you'll get the
    wrong answer if bit 33 or greater is set. If you don't actually care
    about the upper bits, don't pass a 64-bit quantity to fls().

    If you want to use fls with a long long, use fls64 instead. Or for top
    marks, use a u64 and fls64.

    --
    Russell King
    Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
    maintainer of:
    --
    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: [PATCH] asm-generic/bitops/fls64.h

    On Tue, 13 May 2008 14:58:39 +0100, "Russell King"
    said:
    > On Tue, May 13, 2008 at 03:24:13PM +0200, Alexander van Heukelum wrote:
    > > On Tue, 13 May 2008 16:29:04 +0400, "Nickolay Vinogradov"
    > > said:
    > > > Alexander van Heukelum пишет:
    > > >
    > > > > Hi Nickolay,
    > > > >
    > > > > The change is ok, I guess, but the cast should be a no-op (fls
    > > > > takes an int, which is always 32 bit in linux). What is the problem
    > > > > you are seeing? Does fls64() return a wrong value in some cases? If
    > > > > so, what cpu? Which values?
    > > > >
    > > > > Why would this be a bug on big endian systems only? There is no
    > > > > pointer magic involved, so the compiler should take care of the
    > > > > casts in a correct way.
    > > > >
    > > > > Maybe you see a compiler warning? Which compiler version?
    > > > >
    > > > > (also note that current (development) kernels now have separate
    > > > > versions for 32-bit and 64-bit environments.)
    > > >
    > > > Because fls() is a macro for asm-arm:
    > > >
    > > > #define fls(x) \
    > > > ( __builtin_constant_p(x) ? constant_fls(x) : \
    > > > ({ int __r; asm("clz\t%0, %1" : "=r"(__r) : "r"(x) : "cc");
    > > > 32-__r; }) )
    > > >
    > > > We can fix it right here:

    >
    > No. "fls" is for finding the last set bit in an _int_. It is not
    > supposed to have random crap passed to it, such as types longer than
    > sizeof(int).
    >
    > If you're going to pass long long (64-bit) arguments to fls, and then
    > cast them to a u32, you're truncating the value, and you'll get the
    > wrong answer if bit 33 or greater is set. If you don't actually care
    > about the upper bits, don't pass a 64-bit quantity to fls().
    >
    > If you want to use fls with a long long, use fls64 instead. Or for top
    > marks, use a u64 and fls64.


    But that was the problem we began with: the generic fls64 passes an u64
    to fls. Nickolay's original patch solves that by putting a cast to u32
    in fls64. I did not, however, understand why the cast was needed. It
    should not be needed for correctness, imho, because fls is expected to
    behave as if it was a function "int fls(int)", like ffs. Values passed
    to fls should thus be truncated if necessary.

    Making the truncation explicit in fls64 is still a good idea, though.

    Greetings,
    Alexander

    > --
    > Russell King
    > Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
    > maintainer of:

    --
    Alexander van Heukelum
    heukelum@fastmail.fm

    --
    http://www.fastmail.fm - The way an email service should be

    --
    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: [PATCH] asm-generic/bitops/fls64.h

    Russell King writes:

    > On Tue, May 13, 2008 at 03:24:13PM +0200, Alexander van Heukelum wrote:
    >> On Tue, 13 May 2008 16:29:04 +0400, "Nickolay Vinogradov"
    >> said:
    >> > Alexander van Heukelum пишет:
    >> >
    >> > > Hi Nickolay,
    >> > >
    >> > > The change is ok, I guess, but the cast should be a no-op (fls
    >> > > takes an int, which is always 32 bit in linux). What is the problem
    >> > > you are seeing? Does fls64() return a wrong value in some cases? If
    >> > > so, what cpu? Which values?
    >> > >
    >> > > Why would this be a bug on big endian systems only? There is no
    >> > > pointer magic involved, so the compiler should take care of the
    >> > > casts in a correct way.
    >> > >
    >> > > Maybe you see a compiler warning? Which compiler version?
    >> > >
    >> > > (also note that current (development) kernels now have separate
    >> > > versions for 32-bit and 64-bit environments.)
    >> >
    >> > Because fls() is a macro for asm-arm:
    >> >
    >> > #define fls(x) \
    >> > ( __builtin_constant_p(x) ? constant_fls(x) : \
    >> > ({ int __r; asm("clz\t%0, %1" : "=r"(__r) : "r"(x) : "cc");
    >> > 32-__r; }) )
    >> >
    >> > We can fix it right here:

    >
    > No. "fls" is for finding the last set bit in an _int_. It is not
    > supposed to have random crap passed to it, such as types longer than
    > sizeof(int).


    If you write fls as an (inline) function then the argument is implicitly
    converted.

    Andreas.

    --
    Andreas Schwab, SuSE Labs, schwab@suse.de
    SuSE Linux Products GmbH, Maxfeldstrae 5, 90409 Nrnberg, Germany
    PGP key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
    "And now for something completely different."
    --
    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/

  10. Re: [PATCH] asm-generic/bitops/fls64.h



    Alexander van Heukelum пишет:

    >> No. "fls" is for finding the last set bit in an _int_. It is not
    >> supposed to have random crap passed to it, such as types longer than
    >> sizeof(int).
    >>
    >> If you're going to pass long long (64-bit) arguments to fls, and then
    >> cast them to a u32, you're truncating the value, and you'll get the
    >> wrong answer if bit 33 or greater is set. If you don't actually care
    >> about the upper bits, don't pass a 64-bit quantity to fls().
    >>
    >> If you want to use fls with a long long, use fls64 instead. Or for top
    >> marks, use a u64 and fls64.

    >
    > But that was the problem we began with: the generic fls64 passes an u64
    > to fls. Nickolay's original patch solves that by putting a cast to u32
    > in fls64. I did not, however, understand why the cast was needed.


    The cast was needed because fls is a macro, not a function.

    --
    Nickolay Vinogradov
    Protei Research and Development Center
    St.Petersburg, 194044, Russia
    Tel.: +7 812 449 47 27
    --
    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/

  11. Re: [PATCH] asm-generic/bitops/fls64.h

    On Tue, 13 May 2008 14:43:43 +0400 Nickolay Vinogradov wrote:

    > Andrew Morton __________:
    > > On Sun, 04 May 2008 22:58:50 +0400
    > > ____________________ ______________ ____________________ wrote:
    > >
    > >> bugfix in fls64 on a big endian systems(against 2.6.25).
    > >>
    > >> Signed-off-by: Nickolay Vinogradov
    > >>
    > >> --
    > >>
    > >> diff --git a/include/asm-generic/bitops/fls64.h
    > >> b/include/asm-generic/bitops/fls64.h
    > >> index 1b6b17c..2eedb6f 100644
    > >> --- a/include/asm-generic/bitops/fls64.h
    > >> +++ b/include/asm-generic/bitops/fls64.h
    > >> @@ -8,7 +8,7 @@ static inline int fls64(__u64 x)
    > >> __u32 h = x >> 32;
    > >> if (h)
    > >> return fls(h) + 32;
    > >> - return fls(x);
    > >> + return fls((__u32)x);
    > >> }
    > >>
    > >> #endif /* _ASM_GENERIC_BITOPS_FLS64_H_ */

    > >
    > > Please describe the bug which you are fixing?
    > >
    > > Perhaps a more robust fix to would be to
    > > repair fls() so that it works correctly when passed a u64. Perhaps.

    >
    > Repair fls64() so that it works correctly when passed a u64.
    >


    Yes, but what's wrong with it now?

    The fls() in include/asm-generic/bitops/fls.h takes an int.

    The fls() in include/asm-x86/bitops.h takes an int.

    So both of these will already trucate the incoming argument to 32-bits.

    It seems that you are using a version of fls() which doesn't do this.
    Why? Which architecture are you using? Would it not be more robust to
    fix fls()?
    --
    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/

  12. Re: [PATCH] asm-generic/bitops/fls64.h

    Andrew Morton пишет:
    > On Tue, 13 May 2008 14:43:43 +0400 Nickolay Vinogradov wrote:
    >
    >> Andrew Morton __________:
    >>> On Sun, 04 May 2008 22:58:50 +0400
    >>> ____________________ ______________ ____________________ wrote:
    >>>
    >>>> bugfix in fls64 on a big endian systems(against 2.6.25).
    >>>>
    >>>> Signed-off-by: Nickolay Vinogradov
    >>>>
    >>>> --
    >>>>
    >>>> diff --git a/include/asm-generic/bitops/fls64.h
    >>>> b/include/asm-generic/bitops/fls64.h
    >>>> index 1b6b17c..2eedb6f 100644
    >>>> --- a/include/asm-generic/bitops/fls64.h
    >>>> +++ b/include/asm-generic/bitops/fls64.h
    >>>> @@ -8,7 +8,7 @@ static inline int fls64(__u64 x)
    >>>> __u32 h = x >> 32;
    >>>> if (h)
    >>>> return fls(h) + 32;
    >>>> - return fls(x);
    >>>> + return fls((__u32)x);
    >>>> }
    >>>>
    >>>> #endif /* _ASM_GENERIC_BITOPS_FLS64_H_ */
    >>> Please describe the bug which you are fixing?
    >>>
    >>> Perhaps a more robust fix to would be to
    >>> repair fls() so that it works correctly when passed a u64. Perhaps.

    >> Repair fls64() so that it works correctly when passed a u64.
    >>

    >
    > Yes, but what's wrong with it now?
    >
    > The fls() in include/asm-generic/bitops/fls.h takes an int.
    >
    > The fls() in include/asm-x86/bitops.h takes an int.
    >
    > So both of these will already trucate the incoming argument to 32-bits.
    >
    > It seems that you are using a version of fls() which doesn't do this.
    > Why? Which architecture are you using? Would it not be more robust to
    > fix fls()?


    include/asm-arm/bitops.h
    fls() defined here as a macro, not as a function with 32-bit argument.
    Therefore, there are two choices:
    1. explicit cast to u32 before calling.
    2. rewrite fls macro as an inline function.

    --
    Nickolay Vinogradov
    Protei Research and Development Center
    St.Petersburg, 194044, Russia
    Tel.: +7 812 449 47 27
    --
    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/

  13. Re: [PATCH] asm-generic/bitops/fls64.h

    On Tue, 13 May 2008 16:35:25 +0200 Andreas Schwab wrote:

    > Russell King writes:
    >
    > > On Tue, May 13, 2008 at 03:24:13PM +0200, Alexander van Heukelum wrote:
    > >> On Tue, 13 May 2008 16:29:04 +0400, "Nickolay Vinogradov"
    > >> said:
    > >> > Alexander van Heukelum пишет:
    > >> >
    > >> > > Hi Nickolay,
    > >> > >
    > >> > > The change is ok, I guess, but the cast should be a no-op (fls
    > >> > > takes an int, which is always 32 bit in linux). What is the problem
    > >> > > you are seeing? Does fls64() return a wrong value in some cases? If
    > >> > > so, what cpu? Which values?
    > >> > >
    > >> > > Why would this be a bug on big endian systems only? There is no
    > >> > > pointer magic involved, so the compiler should take care of the
    > >> > > casts in a correct way.
    > >> > >
    > >> > > Maybe you see a compiler warning? Which compiler version?
    > >> > >
    > >> > > (also note that current (development) kernels now have separate
    > >> > > versions for 32-bit and 64-bit environments.)
    > >> >
    > >> > Because fls() is a macro for asm-arm:
    > >> >
    > >> > #define fls(x) \
    > >> > ( __builtin_constant_p(x) ? constant_fls(x) : \
    > >> > ({ int __r; asm("clz\t%0, %1" : "=r"(__r) : "r"(x) : "cc");
    > >> > 32-__r; }) )
    > >> >
    > >> > We can fix it right here:

    > >
    > > No. "fls" is for finding the last set bit in an _int_. It is not
    > > supposed to have random crap passed to it, such as types longer than
    > > sizeof(int).

    >
    > If you write fls as an (inline) function then the argument is implicitly
    > converted.
    >


    Yes, and that's what other architectures do.

    It's not pretty, but I do think the arm implementation should zap the
    upper 32 bits as other architectures do, and as one would expect from
    the usual C type conversion rules.

    Converting it to a C implementation would be a suitable means...
    --
    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