[PATCH 0/2] Expose little-endian bitops

[PATCH 0/2] Expose little-endian bitops

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

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.

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

[Ben Hutchings]

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 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(-)

diff --git a/include/asm-generic/bitops/ext2-non-atomic.h b/include/asm-generic/bitops/ext2-non-atomic.h
index 63cf822..b9281c2 100644
--- a/include/asm-generic/bitops/ext2-non-atomic.h
+++ b/include/asm-generic/bitops/ext2-non-atomic.h
@@ -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_ */
diff --git a/include/asm-generic/bitops/le.h b/include/asm-generic/bitops/le.h
index 80e3bf1..a51c4ca 100644
--- a/include/asm-generic/bitops/le.h
+++ b/include/asm-generic/bitops/le.h
@@ -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_ */
diff --git a/include/asm-generic/bitops/minix-le.h b/include/asm-generic/bitops/minix-le.h
index 4a981c1..da3613d 100644
--- a/include/asm-generic/bitops/minix-le.h
+++ b/include/asm-generic/bitops/minix-le.h
@@ -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_ */
diff --git a/include/asm-m68k/bitops.h b/include/asm-m68k/bitops.h
index 83d1f28..27d7ae9 100644
--- a/include/asm-m68k/bitops.h
+++ b/include/asm-m68k/bitops.h
@@ -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__ */
 
diff --git a/include/asm-m68knommu/bitops.h b/include/asm-m68knommu/bitops.h
index c142fbf..6213a08 100644
--- a/include/asm-m68knommu/bitops.h
+++ b/include/asm-m68knommu/bitops.h
@@ -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__ */
diff --git a/include/asm-powerpc/bitops.h b/include/asm-powerpc/bitops.h
index 897eade..dcbf9a8 100644
--- a/include/asm-powerpc/bitops.h
+++ b/include/asm-powerpc/bitops.h
@@ -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 */

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.

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

[Ben Hutchings]

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

Remove clashing macros from these headers.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 include/asm-generic/bitops/le.h |    1 -
 include/asm-powerpc/bitops.h    |   34 +---------------------------------
 2 files changed, 1 insertions(+), 34 deletions(-)

diff --git a/include/asm-generic/bitops/le.h b/include/asm-generic/bitops/le.h
index a51c4ca..08c5df3 100644
--- a/include/asm-generic/bitops/le.h
+++ b/include/asm-generic/bitops/le.h
@@ -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)
diff --git a/include/asm-powerpc/bitops.h b/include/asm-powerpc/bitops.h
index dcbf9a8..afe2fa3 100644
--- a/include/asm-powerpc/bitops.h
+++ b/include/asm-powerpc/bitops.h
@@ -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) \
-- 
1.5.4

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

[Vegard Nossum]

Hi,

On Sun, Jun 1, 2008 at 12:38 AM, Ben Hutchings
<bhutchings@solarflare.com> wrote:
> The powerpc little-endian bitops have no arch-specific optimisations.
>
> Remove clashing macros from these headers.
>
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> ---
>  include/asm-generic/bitops/le.h |    1 -
>  include/asm-powerpc/bitops.h    |   34 +---------------------------------
>  2 files changed, 1 insertions(+), 34 deletions(-)
>
> diff --git a/include/asm-generic/bitops/le.h b/include/asm-generic/bitops/le.h
> index a51c4ca..08c5df3 100644
> --- a/include/asm-generic/bitops/le.h
> +++ b/include/asm-generic/bitops/le.h
> @@ -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)
> diff --git a/include/asm-powerpc/bitops.h b/include/asm-powerpc/bitops.h
> index dcbf9a8..afe2fa3 100644
> --- a/include/asm-powerpc/bitops.h
> +++ b/include/asm-powerpc/bitops.h
> @@ -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>

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

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

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

[Ben Hutchings]

Vegard Nossum wrote:
> Hi,
> 
> On Sun, Jun 1, 2008 at 12:38 AM, Ben Hutchings
> <bhutchings@solarflare.com> wrote:
> > The powerpc little-endian bitops have no arch-specific optimisations.
> >
> > Remove clashing macros from these headers.
> >
> > Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> > ---
> >  include/asm-generic/bitops/le.h |    1 -
> >  include/asm-powerpc/bitops.h    |   34 +---------------------------------
> >  2 files changed, 1 insertions(+), 34 deletions(-)
> >
> > diff --git a/include/asm-generic/bitops/le.h b/include/asm-generic/bitops/le.h
> > index a51c4ca..08c5df3 100644
> > --- a/include/asm-generic/bitops/le.h
> > +++ b/include/asm-generic/bitops/le.h
> > @@ -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)
> > diff --git a/include/asm-powerpc/bitops.h b/include/asm-powerpc/bitops.h
> > index dcbf9a8..afe2fa3 100644
> > --- a/include/asm-powerpc/bitops.h
> > +++ b/include/asm-powerpc/bitops.h
> > @@ -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>
> 
> Is it completely impossible to move this #include to the top of the file?

It's probably entirely possible.

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

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.

Re: [PATCH 0/2] Expose little-endian bitops

[Christoph Hellwig]

On Sat, May 31, 2008 at 11:34:20PM +0100, Ben Hutchings wrote:
> 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.