2008-08-13 00:32:49

by Harvey Harrison

[permalink] [raw]
Subject: [PATCH 19/22] x86: use the new byteorder headers

Signed-off-by: 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(-)

diff --git a/include/asm-x86/byteorder.h b/include/asm-x86/byteorder.h
index e02ae2d..7187b3a 100644
--- a/include/asm-x86/byteorder.h
+++ b/include/asm-x86/byteorder.h
@@ -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 */
--
1.6.0.rc2.233.g3cb9d


2008-08-13 03:55:24

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH 19/22] x86: use the new byteorder headers

On Tue, 12 Aug 2008 17:27:14 -0700 Harvey Harrison <[email protected]> wrote:

> Signed-off-by: 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(-)
>
> diff --git a/include/asm-x86/byteorder.h b/include/asm-x86/byteorder.h
> index e02ae2d..7187b3a 100644
> --- a/include/asm-x86/byteorder.h
> +++ b/include/asm-x86/byteorder.h
> @@ -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 */

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.

2008-08-13 04:06:00

by Harvey Harrison

[permalink] [raw]
Subject: [PATCH] tipc: Use the appropriate swab helper rather than an internal helper

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

Signed-off-by: Harvey Harrison <[email protected]>
---
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(-)

diff --git a/net/tipc/subscr.c b/net/tipc/subscr.c
index 0326d30..f5da05d 100644
--- a/net/tipc/subscr.c
+++ b/net/tipc/subscr.c
@@ -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;
}

/**
--
1.6.0.rc2.233.g3cb9d


2008-08-13 04:20:49

by Andrew Morton

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

On Tue, 12 Aug 2008 21:05:47 -0700 Harvey Harrison <[email protected]> wrote:

> ___generally means I am an internal implementation detail, don't use
> me.
>
> Signed-off-by: Harvey Harrison <[email protected]>
> ---
> 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(-)
>
> diff --git a/net/tipc/subscr.c b/net/tipc/subscr.c
> index 0326d30..f5da05d 100644
> --- a/net/tipc/subscr.c
> +++ b/net/tipc/subscr.c
> @@ -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.

2008-08-13 04:24:17

by Harvey Harrison

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

On Tue, 2008-08-12 at 21:20 -0700, Andrew Morton wrote:
> On Tue, 12 Aug 2008 21:05:47 -0700 Harvey Harrison <[email protected]> wrote:
>
> > ___generally means I am an internal implementation detail, don't use
> > me.
> >
> > Signed-off-by: Harvey Harrison <[email protected]>
> > ---
> > 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(-)
> >
> > diff --git a/net/tipc/subscr.c b/net/tipc/subscr.c
> > index 0326d30..f5da05d 100644
> > --- a/net/tipc/subscr.c
> > +++ b/net/tipc/subscr.c
> > @@ -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.

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

2008-08-13 05:56:35

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH 19/22] x86: use the new byteorder headers


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?

2008-08-13 05:59:34

by David Miller

[permalink] [raw]
Subject: Re: [PATCH 19/22] x86: use the new byteorder headers

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

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

2008-08-13 06:02:28

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH 19/22] x86: use the new byteorder headers

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

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

2008-08-13 06:05:32

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH 19/22] x86: use the new byteorder headers

On Tue, 12 Aug 2008 22:59:24 -0700 (PDT) David Miller <[email protected]> wrote:

> From: Andrew Morton <[email protected]>
> Date: Tue, 12 Aug 2008 22:56:12 -0700
>
> > 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. :-/

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

2008-08-13 06:07:25

by Harvey Harrison

[permalink] [raw]
Subject: Re: [PATCH 19/22] x86: use the new byteorder headers

On Tue, 2008-08-12 at 23:02 -0700, Andrew Morton wrote:
> On Tue, 12 Aug 2008 22:56:12 -0700 Andrew Morton <[email protected]> wrote:
>
> > 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

2008-08-13 06:47:58

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH 19/22] x86: use the new byteorder headers

On Tue, 12 Aug 2008 23:07:15 -0700 Harvey Harrison <[email protected]> wrote:

> On Tue, 2008-08-12 at 23:02 -0700, Andrew Morton wrote:
> > On Tue, 12 Aug 2008 22:56:12 -0700 Andrew Morton <[email protected]> wrote:
> >
> > > 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.

2008-08-13 09:32:43

by David Miller

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

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

> On Tue, 12 Aug 2008 21:05:47 -0700 Harvey Harrison <[email protected]> wrote:
>
> > ___generally means I am an internal implementation detail, don't use
> > me.
> >
> > Signed-off-by: Harvey Harrison <[email protected]>
> > ---
> > 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(-)
> >
> > diff --git a/net/tipc/subscr.c b/net/tipc/subscr.c
> > index 0326d30..f5da05d 100644
> > --- a/net/tipc/subscr.c
> > +++ b/net/tipc/subscr.c
> > @@ -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.

I've applied Andrew's patch.