[CFT] inflate.c rework arch testing needed

[CFT] inflate.c rework arch testing needed

[Matt Mackall]

I've reworked the mess that is lib/inflate.c, including:

- proper formatting
- killing a ton of legacy code
- cleaning up IO and CRC handling
- eliminating all the global variables
- using __init for the core kernel
- proper linking rather than the #include "../lib/inflate.c" hack
- lots of minor cleanups along the way

This drops a ton of support code from all the users of this code as
well:

 arch/arm/boot/compressed/Makefile    |    5
 arch/arm/boot/compressed/misc.c      |  244 --
 arch/i386/boot/compressed/Makefile   |    6
 arch/i386/boot/compressed/misc.c     |  224 --
 arch/x86_64/boot/compressed/Makefile |    6
 arch/x86_64/boot/compressed/misc.c   |  212 --
 include/linux/inflate.h              |    9
 init/do_mounts_rd.c                  |  129 -
 init/initramfs.c                     |  139 -
 lib/Makefile                         |    4
 lib/inflate.c                        | 3047 ++++++++++++++++-----------------
 11 files changed, 1688 insertions(+), 2337 deletions(-)

I've converted only some of the users, and currently only tested x86.
Additional x86 testing as well as testing my current ARM and x86_64
support and doing the fixups for the other arches would be
appreciated.

Current patch rollup against 2.6.5-rc1 is at:

 http://selenic.com/inflate-work.patch.bz2

-- 
Matt Mackall : http://www.selenic.com : Linux development and consulting

Re: [CFT] inflate.c rework arch testing needed

[Jörn Engel]

On Thu, 18 March 2004 17:10:06 -0600, Matt Mackall wrote:
> 
> I've reworked the mess that is lib/inflate.c, including:
> 
> - proper formatting
> - killing a ton of legacy code
> - cleaning up IO and CRC handling
> - eliminating all the global variables
> - using __init for the core kernel
> - proper linking rather than the #include "../lib/inflate.c" hack
> - lots of minor cleanups along the way
> 
> This drops a ton of support code from all the users of this code as
> well:
> 
>  arch/arm/boot/compressed/Makefile    |    5
>  arch/arm/boot/compressed/misc.c      |  244 --
>  arch/i386/boot/compressed/Makefile   |    6
>  arch/i386/boot/compressed/misc.c     |  224 --
>  arch/x86_64/boot/compressed/Makefile |    6
>  arch/x86_64/boot/compressed/misc.c   |  212 --
>  include/linux/inflate.h              |    9
>  init/do_mounts_rd.c                  |  129 -
>  init/initramfs.c                     |  139 -
>  lib/Makefile                         |    4
>  lib/inflate.c                        | 3047 ++++++++++++++++-----------------
>  11 files changed, 1688 insertions(+), 2337 deletions(-)

I like the patch in general.  This is definitely the wrong time for
it, but at least parts of it could go into 2.7 sometime.

Have you thought about updating to a more recent version of zlib?  It
is most likely not worth it but I'd like to know for sure.

Jörn

-- 
To recognize individual spam features you have to try to get into the
mind of the spammer, and frankly I want to spend as little time inside
the minds of spammers as possible.
-- Paul Graham

Re: [CFT] inflate.c rework arch testing needed

[Matt Mackall]

On Fri, Mar 19, 2004 at 01:32:52AM +0100, J?rn Engel wrote:
> On Thu, 18 March 2004 17:10:06 -0600, Matt Mackall wrote:
> > 
> > I've reworked the mess that is lib/inflate.c, including:
> > 
> > - proper formatting
> > - killing a ton of legacy code
> > - cleaning up IO and CRC handling
> > - eliminating all the global variables
> > - using __init for the core kernel
> > - proper linking rather than the #include "../lib/inflate.c" hack
> > - lots of minor cleanups along the way
> > 
> > This drops a ton of support code from all the users of this code as
> > well:
> > 
> >  arch/arm/boot/compressed/Makefile    |    5
> >  arch/arm/boot/compressed/misc.c      |  244 --
> >  arch/i386/boot/compressed/Makefile   |    6
> >  arch/i386/boot/compressed/misc.c     |  224 --
> >  arch/x86_64/boot/compressed/Makefile |    6
> >  arch/x86_64/boot/compressed/misc.c   |  212 --
> >  include/linux/inflate.h              |    9
> >  init/do_mounts_rd.c                  |  129 -
> >  init/initramfs.c                     |  139 -
> >  lib/Makefile                         |    4
> >  lib/inflate.c                        | 3047 ++++++++++++++++-----------------
> >  11 files changed, 1688 insertions(+), 2337 deletions(-)
> 
> I like the patch in general.  This is definitely the wrong time for
> it, but at least parts of it could go into 2.7 sometime.

Well it's been sitting in -tiny for a while already and will stay
there until someone else is ready for it. I may try to push it to -mm
at some point.
 
> Have you thought about updating to a more recent version of zlib?  It
> is most likely not worth it but I'd like to know for sure.

The code for new versions of zlib is significantly scarier last I
checked and there's no particular advantage to it. But one of the
primary motivations here is to get to the point where something like
bunzip2 or even a new zlib is a drop-in replacement.

-- 
Matt Mackall : http://www.selenic.com : Linux development and consulting

Re: [CFT] inflate.c rework arch testing needed

[Jörn Engel]

On Thu, 18 March 2004 21:09:42 -0600, Matt Mackall wrote:
> 
> The code for new versions of zlib is significantly scarier last I
> checked and there's no particular advantage to it. But one of the
> primary motivations here is to get to the point where something like
> bunzip2 or even a new zlib is a drop-in replacement.

Zlib 1.2.1 is supposed to be much faster, but they intoduced new code
so the price appears to be size.  I'll look into it for the other zlib
in the kernel if noone finds the time before me.

Jörn

-- 
It's just what we asked for, but not what we want!
-- anonymous

Re: [CFT] inflate.c rework arch testing needed

[Russell King]

On Thu, Mar 18, 2004 at 05:10:06PM -0600, Matt Mackall wrote:
> I've reworked the mess that is lib/inflate.c, including:

Please don't - this will probably break the PIC decompressor on ARM.

There are un-merged patches in the pipeline which make this all work
correctly with todays toolchains - which mostly means getting rid of
all static data to make the compiler produce the right relocations.

Yes its stupid, but there seems to be no other way to do this with
the ELF toolchains we have available to us now.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: [CFT] inflate.c rework arch testing needed

[Matt Mackall]

On Fri, Mar 19, 2004 at 10:31:26AM +0000, Russell King wrote:
> On Thu, Mar 18, 2004 at 05:10:06PM -0600, Matt Mackall wrote:
> > I've reworked the mess that is lib/inflate.c, including:
> 
> Please don't - this will probably break the PIC decompressor on ARM.

If it works now, I don't see why it should. My last version of this
was test-booted on ARM. The only thing here that could conceivably
break PIC is the change to linking but the old include hack will
continue to work. At any rate my other cleanups should be orthogonal
to all that.
 
> There are un-merged patches in the pipeline which make this all work
> correctly with todays toolchains - which mostly means getting rid of
> all static data to make the compiler produce the right relocations.

Well perhaps you could send them to me.

-- 
Matt Mackall : http://www.selenic.com : Linux development and consulting

Re: [CFT] inflate.c rework arch testing needed

[Russell King]

On Fri, Mar 19, 2004 at 11:16:15AM -0600, Matt Mackall wrote:
> On Fri, Mar 19, 2004 at 10:31:26AM +0000, Russell King wrote:
> > On Thu, Mar 18, 2004 at 05:10:06PM -0600, Matt Mackall wrote:
> > > I've reworked the mess that is lib/inflate.c, including:
> > 
> > Please don't - this will probably break the PIC decompressor on ARM.
> 
> If it works now, I don't see why it should. My last version of this
> was test-booted on ARM.

One test boot isn't sufficient - it's a combination of toolchain and
the way you boot which will bring up the issue.  Maybe you're using an
older toolchain from me, or not actually using the relocatable feature.

Basically, we rely on the toolchain generating GOT relocations for
data references, and we use the GOT table to point them to an area
of RAM.  However, later toolchains seem to "optimise" this and use
GOTOFF relocations for static data.  The GOT table is at a fixed
offset from the text segment, and this effectively makes the whole
thing unrelocatable.

(Having GOTOFF relocs for static read only data is fine though.
It's the read/write data which must use GOT relocs.)

> > There are un-merged patches in the pipeline which make this all work
> > correctly with todays toolchains - which mostly means getting rid of
> > all static data to make the compiler produce the right relocations.
> 
> Well perhaps you could send them to me.

I think this is everything...

diff -urpN orig/arch/arm/boot/compressed/misc.c linux/arch/arm/boot/compressed/misc.c
--- orig/arch/arm/boot/compressed/misc.c	Sun Sep 28 09:53:47 2003
+++ linux/arch/arm/boot/compressed/misc.c	Sun Sep 28 09:46:13 2003
@@ -113,7 +113,7 @@ static inline __ptr_t memcpy(__ptr_t __d
  * gzip delarations
  */
 #define OF(args)  args
-#define STATIC static
+#define STATIC
 
 typedef unsigned char  uch;
 typedef unsigned short ush;
@@ -122,12 +122,12 @@ typedef unsigned long  ulg;
 #define WSIZE 0x8000		/* Window size must be at least 32k, */
 				/* and a power of two */
 
-static uch *inbuf;		/* input buffer */
-static uch window[WSIZE];	/* Sliding window buffer */
+unsigned char *inbuf;		/* input buffer */
+unsigned char window[WSIZE];	/* Sliding window buffer */
 
-static unsigned insize;		/* valid bytes in inbuf */
-static unsigned inptr;		/* index of next byte to be processed in inbuf */
-static unsigned outcnt;		/* bytes in output buffer */
+unsigned int insize;		/* valid bytes in inbuf */
+unsigned int inptr;		/* index of next byte to be processed in inbuf */
+unsigned int outcnt;		/* bytes in output buffer */
 
 /* gzip flag byte */
 #define ASCII_FLAG   0x01 /* bit 0 set: file probably ascii text */
@@ -166,9 +166,9 @@ static void gzip_release(void **);
 extern char input_data[];
 extern char input_data_end[];
 
-static uch *output_data;
-static ulg output_ptr;
-static ulg bytes_out;
+unsigned char *output_data;
+unsigned long output_ptr;
+unsigned long bytes_out;
 
 static void *malloc(int size);
 static void free(void *where);
@@ -179,8 +179,8 @@ static void gzip_release(void **);
 static void puts(const char *);
 
 extern int end;
-static ulg free_mem_ptr;
-static ulg free_mem_ptr_end;
+unsigned long free_mem_ptr;
+unsigned long free_mem_ptr_end;
 
 #define HEAP_SIZE 0x2000
 
--- orig/lib/inflate.c	Fri Jan  9 22:40:07 2004
+++ linux/lib/inflate.c	Fri Jan  9 22:52:48 2004
@@ -1025,8 +1025,8 @@ STATIC int inflate(void)
  *
  **********************************************************************/
 
-static ulg crc_32_tab[256];
-static ulg crc;		/* initialized in makecrc() so it'll reside in bss */
+STATIC ulg crc_32_tab[256];
+STATIC ulg crc;		/* initialized in makecrc() so it'll reside in bss */
 #define CRC_VALUE (crc ^ 0xffffffffUL)
 
 /*



-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: [CFT] inflate.c rework arch testing needed

[Matt Mackall]

On Fri, Mar 19, 2004 at 09:39:57PM +0000, Russell King wrote:
> > > There are un-merged patches in the pipeline which make this all work
> > > correctly with todays toolchains - which mostly means getting rid of
> > > all static data to make the compiler produce the right relocations.
> > 
> > Well perhaps you could send them to me.
> 
> I think this is everything...

Ok, I can merge all of this with my stuff without creating any undue pain.
I've actually killed most of it already in the course of eliminating globals.

-- 
Matt Mackall : http://www.selenic.com : Linux development and consulting

Re: [discuss] Re: [CFT] inflate.c rework arch testing needed

[Pavel Machek]

Hi!

> The code for new versions of zlib is significantly scarier last I
> checked and there's no particular advantage to it. But one of the
> primary motivations here is to get to the point where something like
> bunzip2 or even a new zlib is a drop-in replacement.

Some people want fast-but-not-big-ratio compressor for speeding up swsusp.
If compressors are drop-in, thats very good.

				Pavel
-- 
64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms         

Re: [discuss] Re: [CFT] inflate.c rework arch testing needed

[Jörn Engel]

On Sun, 28 March 2004 09:00:15 +0200, Pavel Machek wrote:
> 
> > The code for new versions of zlib is significantly scarier last I
> > checked and there's no particular advantage to it. But one of the
> > primary motivations here is to get to the point where something like
> > bunzip2 or even a new zlib is a drop-in replacement.
> 
> Some people want fast-but-not-big-ratio compressor for speeding up swsusp.
> If compressors are drop-in, thats very good.

Also keep in mind, which zlib you are talking about.  Matt's patch is
to lib/inflate.c, which is used exclusively for the bootloader which
is built-in to the kernel on some arches.  That one was always far
behind and scary to look at.

The real kernel zlib is in a somewhat better state, many people cared
about and worked on it.  It's not great either, but at least better.
And yes, drop-in replacement or even runtime choice would be nice here.

Jörn

-- 
The story so far:
In the beginning the Universe was created.  This has made a lot
of people very angry and been widely regarded as a bad move.
-- Douglas Adams?