Re: byteorder mess

Jakub Jelinek (jj@sunsite.ms.mff.cuni.cz)
Fri, 5 Dec 1997 13:26:04 +0100 (MET)


>
> Hi Linus
>
> I have just been reading the 2.1.71 and I must admit that I am fairly
> annoyed with the mess of the new byteorder code, to put it in a mild
> way.

I don't see why do you consider it to be a "mess".
I see such a patch as a great simplification and unification, puts the
common stuff from all the byteorder.h headers to common part, keeping there
only stuff which they differ in.
I must admit I haven't tested it yet (need to finish RTSIG first), but
haven't seen there any problems.

>
> First of all it I think it is a total mess and secondly I am very
> annoyed that it removes our nice architecture optimizations. Could you
> please reverse it until it has been done the right way.

Which optimizations are removed? As far as I see the patch, all
optimizations are kept, but the "mess" is gone.
>
> Instead of putting in gigantic files with 22 byte long file-names, which
> will not work on good old Minix v1 filesystems - yes some people still
> use those for Linux - it would be better to keep the names below 14
> chars and there is absolutely no reason to use such long filenames.

Oh yes, this is just one thing which I think could be changed, names like
linux/byteorder_g.h, linux/byteorder_be.h, linux/byteorder_le.h ... would do
it as well, or even be.h, le.h, maybe not linux/byteorder.h for generic as
that would confuse the users, but that's not a thing why to reject such a
patch.

>
> Second, instead of keeping these gigantic files of swap functions and
> defines and then let the architetures overwrite these it would be much
> cleaner to provide a library of generic functions (in fact this would be
> very small, only 3 in fact, 16, 32 and 64 bit swaps) and then let
> asm/byteorder use these if there are no optimized ones.

Putting these into a library might be "clean", but would slow all the
platforms down in an inacceptable way. Most of these are single-liners and
all of them are inlined, what makes a big difference.

I agree many things in the current kernel regarding byteorder should be
revised (like when browsing thru ext2, one sees stuff like
x = cpu_to_le32(le32_to_cpu(x) | flags);
which is not necessary and can be simplified to
x |= cpu_to_le32(flags);
(as flags are usually constant, cpu_to_le32 will be as well and thus no
swapping occurs at all); also, nobody actually uses the p and s variants of
cpu_to_*), I have them pretty high on my TODO list.

Cheers,
Jakub
___________________________________________________________________
Jakub Jelinek | jj@sunsite.mff.cuni.cz | http://sunsite.mff.cuni.cz
Administrator of SunSITE Czech Republic, MFF, Charles University
___________________________________________________________________
Ultralinux - first 64bit OS to take full power of the UltraSparc
Linux version 2.1.66 on a sparc64 machine (333.41 BogoMips).
___________________________________________________________________