2008-11-05 18:38:50

by Harvey Harrison

[permalink] [raw]
Subject: [PATCH 10/10] xtensa: use the new byteorder headers

Signed-off-by: Harvey Harrison <[email protected]>
---
include/asm-xtensa/byteorder.h | 32 +++++++++++++++-----------------
1 files changed, 15 insertions(+), 17 deletions(-)

diff --git a/include/asm-xtensa/byteorder.h b/include/asm-xtensa/byteorder.h
index 765edf1..07d10ad 100644
--- a/include/asm-xtensa/byteorder.h
+++ b/include/asm-xtensa/byteorder.h
@@ -14,7 +14,17 @@
#include <asm/types.h>
#include <linux/compiler.h>

-static __inline__ __attribute_const__ __u32 ___arch__swab32(__u32 x)
+#ifdef __XTENSA_EL__
+# define __LITTLE_ENDIAN
+#elif defined(__XTENSA_EB__)
+# define __BIG_ENDIAN
+#else
+# error processor byte order undefined!
+#endif
+
+#define __SWAB_64_THRU_32__
+
+static inline __attribute_const__ __u32 __arch_swab32(__u32 x)
{
__u32 res;
/* instruction sequence from Xtensa ISA release 2/2000 */
@@ -28,8 +38,9 @@ static __inline__ __attribute_const__ __u32 ___arch__swab32(__u32 x)
);
return res;
}
+#define __arch_swab32 __arch_swab32

-static __inline__ __attribute_const__ __u16 ___arch__swab16(__u16 x)
+static inline __attribute_const__ __u16 __arch_swab16(__u16 x)
{
/* Given that 'short' values are signed (i.e., can be negative),
* we cannot assume that the upper 16-bits of the register are
@@ -62,21 +73,8 @@ static __inline__ __attribute_const__ __u16 ___arch__swab16(__u16 x)

return res;
}
+#define __arch_swab16 __arch_swab16

-#define __arch__swab32(x) ___arch__swab32(x)
-#define __arch__swab16(x) ___arch__swab16(x)
-
-#if !defined(__STRICT_ANSI__) || defined(__KERNEL__)
-# define __BYTEORDER_HAS_U64__
-# define __SWAB_64_THRU_32__
-#endif
-
-#ifdef __XTENSA_EL__
-# include <linux/byteorder/little_endian.h>
-#elif defined(__XTENSA_EB__)
-# include <linux/byteorder/big_endian.h>
-#else
-# error processor byte order undefined!
-#endif
+#include <linux/byteorder.h>

#endif /* _XTENSA_BYTEORDER_H */
--
1.6.0.3.756.gb776d


2008-11-06 17:59:08

by Chris Zankel

[permalink] [raw]
Subject: Re: [PATCH 10/10] xtensa: use the new byteorder headers

Hi Harvey,

I have added your patch to the xtensa-next tree on kernel.org and will
push to Linus soon.

Thanks,
-Chris

Harvey Harrison wrote:
> Signed-off-by: Harvey Harrison <[email protected]>
> ---
> include/asm-xtensa/byteorder.h | 32 +++++++++++++++-----------------
> 1 files changed, 15 insertions(+), 17 deletions(-)
>
> diff --git a/include/asm-xtensa/byteorder.h b/include/asm-xtensa/byteorder.h
> index 765edf1..07d10ad 100644
> --- a/include/asm-xtensa/byteorder.h
> +++ b/include/asm-xtensa/byteorder.h
> @@ -14,7 +14,17 @@
> #include <asm/types.h>
> #include <linux/compiler.h>
>
> -static __inline__ __attribute_const__ __u32 ___arch__swab32(__u32 x)
> +#ifdef __XTENSA_EL__
> +# define __LITTLE_ENDIAN
> +#elif defined(__XTENSA_EB__)
> +# define __BIG_ENDIAN
> +#else
> +# error processor byte order undefined!
> +#endif
> +
> +#define __SWAB_64_THRU_32__
> +
> +static inline __attribute_const__ __u32 __arch_swab32(__u32 x)
> {
> __u32 res;
> /* instruction sequence from Xtensa ISA release 2/2000 */
> @@ -28,8 +38,9 @@ static __inline__ __attribute_const__ __u32 ___arch__swab32(__u32 x)
> );
> return res;
> }
> +#define __arch_swab32 __arch_swab32
>
> -static __inline__ __attribute_const__ __u16 ___arch__swab16(__u16 x)
> +static inline __attribute_const__ __u16 __arch_swab16(__u16 x)
> {
> /* Given that 'short' values are signed (i.e., can be negative),
> * we cannot assume that the upper 16-bits of the register are
> @@ -62,21 +73,8 @@ static __inline__ __attribute_const__ __u16 ___arch__swab16(__u16 x)
>
> return res;
> }
> +#define __arch_swab16 __arch_swab16
>
> -#define __arch__swab32(x) ___arch__swab32(x)
> -#define __arch__swab16(x) ___arch__swab16(x)
> -
> -#if !defined(__STRICT_ANSI__) || defined(__KERNEL__)
> -# define __BYTEORDER_HAS_U64__
> -# define __SWAB_64_THRU_32__
> -#endif
> -
> -#ifdef __XTENSA_EL__
> -# include <linux/byteorder/little_endian.h>
> -#elif defined(__XTENSA_EB__)
> -# include <linux/byteorder/big_endian.h>
> -#else
> -# error processor byte order undefined!
> -#endif
> +#include <linux/byteorder.h>
>
> #endif /* _XTENSA_BYTEORDER_H */

2008-11-08 03:00:43

by Piet Delaney

[permalink] [raw]
Subject: Re: [PATCH 10/10] xtensa: use the new byteorder headers - Merged with your previous xtensa-next and will remerge shortly.

Hi Chris:

I've merged your recent xtensa-next with our 2.6.24-smp repo.
It seems to work fine and I'm in the process of cleaning it
up a bit and adding preliminary XTENSA kgdb support.

Looks like you have moved xtensa include files to arch/xtensa.
I'm pulling your repo and re-merge with your latest bits. I'm
limiting the changes outside of arch/xtensa to just the few
required (like the open core Ethernet driver) and a very small
change I made to mm/slab.c to allow the kernel to be compiled -O0.

One regression that came up after the merge with your previous
bits (2.6.27-rc3 last changed 2008-07-30) was that the NFS RPC
code wouldn't compile -O0 due to a problem with byte order macros
to convert IP addresses from local to net form. In this case it
appears to be the RPC code with it's initialization of a structure
with a htonl(INADDR_LOOPBACK) which isn't being optimized away.

"net/sunrpc/rpcb_clnt.c
-----------------------
static const struct sockaddr_in rpcb_inaddr_loopback = {
.sin_family = AF_INET,
.sin_addr.s_addr = htonl(INADDR_LOOPBACK),
.sin_port = htons(RPCBIND_PORT),
};

Looks like this convention was started by Chuck Lever on 7/14/2008 (Id:44).


Still tweaking and testing a major change to entry.S that Marc wrote
to allow gdb to backtrace across an exception. It's been extremely
valuable while working on problems like bringing up the kgdb stub.

-piet

> Hi Harvey,
>
> I have added your patch to the xtensa-next tree on kernel.org and will
> push to Linus soon.
>
> Thanks,
> -Chris
>
> Harvey Harrison wrote:
>> Signed-off-by: Harvey Harrison <[email protected]>
>> ---
>> include/asm-xtensa/byteorder.h | 32 +++++++++++++++-----------------
>> 1 files changed, 15 insertions(+), 17 deletions(-)
>>
>> diff --git a/include/asm-xtensa/byteorder.h
>> b/include/asm-xtensa/byteorder.h
>> index 765edf1..07d10ad 100644
>> --- a/include/asm-xtensa/byteorder.h
>> +++ b/include/asm-xtensa/byteorder.h
>> @@ -14,7 +14,17 @@
>> #include <asm/types.h>
>> #include <linux/compiler.h>
>>
>> -static __inline__ __attribute_const__ __u32 ___arch__swab32(__u32 x)
>> +#ifdef __XTENSA_EL__
>> +# define __LITTLE_ENDIAN
>> +#elif defined(__XTENSA_EB__)
>> +# define __BIG_ENDIAN
>> +#else
>> +# error processor byte order undefined!
>> +#endif
>> +
>> +#define __SWAB_64_THRU_32__
>> +
>> +static inline __attribute_const__ __u32 __arch_swab32(__u32 x)
>> {
>> __u32 res;
>> /* instruction sequence from Xtensa ISA release 2/2000 */
>> @@ -28,8 +38,9 @@ static __inline__ __attribute_const__ __u32
>> ___arch__swab32(__u32 x)
>> );
>> return res;
>> }
>> +#define __arch_swab32 __arch_swab32
>>
>> -static __inline__ __attribute_const__ __u16 ___arch__swab16(__u16 x)
>> +static inline __attribute_const__ __u16 __arch_swab16(__u16 x)
>> {
>> /* Given that 'short' values are signed (i.e., can be negative),
>> * we cannot assume that the upper 16-bits of the register are
>> @@ -62,21 +73,8 @@ static __inline__ __attribute_const__ __u16
>> ___arch__swab16(__u16 x)
>>
>> return res;
>> }
>> +#define __arch_swab16 __arch_swab16
>>
>> -#define __arch__swab32(x) ___arch__swab32(x)
>> -#define __arch__swab16(x) ___arch__swab16(x)
>> -
>> -#if !defined(__STRICT_ANSI__) || defined(__KERNEL__)
>> -# define __BYTEORDER_HAS_U64__
>> -# define __SWAB_64_THRU_32__
>> -#endif
>> -
>> -#ifdef __XTENSA_EL__
>> -# include <linux/byteorder/little_endian.h>
>> -#elif defined(__XTENSA_EB__)
>> -# include <linux/byteorder/big_endian.h>
>> -#else
>> -# error processor byte order undefined!
>> -#endif
>> +#include <linux/byteorder.h>
>>
>> #endif /* _XTENSA_BYTEORDER_H */
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>

2008-11-08 03:38:19

by Harvey Harrison

[permalink] [raw]
Subject: Re: [PATCH 10/10] xtensa: use the new byteorder headers - Merged with your previous xtensa-next and will remerge shortly.

On Fri, 2008-11-07 at 19:00 -0800, Piet Delaney wrote:
> Hi Chris:
>
> I've merged your recent xtensa-next with our 2.6.24-smp repo.
> It seems to work fine and I'm in the process of cleaning it
> up a bit and adding preliminary XTENSA kgdb support.

2.6.24? In that case you probably don't have include/linux/byteorder.h,
or include/linux/swab.h which would explain your byteorder problems.

Or is that a typo in the version numbers?

You should be able to just pull those two headers from Linus' tree
and drop them in, there is no dependencies other than those two
headers.

If it is some other problem, let me know and I'll look into it.

Harvey

2008-11-09 03:56:22

by Piet Delaney

[permalink] [raw]
Subject: Re: [PATCH 10/10] xtensa: use the new byteorder headers - Merged with your previous xtensa-next and will remerge shortly.

Harvey Harrison wrote:
> On Fri, 2008-11-07 at 19:00 -0800, Piet Delaney wrote:
>> Hi Chris:
>>
>> I've merged your recent xtensa-next with our 2.6.24-smp repo.
>> It seems to work fine and I'm in the process of cleaning it
>> up a bit and adding preliminary XTENSA kgdb support.
>
> 2.6.24? In that case you probably don't have include/linux/byteorder.h,
> or include/linux/swab.h which would explain your byteorder problems.
>
> Or is that a typo in the version numbers?

I was on 2.6.24 and didn't have a problem compiling the kernel -O0 other
than a minor tweak in slab.c. Now in 2.6.27-rc3 I get a compile problem
with rpcb_clnt.c at lines 122, 123, and 129:

/export/src/xtensa-next/net/sunrpc/rpcb_clnt.c:129: error: (near initialization for 'rpcb_in6addr_loopback.sin6_port')


>
> You should be able to just pull those two headers from Linus' tree
> and drop them in, there is no dependencies other than those two
> headers.

byteorder.h was already in sync with linus's tree and I updated swab.c
as it had a few changes. Unfortunately the problem persist. I think the
problem is likely in changes made to rpcb_clnt.c since 2.6.24.

>
> If it is some other problem, let me know and I'll look into it.

You can likely reproduce it by configuring NFS and compiling -O0.

-piet

>
> Harvey
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>

2008-11-14 08:14:31

by Piet Delaney

[permalink] [raw]
Subject: Re: [PATCH 10/10] xtensa: use the new byteorder headers - Merged with your previous xtensa-next and will remerge shortly.

Piet Delaney wrote:
> Harvey Harrison wrote:
>> On Fri, 2008-11-07 at 19:00 -0800, Piet Delaney wrote:
>>> Hi Chris:
>>>
>>> I've merged your recent xtensa-next with our 2.6.24-smp repo.
>>> It seems to work fine and I'm in the process of cleaning it
>>> up a bit and adding preliminary XTENSA kgdb support.
>>
>> 2.6.24? In that case you probably don't have include/linux/byteorder.h,
>> or include/linux/swab.h which would explain your byteorder problems.
>>
>> Or is that a typo in the version numbers?
>
> I was on 2.6.24 and didn't have a problem compiling the kernel -O0 other
> than a minor tweak in slab.c. Now in 2.6.27-rc3 I get a compile problem
> with rpcb_clnt.c at lines 122, 123, and 129:
>
> /export/src/xtensa-next/net/sunrpc/rpcb_clnt.c:129: error: (near
> initialization for 'rpcb_in6addr_loopback.sin6_port')
>

The problem is fixed by changing htons() to __constant_htons().
The __constant_*() flavors use a #define and always compile
to a constant. Ex:

#define ___constant_swab32(x) \
((__u32)( \
(((__u32)(x) & (__u32)0x000000ffUL) << 24) | \
(((__u32)(x) & (__u32)0x0000ff00UL) << 8) | \
(((__u32)(x) & (__u32)0x00ff0000UL) >> 8) | \
(((__u32)(x) & (__u32)0xff000000UL) >> 24) ))

I tried compiling the recent snapshot from Linus's tree and it
has a new instance of the problem in a common network file.

In this case the recent change was FROM __constant_htons()
to htons(). See net/core/dev.c`simple_tx_hash():

Change by Arnaldo Carvalho de Melo <[email protected]>
Date: 09/20/2008 10:20:49 PM

The log just says:

net: Use hton[sl]() instead of __constant_hton[sl]() where applicable

Doesn't mention that it's prevent the kernel from being compiled for
unoptimized/easy kgdb debugging.

I'll leave this and similar changes on a xtensa smp branch and leave
the head of xtensa-next repo without these changes.

-piet

>
>>
>> You should be able to just pull those two headers from Linus' tree
>> and drop them in, there is no dependencies other than those two
>> headers.
>
> byteorder.h was already in sync with linus's tree and I updated swab.c
> as it had a few changes. Unfortunately the problem persist. I think the
> problem is likely in changes made to rpcb_clnt.c since 2.6.24.
>
>>
>> If it is some other problem, let me know and I'll look into it.
>
> You can likely reproduce it by configuring NFS and compiling -O0.
>
> -piet
>
>>
>> Harvey
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe
>> linux-kernel" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>

2008-11-14 16:34:01

by Harvey Harrison

[permalink] [raw]
Subject: Re: [PATCH 10/10] xtensa: use the new byteorder headers - Merged with your previous xtensa-next and will remerge shortly.

On Fri, 2008-11-14 at 00:13 -0800, Piet Delaney wrote:
> Piet Delaney wrote:
> > Harvey Harrison wrote:
> >> On Fri, 2008-11-07 at 19:00 -0800, Piet Delaney wrote:
> >>> Hi Chris:
> >>>
> >>> I've merged your recent xtensa-next with our 2.6.24-smp repo.
> >>> It seems to work fine and I'm in the process of cleaning it
> >>> up a bit and adding preliminary XTENSA kgdb support.
> >>
> >> 2.6.24? In that case you probably don't have include/linux/byteorder.h,
> >> or include/linux/swab.h which would explain your byteorder problems.
> >>
> >> Or is that a typo in the version numbers?
> >
> > I was on 2.6.24 and didn't have a problem compiling the kernel -O0 other
> > than a minor tweak in slab.c. Now in 2.6.27-rc3 I get a compile problem
> > with rpcb_clnt.c at lines 122, 123, and 129:
> >
> > /export/src/xtensa-next/net/sunrpc/rpcb_clnt.c:129: error: (near
> > initialization for 'rpcb_in6addr_loopback.sin6_port')
> >
>
> The problem is fixed by changing htons() to __constant_htons().
> The __constant_*() flavors use a #define and always compile
> to a constant. Ex:
>
> #define ___constant_swab32(x) \
> ((__u32)( \
> (((__u32)(x) & (__u32)0x000000ffUL) << 24) | \
> (((__u32)(x) & (__u32)0x0000ff00UL) << 8) | \
> (((__u32)(x) & (__u32)0x00ff0000UL) >> 8) | \
> (((__u32)(x) & (__u32)0xff000000UL) >> 24) ))
>
> I tried compiling the recent snapshot from Linus's tree and it
> has a new instance of the problem in a common network file.
>
> In this case the recent change was FROM __constant_htons()
> to htons(). See net/core/dev.c`simple_tx_hash():

I find it strange that the net/core/dev.c bits of this commit were
a problem as they were only changing the use inside the case: statements
which had to be constants in the first place and had better be picked
up by the __builtin_constant_p inside swab32().

I believe you said you were compiling at -O0? What compiler?

Harvey