Re: x86: ppc fixes for find_first_bit - Kernel

This is a discussion on Re: x86: ppc fixes for find_first_bit - Kernel ; Hello Thomas, I see Ingo has applied three fixes to the x86-tree: find_first_bit() ppc fix powerpc: fix powerpc build find_next_bit() fix Could you please give some insight in what went wrong with ppc and powerpc? "find_first_bit() ppc fix" disables the ...

+ Reply to Thread
Results 1 to 7 of 7

Thread: Re: x86: ppc fixes for find_first_bit

  1. Re: x86: ppc fixes for find_first_bit

    Hello Thomas,

    I see Ingo has applied three fixes to the x86-tree:
    find_first_bit() ppc fix
    powerpc: fix powerpc build
    find_next_bit() fix

    Could you please give some insight in what went wrong with
    ppc and powerpc?

    "find_first_bit() ppc fix" disables the use of find_first_bit
    for every user of GENERIC_FIND_NEXT_BIT=y. It replaces it by a
    macro to call find_next_bit with offset=0. It should be possible
    for an arch to use GENERIC_FIND_NEXT_BIT=y and implement
    find_first_bit by itself.

    "powerpc: fix powerpc build" removes the private 'implementation'
    of asm-generic/bitops/find.h. It seems correct code to me. What
    was the problem here? If it is duplicate declarations, then
    I would suggest putting #ifndef GENERIC_FIND_NEXT_BIT around
    them.

    "find_next_bit() fix" changes asm-generic/bitops/find.h to
    declare find_next_bit only if CONFIG_GENERIC_FIND_NEXT_BIT=n.
    That is indeed a good change. It would be better if this
    file disappeared completely, though.

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

    --
    http://www.fastmail.fm - One of many happy users:
    http://www.fastmail.fm/docs/quotes.html

    --
    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: x86: ppc fixes for find_first_bit


    * Alexander van Heukelum wrote:

    > Hello Thomas,
    >
    > I see Ingo has applied three fixes to the x86-tree:
    > find_first_bit() ppc fix
    > powerpc: fix powerpc build
    > find_next_bit() fix
    >
    > Could you please give some insight in what went wrong with
    > ppc and powerpc?
    >
    > "find_first_bit() ppc fix" disables the use of find_first_bit
    > for every user of GENERIC_FIND_NEXT_BIT=y. It replaces it by a
    > macro to call find_next_bit with offset=0. It should be possible
    > for an arch to use GENERIC_FIND_NEXT_BIT=y and implement
    > find_first_bit by itself.
    >
    > "powerpc: fix powerpc build" removes the private 'implementation'
    > of asm-generic/bitops/find.h. It seems correct code to me. What
    > was the problem here? If it is duplicate declarations, then
    > I would suggest putting #ifndef GENERIC_FIND_NEXT_BIT around
    > them.
    >
    > "find_next_bit() fix" changes asm-generic/bitops/find.h to
    > declare find_next_bit only if CONFIG_GENERIC_FIND_NEXT_BIT=n.
    > That is indeed a good change. It would be better if this
    > file disappeared completely, though.


    we had trouble making ppc64 defconfig build fine with your bitops
    changes applied (Thomas might still have the build failure logs). The
    fixes are ad-hoc band-aids to get it to build. We used crosscompilers to
    build on ppc64.

    Ingo
    --
    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. [PATCH] x86: fix find_next_bit breakage on ppc and powerpc

    Powerpc (and ppc) have their have some code in their bitops.h
    which used to be exacly the same as asm-generic/bitops/find.h.
    Include this header instead.

    This should also fix the compile problems due to the generic
    find_next_bit changes. Those were fixed by Thomas Gleixner in
    asm-generic/bitops/find.h earlier.

    Signed-off-by: Alexander van Heukelum

    ---

    On Wed, Apr 16, 2008 at 02:57:24PM +0200, Ingo Molnar wrote:
    > * Alexander van Heukelum wrote:
    > > Hello Thomas,
    > >
    > > I see Ingo has applied three fixes to the x86-tree:
    > > find_first_bit() ppc fix
    > > powerpc: fix powerpc build
    > > find_next_bit() fix
    > >
    > > Could you please give some insight in what went wrong with
    > > ppc and powerpc?
    > >
    > > "find_first_bit() ppc fix" disables the use of find_first_bit
    > > for every user of GENERIC_FIND_NEXT_BIT=y. It replaces it by a
    > > macro to call find_next_bit with offset=0. It should be possible
    > > for an arch to use GENERIC_FIND_NEXT_BIT=y and implement
    > > find_first_bit by itself.
    > >
    > > "powerpc: fix powerpc build" removes the private 'implementation'
    > > of asm-generic/bitops/find.h. It seems correct code to me. What
    > > was the problem here? If it is duplicate declarations, then
    > > I would suggest putting #ifndef GENERIC_FIND_NEXT_BIT around
    > > them.
    > >
    > > "find_next_bit() fix" changes asm-generic/bitops/find.h to
    > > declare find_next_bit only if CONFIG_GENERIC_FIND_NEXT_BIT=n.
    > > That is indeed a good change. It would be better if this
    > > file disappeared completely, though.

    >
    > we had trouble making ppc64 defconfig build fine with your bitops
    > changes applied (Thomas might still have the build failure logs). The
    > fixes are ad-hoc band-aids to get it to build. We used crosscompilers to
    > build on ppc64.


    Hello,

    Yeah, I reproduced the breakage on x86 by putting an #include
    in asm-x86/bitops.h. It's complaining
    a lot, then. Sorry about that breakage. I should have put more
    thought in the possibility of breakage due to that header file
    (I did look at it, but I thought it was harmless).

    Thomas' "find_next_bit() fix" is certainly needed and correct.
    Acked-by: Alexander van Heukelum .

    Could you/Thomas try the following on top of that fix? (i.e., with
    "find_next_bit() fix" and "find_first_bit() ppc fix" removed?)
    I think it should work then.

    Architectures that use CONFIG_GENERIC_FIND_NEXT_BIT=y and include
    asm-generic/bitops/find.h should be able to switch to the generic
    find_first_bit implementation by setting GENERIC_FIND_FIRST_BIT=y
    in asm-$ARCH/Kconfig and removing the #include
    from their asm-$ARCH/bitops.h.

    Greetings,
    Alexander

    include/asm-powerpc/bitops.h | 17 +----------------
    1 files changed, 1 insertions(+), 16 deletions(-)

    diff --git a/include/asm-powerpc/bitops.h b/include/asm-powerpc/bitops.h
    index 2fc0c45..e2dbb53 100644
    --- a/include/asm-powerpc/bitops.h
    +++ b/include/asm-powerpc/bitops.h
    @@ -318,23 +318,8 @@ static __inline__ unsigned long __fls(unsigned long x)
    return __ilog2(x);
    }
    #include
    -
    #include
    -
    -#define find_first_zero_bit(addr, size) find_next_zero_bit((addr), (size), 0)
    -unsigned long find_next_zero_bit(const unsigned long *addr,
    - unsigned long size, unsigned long offset);
    -/**
    - * find_first_bit - find the first set bit in a memory region
    - * @addr: The address to start the search at
    - * @size: The maximum size to search
    - *
    - * Returns the bit-number of the first set bit, not the number of the byte
    - * containing a bit.
    - */
    -#define find_first_bit(addr, size) find_next_bit((addr), (size), 0)
    -unsigned long find_next_bit(const unsigned long *addr,
    - unsigned long size, unsigned long offset);
    +#include

    /* Little-endian versions */

    --
    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] x86: fix find_next_bit breakage on ppc and powerpc


    * Alexander van Heukelum wrote:

    > Powerpc (and ppc) have their have some code in their bitops.h which
    > used to be exacly the same as asm-generic/bitops/find.h. Include this
    > header instead.
    >
    > This should also fix the compile problems due to the generic
    > find_next_bit changes. Those were fixed by Thomas Gleixner in
    > asm-generic/bitops/find.h earlier.
    >
    > Signed-off-by: Alexander van Heukelum


    thanks, applied. I dropped:

    Subject: powerpc: fix powerpc build
    From: Thomas Gleixner

    and i'm keeping:

    Subject: generic: find_next_bit() fix
    From: Thomas Gleixner

    should i also drop:

    Subject: find_first_bit() ppc fix
    From: Thomas Gleixner

    ?

    Ingo
    --
    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] x86: fix find_next_bit breakage on ppc and powerpc


    On Wed, 16 Apr 2008 16:40:51 +0200, "Ingo Molnar" said:
    >
    > * Alexander van Heukelum wrote:
    >
    > > Powerpc (and ppc) have their have some code in their bitops.h which
    > > used to be exacly the same as asm-generic/bitops/find.h. Include this
    > > header instead.
    > >
    > > This should also fix the compile problems due to the generic
    > > find_next_bit changes. Those were fixed by Thomas Gleixner in
    > > asm-generic/bitops/find.h earlier.
    > >
    > > Signed-off-by: Alexander van Heukelum

    >
    > thanks, applied. I dropped:
    >
    > Subject: powerpc: fix powerpc build
    > From: Thomas Gleixner
    >
    > and i'm keeping:
    >
    > Subject: generic: find_next_bit() fix
    > From: Thomas Gleixner
    >
    > should i also drop:
    >
    > Subject: find_first_bit() ppc fix
    > From: Thomas Gleixner
    >
    > ?


    Yes, that problem should be fixed by my patch.

    Thanks,
    Alexander

    > Ingo

    --
    Alexander van Heukelum
    heukelum@fastmail.fm

    --
    http://www.fastmail.fm - Does exactly what it says on the tin

    --
    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] x86: fix find_next_bit breakage on ppc and powerpc

    Ingo Molnar writes:
    >
    > * Alexander van Heukelum wrote:
    >
    > > Powerpc (and ppc) have their have some code in their bitops.h which
    > > used to be exacly the same as asm-generic/bitops/find.h. Include this
    > > header instead.
    > >
    > > This should also fix the compile problems due to the generic
    > > find_next_bit changes. Those were fixed by Thomas Gleixner in
    > > asm-generic/bitops/find.h earlier.
    > >
    > > Signed-off-by: Alexander van Heukelum

    >
    > thanks, applied. I dropped:


    Why are powerpc (and ppc) patches
    - not being sent to the powerpc maintainer (me)
    - not being cc'd to the linuxppc-dev@ozlabs.org list
    - ending up going through the x86 tree?

    How come patches to unify x86_32 and x86_64 bitops need to end up
    touching powerpc?

    Paul.
    --
    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] x86: fix find_next_bit breakage on ppc and powerpc

    On Thu, 17 Apr 2008 08:55:12 +1000, "Paul Mackerras"
    said:
    > Ingo Molnar writes:
    > >
    > > * Alexander van Heukelum wrote:
    > >
    > > > Powerpc (and ppc) have their have some code in their bitops.h which
    > > > used to be exacly the same as asm-generic/bitops/find.h. Include this
    > > > header instead.
    > > >
    > > > This should also fix the compile problems due to the generic
    > > > find_next_bit changes. Those were fixed by Thomas Gleixner in
    > > > asm-generic/bitops/find.h earlier.
    > > >
    > > > Signed-off-by: Alexander van Heukelum

    > >
    > > thanks, applied. I dropped:

    >
    > Why are powerpc (and ppc) patches
    > - not being sent to the powerpc maintainer (me)
    > - not being cc'd to the linuxppc-dev@ozlabs.org list
    > - ending up going through the x86 tree?


    Hello,

    My apologies for that. The patches that are now in x86#testing were
    needed because of changes I introduced. Thomas Gleixner found a
    problem with the patches that caused compile problems for basically
    all archs with GENERIC_FIND_NEXT_BIT=y, and his fix was to change
    asm-generic/bitops/find.h. However, ppc and powerpc did things
    differently... (x86 too, but they have special permissions )

    (Better late than never) Please consider applying the patch in
    http://lkml.org/lkml/2008/4/16/107
    as:

    [PATCH] powerpc: use asm-generic/bitops/find.h in bitops.h

    Powerpc (and ppc) have some code in their bitops.h that is
    exacly the same as asm-generic/bitops/find.h. Include this
    header instead of the private implementation.

    > How come patches to unify x86_32 and x86_64 bitops need to end up
    > touching powerpc?


    This was not a pure unification. Originally I wanted to convert
    both x86_64 and i386 to the existing generic bitops. Andi Kleen,
    however, objected because x86_64 would than lose a certain
    optimization for small bitmaps. I moved this optimization to
    the generic code, and broke non-x86. Everyone except ppc/powerpc
    was fixed by Thomas Gleixner (in a generic header file).
    http://lkml.org/lkml/2008/4/16/107 just changes ppc/powerpc in
    such a way that Thomas' fix works there too.

    Greetings,
    Alexander

    > Paul.

    --
    Alexander van Heukelum
    heukelum@fastmail.fm

    --
    http://www.fastmail.fm - The professional 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/

+ Reply to Thread