Skip to contentSkip to conversation

x86: use the new byteorder headers

PATCH In review Harvey Harrison 3 participants 12 messages Aug 13, 2008

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 19/22 x86: use the new byteorder headers

---
The prerequisite patches have landed in Linus' tree now.

 include/asm-x86/byteorder.h |   69 +++++++++++++++++--------------------------
 1 files changed, 27 insertions(+), 42 deletions(-)
include/asm-x86/byteorder.h +27 -42
index e02ae2d..7187b3a 100644
@@ -4,26 +4,33 @@
 #include <asm/types.h>
 #include <linux/compiler.h>
 
-#ifdef __GNUC__
+#define __LITTLE_ENDIAN
 
-#ifdef __i386__
-
-static inline __attribute_const__ __u32 ___arch__swab32(__u32 x)
+static inline __attribute_const__ __u32 __arch_swab32(__u32 val)
 {
-#ifdef CONFIG_X86_BSWAP
-	asm("bswap %0" : "=r" (x) : "0" (x));
-#else
+#ifdef __i386__
+# ifdef CONFIG_X86_BSWAP
+	asm("bswap %0" : "=r" (val) : "0" (val));
+# else
 	asm("xchgb %b0,%h0\n\t"	/* swap lower bytes	*/
 	    "rorl $16,%0\n\t"	/* swap words		*/
 	    "xchgb %b0,%h0"	/* swap higher bytes	*/
-	    : "=q" (x)
-	    : "0" (x));
+	    : "=q" (val)
+	    : "0" (val));
+# endif
+
+#else /* __i386__ */
+	asm("bswapl %0"
+	    : "=r" (val)
+	    : "0" (val));
 #endif
-	return x;
+	return val;
 }
+#define __arch_swab32 __arch_swab32
 
-static inline __attribute_const__ __u64 ___arch__swab64(__u64 val)
+static inline __attribute_const__ __u64 __arch_swab64(__u64 val)
 {
+#ifdef __i386__
 	union {
 		struct {
 			__u32 a;
@@ -32,50 +39,28 @@ static inline __attribute_const__ __u64 ___arch__swab64(__u64 val)
 		__u64 u;
 	} v;
 	v.u = val;
-#ifdef CONFIG_X86_BSWAP
+# ifdef CONFIG_X86_BSWAP
 	asm("bswapl %0 ; bswapl %1 ; xchgl %0,%1"
 	    : "=r" (v.s.a), "=r" (v.s.b)
 	    : "0" (v.s.a), "1" (v.s.b));
-#else
+# else
 	v.s.a = ___arch__swab32(v.s.a);
 	v.s.b = ___arch__swab32(v.s.b);
 	asm("xchgl %0,%1"
 	    : "=r" (v.s.a), "=r" (v.s.b)
 	    : "0" (v.s.a), "1" (v.s.b));
-#endif
+# endif
 	return v.u;
-}
 
 #else /* __i386__ */
-
-static inline __attribute_const__ __u64 ___arch__swab64(__u64 x)
-{
 	asm("bswapq %0"
-	    : "=r" (x)
-	    : "0" (x));
-	return x;
-}
-
-static inline __attribute_const__ __u32 ___arch__swab32(__u32 x)
-{
-	asm("bswapl %0"
-	    : "=r" (x)
-	    : "0" (x));
-	return x;
-}
-
+	    : "=r" (val)
+	    : "0" (val));
+	return val;
 #endif
+}
+#define __arch_swab64 __arch_swab64
 
-/* Do not define swab16.  Gcc is smart enough to recognize "C" version and
-   convert it into rotation or exhange.  */
-
-#define __arch__swab64(x) ___arch__swab64(x)
-#define __arch__swab32(x) ___arch__swab32(x)
-
-#define __BYTEORDER_HAS_U64__
-
-#endif /* __GNUC__ */
-
-#include <linux/byteorder/little_endian.h>
+#include <linux/byteorder.h>
 
 #endif /* _ASM_X86_BYTEORDER_H */
Signature · 1 line
1.6.0.rc2.233.g3cb9d
#1 lore raw
Quoted 106 lines · On Tue, 12 Aug 2008 17:27:14 -0700 Harvey Harrison <[email protected]>…

---
The prerequisite patches have landed in Linus' tree now.

 include/asm-x86/byteorder.h |   69 +++++++++++++++++--------------------------
 1 files changed, 27 insertions(+), 42 deletions(-)
include/asm-x86/byteorder.h +26 -35
index e02ae2d..7187b3a 100644
@@ -4,26 +4,33 @@
 #include <asm/types.h>
 #include <linux/compiler.h>
 
-#ifdef __GNUC__
+#define __LITTLE_ENDIAN
 
-#ifdef __i386__
-
-static inline __attribute_const__ __u32 ___arch__swab32(__u32 x)
+static inline __attribute_const__ __u32 __arch_swab32(__u32 val)
 {
-#ifdef CONFIG_X86_BSWAP
-	asm("bswap %0" : "=r" (x) : "0" (x));
-#else
+#ifdef __i386__
+# ifdef CONFIG_X86_BSWAP
+	asm("bswap %0" : "=r" (val) : "0" (val));
+# else
 	asm("xchgb %b0,%h0\n\t"	/* swap lower bytes	*/
 	    "rorl $16,%0\n\t"	/* swap words		*/
 	    "xchgb %b0,%h0"	/* swap higher bytes	*/
-	    : "=q" (x)
-	    : "0" (x));
+	    : "=q" (val)
+	    : "0" (val));
+# endif
+
+#else /* __i386__ */
+	asm("bswapl %0"
+	    : "=r" (val)
+	    : "0" (val));
 #endif
-	return x;
+	return val;
 }
+#define __arch_swab32 __arch_swab32
 
-static inline __attribute_const__ __u64 ___arch__swab64(__u64 val)
+static inline __attribute_const__ __u64 __arch_swab64(__u64 val)
 {
+#ifdef __i386__
 	union {
 		struct {
 			__u32 a;
@@ -32,50 +39,28 @@ static inline __attribute_const__ __u64 ___arch__swab64(__u64 val)
 		__u64 u;
 	} v;
 	v.u = val;
-#ifdef CONFIG_X86_BSWAP
+# ifdef CONFIG_X86_BSWAP
 	asm("bswapl %0 ; bswapl %1 ; xchgl %0,%1"
 	    : "=r" (v.s.a), "=r" (v.s.b)
 	    : "0" (v.s.a), "1" (v.s.b));
-#else
+# else
 	v.s.a = ___arch__swab32(v.s.a);
 	v.s.b = ___arch__swab32(v.s.b);
 	asm("xchgl %0,%1"
 	    : "=r" (v.s.a), "=r" (v.s.b)
 	    : "0" (v.s.a), "1" (v.s.b));
-#endif
+# endif
 	return v.u;
-}
 
 #else /* __i386__ */
-
-static inline __attribute_const__ __u64 ___arch__swab64(__u64 x)
-{
 	asm("bswapq %0"
-	    : "=r" (x)
-	    : "0" (x));
-	return x;
-}
-
-static inline __attribute_const__ __u32 ___arch__swab32(__u32 x)
-{
-	asm("bswapl %0"
-	    : "=r" (x)
-	    : "0" (x));
-	return x;
-}
-
+	    : "=r" (val)
+	    : "0" (val));
+	return val;
 #endif
+}
+#define __arch_swab64 __arch_swab64
 
-/* Do not define swab16.  Gcc is smart enough to recognize "C" version and
-   convert it into rotation or exhange.  */
-
-#define __arch__swab64(x) ___arch__swab64(x)

include/asm-x86/byteorder.h static inline __attribute_const__ __u64 ___arch__swab64(__u64 val) · line 66

Quoted 10 lines
include/asm-x86/byteorder.h +1 -7
-#define __arch__swab32(x) ___arch__swab32(x)
-
-#define __BYTEORDER_HAS_U64__
-
-#endif /* __GNUC__ */
-
-#include <linux/byteorder/little_endian.h>
+#include <linux/byteorder.h>
 
 #endif /* _ASM_X86_BYTEORDER_H */

net/tipc/subscr.c: In function 'htohl':
net/tipc/subscr.c:88: error: implicit declaration of function '___constant_swab32'

That's just a basic i386 allmodconfig. How come you're not seeing this?

Heaven alone knows what tipc thinks it's doing poking into this sort of thing. Could you please take a look and teach it some manners?

Thanks.

#2 lore raw

PATCH 1/1 tipc: Use the appropriate swab helper rather than an internal helper

___generally means I am an internal implementation detail, don't use me.

---
Sorry for breaking the build Andrew, my testing has been with all the __constant versions in-tree removed.

 net/tipc/subscr.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
net/tipc/subscr.c +1 -1
index 0326d30..f5da05d 100644
@@ -85,7 +85,7 @@ static struct top_srv topsrv = { 0 };
 
 static u32 htohl(u32 in, int swap)
 {
-	return swap ? (u32)___constant_swab32(in) : in;
+	return swap ? (u32)swab32(in) : in;
 }
 
 /**
Signature · 1 line
1.6.0.rc2.233.g3cb9d
#3 lore raw

Re: [PATCH] tipc: Use the appropriate swab helper rather than an internal helper

Quoted 16 lines · On Tue, 12 Aug 2008 21:05:47 -0700 Harvey Harrison <[email protected]>…

___generally means I am an internal implementation detail, don't use me.

---
Sorry for breaking the build Andrew, my testing has been with all the __constant versions in-tree removed.

 net/tipc/subscr.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
net/tipc/subscr.c +0 -0
index 0326d30..f5da05d 100644

net/tipc/subscr.c static struct top_srv topsrv = { 0 }; · line 89

Quoted 6 lines
net/tipc/subscr.c +1 -1
@@ -85,7 +85,7 @@ static struct top_srv topsrv = { 0 };
 
 static u32 htohl(u32 in, int swap)
 {
-	return swap ? (u32)___constant_swab32(in) : in;
+	return swap ? (u32)swab32(in) : in;
 }

yes, that's what I did too, only I dropped the
seems-to-me-to-be-unneeded (u32) cast.

#4 lore raw

Re: [PATCH] tipc: Use the appropriate swab helper rather than an internal helper

Quoted 19 lines · Andrew Morton
Quoted 17 lines · On Tue, 12 Aug 2008 21:05:47 -0700 Harvey Harrison <[email protected]>…

___generally means I am an internal implementation detail, don't use me.

---
Sorry for breaking the build Andrew, my testing has been with all the __constant versions in-tree removed.

 net/tipc/subscr.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
net/tipc/subscr.c +0 -0
index 0326d30..f5da05d 100644
@@ -85,7 +85,7 @@ static struct top_srv topsrv = { 0 };
 
Quoted 8 lines

net/tipc/subscr.c static struct top_srv topsrv = { 0 }; · line 89

Quoted 5 lines
net/tipc/subscr.c +1 -1
 static u32 htohl(u32 in, int swap)
 {
-	return swap ? (u32)___constant_swab32(in) : in;
+	return swap ? (u32)swab32(in) : in;
 }

yes, that's what I did too, only I dropped the
seems-to-me-to-be-unneeded (u32) cast.

I made the mistake of folding this bit into the series that removes all users of the __constant_ helpers as I forgot it was a build breaker. That's why I wasn't seeing it here.

Harvey

maintainer
#5 lore raw

Re: [PATCH] tipc: Use the appropriate swab helper rather than an internal helper

In reply to Andrew Morton

From: Andrew Morton <[email protected]>
Date: Tue, 12 Aug 2008 21:20:17 -0700

Quoted 19 lines · On Tue, 12 Aug 2008 21:05:47 -0700 Harvey Harrison <[email protected]>…
Quoted 18 lines

___generally means I am an internal implementation detail, don't use me.

---
Sorry for breaking the build Andrew, my testing has been with all the __constant versions in-tree removed.

 net/tipc/subscr.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
net/tipc/subscr.c +0 -0
index 0326d30..f5da05d 100644
@@ -85,7 +85,7 @@ static struct top_srv topsrv = { 0 };
 
Quoted 8 lines

net/tipc/subscr.c static struct top_srv topsrv = { 0 }; · line 89

Quoted 5 lines
net/tipc/subscr.c +1 -1
 static u32 htohl(u32 in, int swap)
 {
-	return swap ? (u32)___constant_swab32(in) : in;
+	return swap ? (u32)swab32(in) : in;
 }

yes, that's what I did too, only I dropped the
seems-to-me-to-be-unneeded (u32) cast.

I've applied Andrew's patch.

#6 lore raw
In reply to Harvey Harrison

akpm2:/usr/src/25> make arch/x86/boot/memory.o

  CHK     include/linux/version.h
  CHK     include/linux/utsrelease.h
  CC      arch/x86/kernel/asm-offsets.s
  GEN     include/asm/asm-offsets.h
  CALL    scripts/checksyscalls.sh
  CC      arch/x86/boot/memory.o
In file included from include/asm-generic/bitops/le.h:5,
                 from include/asm-generic/bitops/ext2-non-atomic.h:4,
                 from include/asm/bitops.h:451,
                 from include/linux/bitops.h:17,
                 from include/linux/kernel.h:15,
                 from arch/x86/boot/memory.c:16:

include/asm/byteorder.h: In function '__arch_swab64':
include/asm/byteorder.h:47: warning: implicit declaration of function '___arch__swab32'

I'll drop them all. Do you have a cross-compiler suite there?

maintainer
#7 lore raw

From: Andrew Morton <[email protected]>
Date: Tue, 12 Aug 2008 22:56:12 -0700

Quoted 1 line

I'll drop them all. Do you have a cross-compiler suite there?

I think Harvey needs a mentor which can help him learn how to properly build validate his patches before he submits them, and how to not mix things up by, for example, validating the build with patches applied that won't be submitted etc.

Every time I integrate these byteorder patches directly for networking or sparc, it tends to be a build regression nightmare.

I think Andrew has hit 3 already with this patch series, quite an accomplishment. :-/

#8 lore raw
Quoted 10 lines · On Tue, 12 Aug 2008 22:59:24 -0700 (PDT) David Miller <[email protected]>…

From: Andrew Morton <[email protected]>
Date: Tue, 12 Aug 2008 22:56:12 -0700

Quoted 1 line

I'll drop them all. Do you have a cross-compiler suite there?

I think Harvey needs a mentor which can help him learn how to properly build validate his patches before he submits them, and how to not mix things up by, for example, validating the build with patches applied that won't be submitted etc.

Quoted 5 lines

Every time I integrate these byteorder patches directly for networking or sparc, it tends to be a build regression nightmare.

I think Andrew has hit 3 already with this patch series, quite an accomplishment. :-/

heh. This is why I habitually wait until Harveypatches hit version 4. Maybe I need to increment that a bit.

#9 lore raw
In reply to Andrew Morton
Quoted 1 line · On Tue, 12 Aug 2008 22:56:12 -0700 Andrew Morton <[email protected]>…

I'll drop them all.

btw, I had to rework several of these patches because everyone is moving include/asm-arch/* into arch/include/asm/. Patches against linux-next would be preferred please.

#10 lore raw
Quoted 8 lines · Andrew Morton
Quoted 1 line · On Tue, 12 Aug 2008 22:56:12 -0700 Andrew Morton <[email protected]>…

I'll drop them all.

btw, I had to rework several of these patches because everyone is moving include/asm-arch/* into arch/include/asm/. Patches against linux-next would be preferred please.

Which arches? These were all against linux-next20080811.

Harvey

#11 lore raw
Quoted 12 lines · On Tue, 12 Aug 2008 23:07:15 -0700 Harvey Harrison <[email protected]>…
Quoted 8 lines · Andrew Morton
Quoted 1 line · On Tue, 12 Aug 2008 22:56:12 -0700 Andrew Morton <[email protected]>…

I'll drop them all.

btw, I had to rework several of these patches because everyone is moving include/asm-arch/* into arch/include/asm/. Patches against linux-next would be preferred please.

Which arches? These were all against linux-next20080811.

The ones which got changed in the past 24 hours I guess ;)

avr32 and xtensa, I think.

Ah, yes, I seem to have picked some trees which aren't in linux-next for whatever reason. See mmotm's git-parisc.patch and git-xtensa.patch.