x86: ppc fixes for find_first_bit

PATCH Applied Alexander van Heukelum 4 participants 7 messages Apr 14, 2008 – Apr 17, 2008

Share & tools

Alexander van Heukelum

author

2008-04-14 16:48 UTC #0 lore raw

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

Signature · 6 lines

  Alexander van Heukelum
  [email protected]

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

Ingo Molnar

2008-04-16 12:57 UTC #1 lore raw

Quoted 22 lines · Alexander van Heukelum

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.

Quoted 4 lines

"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

Alexander van Heukelum

2008-04-16 13:55 UTC #2 lore raw

PATCH 1/1 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 <[email protected]>

---

Quoted 28 lines · Ingo Molnar

Quoted 22 lines · Alexander van Heukelum
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.
Quoted 4 lines
"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.

Quoted 4 lines

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 <asm-generic/bitops/find.h> 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 <[email protected]>.

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 <asm-generic/bitops/find.h> from their asm-$ARCH/bitops.h.

Greetings,
Alexander

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

include/asm-powerpc/bitops.h +1 ~~-16~~

index 2fc0c45..e2dbb53 100644
@@ -318,23 +318,8 @@ static __inline__ unsigned long __fls(unsigned long x)
 	return __ilog2(x);
 }
 #include <asm-generic/bitops/fls64.h>
-
 #include <asm-generic/bitops/hweight.h>
-
-#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 <asm-generic/bitops/find.h>
 
 /* Little-endian versions */

Ingo Molnar

Applied

2008-04-16 14:41 UTC #3 lore raw

Re: [PATCH] x86: fix find_next_bit breakage on ppc and powerpc

Quoted 9 lines · Alexander van Heukelum

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 <[email protected]>

thanks, applied. I dropped:

Subject: powerpc: fix powerpc build
From: Thomas Gleixner <[email protected]>

and i'm keeping:

Subject: generic: find_next_bit() fix
From: Thomas Gleixner <[email protected]>

should i also drop:

Subject: find_first_bit() ppc fix
From: Thomas Gleixner <[email protected]>

Ingo

Alexander van Heukelum

author

2008-04-16 15:03 UTC #4 lore raw

Re: [PATCH] x86: fix find_next_bit breakage on ppc and powerpc

Quoted 23 lines · Ingo Molnar

Quoted 9 lines · Alexander van Heukelum
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 <[email protected]>
thanks, applied. I dropped:
Subject: powerpc: fix powerpc build
From: Thomas Gleixner <[email protected]>
and i'm keeping:
Subject: generic: find_next_bit() fix
From: Thomas Gleixner <[email protected]>

Quoted 6 lines

should i also drop:
Subject: find_first_bit() ppc fix
From: Thomas Gleixner <[email protected]>
?

Yes, that problem should be fixed by my patch.

Thanks,
Alexander

Quoted 1 line

Ingo

Signature · 5 lines

  Alexander van Heukelum
  [email protected]

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

Paul Mackerras

2008-04-16 22:55 UTC #5 lore raw

Re: [PATCH] x86: fix find_next_bit breakage on ppc and powerpc

In reply to Ingo Molnar

Quoted 7 lines · Ingo Molnar

Quoted 4 lines · Alexander van Heukelum
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.

Quoted 7 lines

Quoted 5 lines
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 <[email protected]>
thanks, applied. I dropped:

Why are powerpc (and ppc) patches
- not being sent to the powerpc maintainer (me)
- not being cc'd to the [email protected] 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.

Alexander van Heukelum

author

2008-04-17 09:04 UTC #6 lore raw

Re: [PATCH] x86: fix find_next_bit breakage on ppc and powerpc

Quoted 12 lines · Paul Mackerras

Quoted 11 lines · Ingo Molnar
Quoted 8 lines · Alexander van Heukelum
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.

Quoted 8 lines

Quoted 3 lines
Quoted 1 line
Signed-off-by Alexander van Heukelum <[email protected]>
thanks, applied. I dropped:
Why are powerpc (and ppc) patches
- not being sent to the powerpc maintainer (me)
- not being cc'd to the [email protected] 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.

Quoted 2 lines

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

Quoted 1 line

Paul.

Signature · 5 lines

  Alexander van Heukelum
  [email protected]

-- 
http://www.fastmail.fm - The professional email service