From: Stephen Rothwell on
Hi Andrew, Joakim,

On Tue, 25 May 2010 23:41:16 -0700 Andrew Morton <akpm(a)linux-foundation.org> wrote:
>
> On Wed, 26 May 2010 08:29:45 +0200 Joakim Tjernlund <joakim.tjernlund(a)transmode.se> wrote:
>
> > Stephen Rothwell <sfr(a)canb.auug.org.au> wrote on 2010/05/26 06:09:00:
> > >
> > > On Tue, 25 May 2010 18:20:40 -0700 Andrew Morton <akpm(a)linux-foundation.org> wrote:
> > > >
> > > > hmpf. Does this fix?
> > >
> > > No.
> > >
> > > The problem is that __LITTLE_ENDIAN is only defined in
> > > linux/byteorder/little_endian.h which is only included by little endian
> > > architectures ...
> >
> > Ah, not sure how to fix this ATM. Perhaps best to revert
> > crc32: use __BYTE_ORDER macro for endian detection
> > for now.
>
> yup.

We also need to revert b3b77c8caef1750ebeea1054e39e358550ea9f55 ("endian:
#define __BYTE_ORDER") in order to get rid of the other warning I pointed out:

In file included from fs/jfs/jfs_types.h:33,
from fs/jfs/jfs_incore.h:26,
from fs/jfs/file.c:22:
fs/jfs/endian24.h:36:101: warning: "__LITTLE_ENDIAN" is not defined

I get this warning on powerpc builds for most of the files in fs/jfs ...
--
Cheers,
Stephen Rothwell sfr(a)canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
From: Joakim Tjernlund on
Stephen Rothwell <sfr(a)canb.auug.org.au> wrote on 2010/05/26 09:14:24:
>
> Hi Andrew, Joakim,
>
> On Tue, 25 May 2010 23:41:16 -0700 Andrew Morton <akpm(a)linux-foundation.org> wrote:
> >
> > On Wed, 26 May 2010 08:29:45 +0200 Joakim Tjernlund
> <joakim.tjernlund(a)transmode.se> wrote:
> >
> > > Stephen Rothwell <sfr(a)canb.auug.org.au> wrote on 2010/05/26 06:09:00:
> > > >
> > > > On Tue, 25 May 2010 18:20:40 -0700 Andrew Morton <akpm(a)linux-
> foundation.org> wrote:
> > > > >
> > > > > hmpf. Does this fix?
> > > >
> > > > No.
> > > >
> > > > The problem is that __LITTLE_ENDIAN is only defined in
> > > > linux/byteorder/little_endian.h which is only included by little endian
> > > > architectures ...
> > >
> > > Ah, not sure how to fix this ATM. Perhaps best to revert
> > > crc32: use __BYTE_ORDER macro for endian detection
> > > for now.
> >
> > yup.
>
> We also need to revert b3b77c8caef1750ebeea1054e39e358550ea9f55 ("endian:
> #define __BYTE_ORDER") in order to get rid of the other warning I pointed out:
>
> In file included from fs/jfs/jfs_types.h:33,
> from fs/jfs/jfs_incore.h:26,
> from fs/jfs/file.c:22:
> fs/jfs/endian24.h:36:101: warning: "__LITTLE_ENDIAN" is not defined
>
> I get this warning on powerpc builds for most of the files in fs/jfs ...

endian24.h has:
#if (defined(__KERNEL__) && defined(__LITTLE_ENDIAN)) || (defined(__BYTE_ORDER) && (__BYTE_ORDER == __LITTLE_ENDIAN))
That won't work for BE CPU's. Perhaps something like this will?
#if defined(__BIG_ENDIAN) && defined(__LITTLE_ENDIAN) && (__BYTE_ORDER == __LITTLE_ENDIAN) || defined(__LITTLE_ENDIAN)

The kernel borrows __LITTLE_ENDIAN/__BIG_ENDIAN from user space but does not impl. the
full semantics that user space has. Adding full user space semantics isn't easy as it clashes with
current use of __LITTLE_ENDIAN/__BIG_ENDIAN.

If my suggestion above works, then one could start transforming current uses of __BYTE_ORDER,
into similar constructs and once all are done, #define both __LITTLE_ENDIAN/__BIG_ENDIAN and
move back to #if __BYTE_ORDER == __LITTLE_ENDIAN

Jocke

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Linus Torvalds on


On Wed, 26 May 2010, Stephen Rothwell wrote:
>
> We also need to revert b3b77c8caef1750ebeea1054e39e358550ea9f55 ("endian:
> #define __BYTE_ORDER") in order to get rid of the other warning I pointed out:

Done.

Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Linus Torvalds on


On Tue, 25 May 2010, Andrew Morton wrote:
>
> > I suspect fixing this entails moving all endian tests to #if __BYTE_ORDER ...
> > and then always define both __LITTLE_ENDIAN and __BIG_ENDIAN
>
> Yes we can't define __LITTLE_ENDIAN to anything on big-endian without
> breaking lots of other things.

Indeed. The whole glibc thing of doing

# if __BYTE_ORDER == __BIG_ENDIAN

(see "grep ENDIAN /usr/include/*") is a f*cking idiotic. It is way less
readable, and insane to do. I guess it's some traditional unix disease.

The way to do it in Linux is to just do

#ifdef __{BIG,LITTLE}_ENDIAN

which is a lot more readable.

If anybody wants to change that, they still shouldn't change it to the
idiotic glibc model. If you want it to be more readable, maybe somebody
can introduce a CONFIG_{BIG,LITTLE}_ENDIAN{_BITFIELDS}

Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Joakim Tjernlund on
Linus Torvalds <torvalds(a)linux-foundation.org> wrote on 2010/05/26 17:29:02:
>
> On Wed, 26 May 2010, Joakim Tjernlund wrote:
> >
> > If my suggestion above works, then one could start transforming current uses
> of __BYTE_ORDER,
> > into similar constructs and once all are done, #define both __LITTLE_ENDIAN/
> __BIG_ENDIAN and
> > move back to #if __BYTE_ORDER == __LITTLE_ENDIAN
>
> No. Don't do it. Why the hell would we want to use the inferior model?

I agree that the glibc model(dunno if this is glibc/gcc specific or some standard)
is less than optimal but the linux model has two major flaws too:

1) It silently breaks when neither of {__LITTLE_,__BIG}_ENDIAN (or both)are
defined depending on the endianess of the target CPU.
The glibc model generates a compile error if you forget to include __BYTE_ORDER.

2) It clashes with user space so one cannot use it in exported header files.

If you grep for __BYTE_ORDER in the kernel you will find some ugly #ifdefs
to overcome 2). It would have been somewhat better if linux had defined its own
names instead of reusing the glibc/gcc names with different semantics.

1) is worse and the reason for this patch. I moved lib/crc32.c
to user space so I could run the builtin unit test program because
I wanted to do some optimizations. It just silently broke depending
on the endianess of the CPU. Took half a day to figure out why :(
This could easily happen in the kernel too.

Jocke

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