Skip to contentSkip to conversation

Expose little-endian bitops

PATCH In review Ben Hutchings 3 participants 6 messages May 31, 2008 – Jun 1, 2008

Subsystems (2)
Generic include/asm header files Bitops API
lore
Share & tools
lore

Ben Hutchings

author
#0 lore raw

cover Expose little-endian bitops

All architectures already have little-endian bitops for use by file- systems that use little-endian allocation tables. Since these are more generally useful - Andrew Morton suggested we use them in the sfc driver - they should be exposed under consistent names. The following patch series does that, using the names from <asm-generic/bitops/le.h> but without the "generic_" prefix.

Ben.

Signature · 2 lines
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
author
#1 lore raw

PATCH 1/2 Remove 'generic_' prefix from generic little-endian bitops

Signed-off-by Ben Hutchings <[email protected]>
---
 include/asm-generic/bitops/ext2-non-atomic.h |   12 ++++----
 include/asm-generic/bitops/le.h              |   40 ++++++++++++++-----------
 include/asm-generic/bitops/minix-le.h        |   10 +++---
 include/asm-m68k/bitops.h                    |    2 +-
 include/asm-m68knommu/bitops.h               |    2 +-
 include/asm-powerpc/bitops.h                 |    7 ++++-
 6 files changed, 41 insertions(+), 32 deletions(-)
include/asm-generic/bitops/ext2-non-atomic.h +6 -6
index 63cf822..b9281c2 100644
@@ -4,17 +4,17 @@
 #include <asm-generic/bitops/le.h>
 
 #define ext2_set_bit(nr,addr)	\
-	generic___test_and_set_le_bit((nr),(unsigned long *)(addr))
+	__test_and_set_le_bit((nr),(unsigned long *)(addr))
 #define ext2_clear_bit(nr,addr)	\
-	generic___test_and_clear_le_bit((nr),(unsigned long *)(addr))
+	__test_and_clear_le_bit((nr),(unsigned long *)(addr))
 
 #define ext2_test_bit(nr,addr)	\
-	generic_test_le_bit((nr),(unsigned long *)(addr))
+	test_le_bit((nr),(unsigned long *)(addr))
 #define ext2_find_first_zero_bit(addr, size) \
-	generic_find_first_zero_le_bit((unsigned long *)(addr), (size))
+	find_first_zero_le_bit((unsigned long *)(addr), (size))
 #define ext2_find_next_zero_bit(addr, size, off) \
-	generic_find_next_zero_le_bit((unsigned long *)(addr), (size), (off))
+	find_next_zero_le_bit((unsigned long *)(addr), (size), (off))
 #define ext2_find_next_bit(addr, size, off) \
-	generic_find_next_le_bit((unsigned long *)(addr), (size), (off))
+	find_next_le_bit((unsigned long *)(addr), (size), (off))
 
 #endif /* _ASM_GENERIC_BITOPS_EXT2_NON_ATOMIC_H_ */
include/asm-generic/bitops/le.h +22 -18
index 80e3bf1..a51c4ca 100644
@@ -9,41 +9,45 @@
 
 #if defined(__LITTLE_ENDIAN)
 
-#define generic_test_le_bit(nr, addr) test_bit(nr, addr)
-#define generic___set_le_bit(nr, addr) __set_bit(nr, addr)
-#define generic___clear_le_bit(nr, addr) __clear_bit(nr, addr)
+#define test_le_bit(nr, addr) test_bit(nr, addr)
+#define __set_le_bit(nr, addr) __set_bit(nr, addr)
+#define __clear_le_bit(nr, addr) __clear_bit(nr, addr)
 
-#define generic_test_and_set_le_bit(nr, addr) test_and_set_bit(nr, addr)
-#define generic_test_and_clear_le_bit(nr, addr) test_and_clear_bit(nr, addr)
+#define test_and_set_le_bit(nr, addr) test_and_set_bit(nr, addr)
+#define test_and_clear_le_bit(nr, addr) test_and_clear_bit(nr, addr)
 
-#define generic___test_and_set_le_bit(nr, addr) __test_and_set_bit(nr, addr)
-#define generic___test_and_clear_le_bit(nr, addr) __test_and_clear_bit(nr, addr)
+#define __test_and_set_le_bit(nr, addr) __test_and_set_bit(nr, addr)
+#define __test_and_clear_le_bit(nr, addr) __test_and_clear_bit(nr, addr)
 
-#define generic_find_next_zero_le_bit(addr, size, offset) find_next_zero_bit(addr, size, offset)
-#define generic_find_next_le_bit(addr, size, offset) \
+#define find_next_zero_le_bit(addr, size, offset) find_next_zero_bit(addr, size, offset)
+#define find_next_le_bit(addr, size, offset) \
 			find_next_bit(addr, size, offset)
 
 #elif defined(__BIG_ENDIAN)
 
-#define generic_test_le_bit(nr, addr) \
+#define test_le_bit(nr, addr) \
 	test_bit((nr) ^ BITOP_LE_SWIZZLE, (addr))
-#define generic___set_le_bit(nr, addr) \
+#define __set_le_bit(nr, addr) \
 	__set_bit((nr) ^ BITOP_LE_SWIZZLE, (addr))
-#define generic___clear_le_bit(nr, addr) \
+#define __clear_le_bit(nr, addr) \
 	__clear_bit((nr) ^ BITOP_LE_SWIZZLE, (addr))
 
-#define generic_test_and_set_le_bit(nr, addr) \
+#define test_and_set_le_bit(nr, addr) \
 	test_and_set_bit((nr) ^ BITOP_LE_SWIZZLE, (addr))
-#define generic_test_and_clear_le_bit(nr, addr) \
+#define test_and_clear_le_bit(nr, addr) \
 	test_and_clear_bit((nr) ^ BITOP_LE_SWIZZLE, (addr))
 
-#define generic___test_and_set_le_bit(nr, addr) \
+#define __test_and_set_le_bit(nr, addr) \
 	__test_and_set_bit((nr) ^ BITOP_LE_SWIZZLE, (addr))
-#define generic___test_and_clear_le_bit(nr, addr) \
+#define __test_and_clear_le_bit(nr, addr) \
 	__test_and_clear_bit((nr) ^ BITOP_LE_SWIZZLE, (addr))
 
+#define find_next_zero_le_bit(addr, size, offset) \
+	generic_find_next_zero_le_bit(addr, size, offset)
 extern unsigned long generic_find_next_zero_le_bit(const unsigned long *addr,
 		unsigned long size, unsigned long offset);
+#define find_next_le_bit(addr, size, offset) \
+	generic_find_next_le_bit(addr, size, offset)
 extern unsigned long generic_find_next_le_bit(const unsigned long *addr,
 		unsigned long size, unsigned long offset);
 

@@ -51,7 +55,7 @@ extern unsigned long generic_find_next_le_bit(const unsigned long *addr,
 #error "Please fix <asm/byteorder.h>"
 #endif
 
-#define generic_find_first_zero_le_bit(addr, size) \
-        generic_find_next_zero_le_bit((addr), (size), 0)
+#define find_first_zero_le_bit(addr, size) \
+        find_next_zero_le_bit((addr), (size), 0)
 
 #endif /* _ASM_GENERIC_BITOPS_LE_H_ */
include/asm-generic/bitops/minix-le.h +5 -5
index 4a981c1..da3613d 100644
@@ -4,14 +4,14 @@
 #include <asm-generic/bitops/le.h>
 
 #define minix_test_and_set_bit(nr,addr)	\
-	generic___test_and_set_le_bit((nr),(unsigned long *)(addr))
+	__test_and_set_le_bit((nr),(unsigned long *)(addr))
 #define minix_set_bit(nr,addr)		\
-	generic___set_le_bit((nr),(unsigned long *)(addr))
+	__set_le_bit((nr),(unsigned long *)(addr))
 #define minix_test_and_clear_bit(nr,addr) \
-	generic___test_and_clear_le_bit((nr),(unsigned long *)(addr))
+	__test_and_clear_le_bit((nr),(unsigned long *)(addr))
 #define minix_test_bit(nr,addr)		\
-	generic_test_le_bit((nr),(unsigned long *)(addr))
+	test_le_bit((nr),(unsigned long *)(addr))
 #define minix_find_first_zero_bit(addr,size) \
-	generic_find_first_zero_le_bit((unsigned long *)(addr),(size))
+	find_first_zero_le_bit((unsigned long *)(addr),(size))
 
 #endif /* _ASM_GENERIC_BITOPS_MINIX_LE_H_ */
include/asm-m68k/bitops.h +1 -1
index 83d1f28..27d7ae9 100644
@@ -411,7 +411,7 @@ static inline int ext2_find_next_zero_bit(const void *vaddr, unsigned size,
 	return (p - addr) * 32 + res;
 }
 #define ext2_find_next_bit(addr, size, off) \
-	generic_find_next_le_bit((unsigned long *)(addr), (size), (off))
+	find_next_le_bit((unsigned long *)(addr), (size), (off))
 
 #endif /* __KERNEL__ */
include/asm-m68knommu/bitops.h +1 -1
index c142fbf..6213a08 100644
@@ -295,7 +295,7 @@ found_middle:
 }
 
 #define ext2_find_next_bit(addr, size, off) \
-	generic_find_next_le_bit((unsigned long *)(addr), (size), (off))
+	find_next_le_bit((unsigned long *)(addr), (size), (off))
 #include <asm-generic/bitops/minix.h>
 
 #endif /* __KERNEL__ */
include/asm-powerpc/bitops.h +6 -1
index 897eade..dcbf9a8 100644
@@ -362,10 +362,15 @@ static __inline__ int test_le_bit(unsigned long nr,
 #define __test_and_clear_le_bit(nr, addr) \
 	__test_and_clear_bit((nr) ^ BITOP_LE_SWIZZLE, (addr))
 
-#define find_first_zero_le_bit(addr, size) generic_find_next_zero_le_bit((addr), (size), 0)
+#define find_first_zero_le_bit(addr, size) \
+	generic_find_next_zero_le_bit(addr, size, 0)
+#define find_next_zero_le_bit(addr, size, offset) \
+	generic_find_next_zero_le_bit(addr, size, offset)
 unsigned long generic_find_next_zero_le_bit(const unsigned long *addr,
 				    unsigned long size, unsigned long offset);
 
+#define find_next_le_bit(addr, size, offset) \
+	generic_find_next_le_bit(addr, size, offset)
 unsigned long generic_find_next_le_bit(const unsigned long *addr,
 				    unsigned long size, unsigned long offset);
 /* Bitmap functions for the ext2 filesystem */
Signature · 2 lines
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
author
#2 lore raw

PATCH 2/2 Use <asm-generic/bitops/le.h> in <asm-powerpc/bitops.h>

In reply to Ben Hutchings

The powerpc little-endian bitops have no arch-specific optimisations.

Remove clashing macros from these headers.

Signed-off-by Ben Hutchings <[email protected]>
---
 include/asm-generic/bitops/le.h |    1 -
 include/asm-powerpc/bitops.h    |   34 +---------------------------------
 2 files changed, 1 insertions(+), 34 deletions(-)
include/asm-generic/bitops/le.h +0 -1
index a51c4ca..08c5df3 100644
@@ -4,7 +4,6 @@
 #include <asm/types.h>
 #include <asm/byteorder.h>
 
-#define BITOP_WORD(nr)		((nr) / BITS_PER_LONG)
 #define BITOP_LE_SWIZZLE	((BITS_PER_LONG-1) & ~0x7)
 
 #if defined(__LITTLE_ENDIAN)
include/asm-powerpc/bitops.h +1 -33
index dcbf9a8..afe2fa3 100644
@@ -54,7 +54,6 @@
 
 #define BITOP_MASK(nr)		(1UL << ((nr) % BITS_PER_LONG))
 #define BITOP_WORD(nr)		((nr) / BITS_PER_LONG)
-#define BITOP_LE_SWIZZLE	((BITS_PER_LONG-1) & ~0x7)
 
 static __inline__ void set_bit(int nr, volatile unsigned long *addr)
 {

@@ -340,39 +339,8 @@ static __inline__ int fls64(__u64 x)
 
 /* Little-endian versions */
 
-static __inline__ int test_le_bit(unsigned long nr,
-				  __const__ unsigned long *addr)
-{
-	__const__ unsigned char	*tmp = (__const__ unsigned char *) addr;
-	return (tmp[nr >> 3] >> (nr & 7)) & 1;
-}
+#include <asm-generic/bitops/le.h>
 
-#define __set_le_bit(nr, addr) \
-	__set_bit((nr) ^ BITOP_LE_SWIZZLE, (addr))
-#define __clear_le_bit(nr, addr) \
-	__clear_bit((nr) ^ BITOP_LE_SWIZZLE, (addr))
-
-#define test_and_set_le_bit(nr, addr) \
-	test_and_set_bit((nr) ^ BITOP_LE_SWIZZLE, (addr))
-#define test_and_clear_le_bit(nr, addr) \
-	test_and_clear_bit((nr) ^ BITOP_LE_SWIZZLE, (addr))
-
-#define __test_and_set_le_bit(nr, addr) \
-	__test_and_set_bit((nr) ^ BITOP_LE_SWIZZLE, (addr))
-#define __test_and_clear_le_bit(nr, addr) \
-	__test_and_clear_bit((nr) ^ BITOP_LE_SWIZZLE, (addr))
-
-#define find_first_zero_le_bit(addr, size) \
-	generic_find_next_zero_le_bit(addr, size, 0)
-#define find_next_zero_le_bit(addr, size, offset) \
-	generic_find_next_zero_le_bit(addr, size, offset)
-unsigned long generic_find_next_zero_le_bit(const unsigned long *addr,
-				    unsigned long size, unsigned long offset);
-
-#define find_next_le_bit(addr, size, offset) \
-	generic_find_next_le_bit(addr, size, offset)
-unsigned long generic_find_next_le_bit(const unsigned long *addr,
-				    unsigned long size, unsigned long offset);
 /* Bitmap functions for the ext2 filesystem */
 
 #define ext2_set_bit(nr,addr) \
Signature · 1 line
1.5.4
#3 lore raw

Re: [PATCH 2/2] Use <asm-generic/bitops/le.h> in <asm-powerpc/bitops.h>

Hi,

Quoted 35 lines · Ben Hutchings

The powerpc little-endian bitops have no arch-specific optimisations.

Remove clashing macros from these headers.

Signed-off-by Ben Hutchings <[email protected]>
---
 include/asm-generic/bitops/le.h |    1 -
 include/asm-powerpc/bitops.h    |   34 +---------------------------------
 2 files changed, 1 insertions(+), 34 deletions(-)
include/asm-generic/bitops/le.h +0 -1
index a51c4ca..08c5df3 100644
@@ -4,7 +4,6 @@
 #include <asm/types.h>
 #include <asm/byteorder.h>
 
-#define BITOP_WORD(nr)         ((nr) / BITS_PER_LONG)
 #define BITOP_LE_SWIZZLE       ((BITS_PER_LONG-1) & ~0x7)
 
 #if defined(__LITTLE_ENDIAN)
include/asm-powerpc/bitops.h +0 -1
index dcbf9a8..afe2fa3 100644
@@ -54,7 +54,6 @@
 
 #define BITOP_MASK(nr)         (1UL << ((nr) % BITS_PER_LONG))
 #define BITOP_WORD(nr)         ((nr) / BITS_PER_LONG)
-#define BITOP_LE_SWIZZLE       ((BITS_PER_LONG-1) & ~0x7)
 
 static __inline__ void set_bit(int nr, volatile unsigned long *addr)
 {

include/asm-powerpc/bitops.h static __inline__ int fls64(__u64 x) · line 342

Quoted 10 lines
include/asm-powerpc/bitops.h +1 -6
@@ -340,39 +339,8 @@ static __inline__ int fls64(__u64 x)
 
 /* Little-endian versions */
 
-static __inline__ int test_le_bit(unsigned long nr,
-                                 __const__ unsigned long *addr)
-{
-       __const__ unsigned char *tmp = (__const__ unsigned char *) addr;
-       return (tmp[nr >> 3] >> (nr & 7)) & 1;
-}
+#include <asm-generic/bitops/le.h>

Is it completely impossible to move this #include to the top of the file?

I know that a lot of the current headers don't do this, and I don't think it's a written rule with the kernel sources, BUT it's a nice convention IMHO, and makes headers generally more maintainable. What do you think?

Vegard

Signature · 4 lines
"The animistic metaphor of the bug that maliciously sneaked in while
the programmer was not looking is intellectually dishonest as it
disguises that the error is the programmer's own creation."
	-- E. W. Dijkstra, EWD1036
author
#4 lore raw

Re: [PATCH 2/2] Use <asm-generic/bitops/le.h> in <asm-powerpc/bitops.h>

Quoted 42 lines · Vegard Nossum

Hi,

Quoted 38 lines · Ben Hutchings

The powerpc little-endian bitops have no arch-specific optimisations.

Remove clashing macros from these headers.

Signed-off-by Ben Hutchings <[email protected]>
---
 include/asm-generic/bitops/le.h |    1 -
 include/asm-powerpc/bitops.h    |   34 +---------------------------------
 2 files changed, 1 insertions(+), 34 deletions(-)
include/asm-generic/bitops/le.h +0 -1
index a51c4ca..08c5df3 100644
@@ -4,7 +4,6 @@
 #include <asm/types.h>
 #include <asm/byteorder.h>
 
-#define BITOP_WORD(nr)         ((nr) / BITS_PER_LONG)
 #define BITOP_LE_SWIZZLE       ((BITS_PER_LONG-1) & ~0x7)
 
 #if defined(__LITTLE_ENDIAN)
include/asm-powerpc/bitops.h +0 -1
index dcbf9a8..afe2fa3 100644
@@ -54,7 +54,6 @@
 
 #define BITOP_MASK(nr)         (1UL << ((nr) % BITS_PER_LONG))
 #define BITOP_WORD(nr)         ((nr) / BITS_PER_LONG)
-#define BITOP_LE_SWIZZLE       ((BITS_PER_LONG-1) & ~0x7)
 
 static __inline__ void set_bit(int nr, volatile unsigned long *addr)
 {

@@ -340,39 +339,8 @@ static __inline__ int fls64(__u64 x)
 
 /* Little-endian versions */
Quoted 9 lines

include/asm-powerpc/bitops.h static __inline__ int fls64(__u64 x) · line 342

Quoted 7 lines
include/asm-powerpc/bitops.h +1 -6
-static __inline__ int test_le_bit(unsigned long nr,
-                                 __const__ unsigned long *addr)
-{
-       __const__ unsigned char *tmp = (__const__ unsigned char *) addr;
-       return (tmp[nr >> 3] >> (nr & 7)) & 1;
-}
+#include <asm-generic/bitops/le.h>

Is it completely impossible to move this #include to the top of the file?

It's probably entirely possible.

Quoted 4 lines

I know that a lot of the current headers don't do this, and I don't think it's a written rule with the kernel sources, BUT it's a nice convention IMHO, and makes headers generally more maintainable. What do you think?

If you look at the current version of this header you'll see it includes several other bitops headers at around this point. I tend to follow the conventions I see.

Ben.

Signature · 2 lines
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
#5 lore raw
In reply to Ben Hutchings
Quoted 6 lines · Ben Hutchings

All architectures already have little-endian bitops for use by file- systems that use little-endian allocation tables. Since these are more generally useful - Andrew Morton suggested we use them in the sfc driver - they should be exposed under consistent names. The following patch series does that, using the names from <asm-generic/bitops/le.h> but without the "generic_" prefix.

Good idea. But please also kill the ext2_ and minix_ prefixed aliases while you're at it so that everyone uses the le_ prefixed ones.