[PATCH 0/7] inflate pt1: refactor boot-time inflate code

[PATCH 0/7] inflate pt1: refactor boot-time inflate code

[Matt Mackall]

This is a refactored version of the lib/inflate.c:

- clean up some really ugly code
- clean up atrocities like '#include "../../../lib/inflate.c"'
- drop a ton of cut and paste code from the kernel boot
- move towards making the boot decompressor pluggable
- move towards unifying the multiple inflate implementations
- save space

I'm sending this out in three batches. This first batch is core
clean-ups without arch-specific changes.

(This work was sponsored in part by the CE Linux Forum.)

Re: [PATCH 0/7] inflate pt1: refactor boot-time inflate code

[Russell King]

On Fri, Feb 24, 2006 at 02:12:15PM -0600, Matt Mackall wrote:
> This is a refactored version of the lib/inflate.c:
> 
> - clean up some really ugly code
> - clean up atrocities like '#include "../../../lib/inflate.c"'
> - drop a ton of cut and paste code from the kernel boot
> - move towards making the boot decompressor pluggable
> - move towards unifying the multiple inflate implementations
> - save space
> 
> I'm sending this out in three batches. This first batch is core
> clean-ups without arch-specific changes.
> 
> (This work was sponsored in part by the CE Linux Forum.)

ISTR something like this was posted months back, but I don't remember
what the status of it was.  Hence, I might be repeating myself in this
reply, but I feel it's better to mention this than not to.

There's a comment at the top of arch/arm/boot/compressed/misc.c which
describes the use of the inflate code on ARM - for the kernel it's a
special case where the decompressor is run from ROM.

There's also another twist to it though - our relocatable zImage
requires us to build all files in the executable part of zImage
without _any_ static variables.  If there's one or more static
variables, this feature breaks horribly (and silently in the non-
relocated cases.)

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

Re: [PATCH 0/7] inflate pt1: refactor boot-time inflate code

[Matt Mackall]

On Fri, Feb 24, 2006 at 08:56:26PM +0000, Russell King wrote:
> On Fri, Feb 24, 2006 at 02:12:15PM -0600, Matt Mackall wrote:
> > This is a refactored version of the lib/inflate.c:
> > 
> > - clean up some really ugly code
> > - clean up atrocities like '#include "../../../lib/inflate.c"'
> > - drop a ton of cut and paste code from the kernel boot
> > - move towards making the boot decompressor pluggable
> > - move towards unifying the multiple inflate implementations
> > - save space
> > 
> > I'm sending this out in three batches. This first batch is core
> > clean-ups without arch-specific changes.
> > 
> > (This work was sponsored in part by the CE Linux Forum.)
> 
> ISTR something like this was posted months back, but I don't remember
> what the status of it was.  Hence, I might be repeating myself in this
> reply, but I feel it's better to mention this than not to.
> 
> There's a comment at the top of arch/arm/boot/compressed/misc.c which
> describes the use of the inflate code on ARM - for the kernel it's a
> special case where the decompressor is run from ROM.
> 
> There's also another twist to it though - our relocatable zImage
> requires us to build all files in the executable part of zImage
> without _any_ static variables.  If there's one or more static
> variables, this feature breaks horribly (and silently in the non-
> relocated cases.)

I think I addressed all those issues last time around, and none of
them should be present in this batch anyway. But it's possible I've
missed something. If you'd like, I can send the whole set to you for
testing.

-- 
Mathematics is the supreme nostalgia of our time.

Re: [PATCH 0/7] inflate pt1: refactor boot-time inflate code

[Nicolas Pitre]

On Fri, 24 Feb 2006, Matt Mackall wrote:

> On Fri, Feb 24, 2006 at 08:56:26PM +0000, Russell King wrote:
> > There's a comment at the top of arch/arm/boot/compressed/misc.c which
> > describes the use of the inflate code on ARM - for the kernel it's a
> > special case where the decompressor is run from ROM.
> > 
> > There's also another twist to it though - our relocatable zImage
> > requires us to build all files in the executable part of zImage
> > without _any_ static variables.  If there's one or more static
> > variables, this feature breaks horribly (and silently in the non-
> > relocated cases.)
> 
> I think I addressed all those issues last time around, and none of
> them should be present in this batch anyway. But it's possible I've
> missed something. If you'd like, I can send the whole set to you for
> testing.

Just make sure that, once compiled, none of the related object files 
have a non-zero data size when the "size" command is used on them.


Nicolas

Re: [PATCH 0/7] inflate pt1: refactor boot-time inflate code

[Nicolas Pitre]

On Fri, 24 Feb 2006, Matt Mackall wrote:

> This is a refactored version of the lib/inflate.c:
> 
> - clean up some really ugly code
> - clean up atrocities like '#include "../../../lib/inflate.c"'
> - drop a ton of cut and paste code from the kernel boot
> - move towards making the boot decompressor pluggable
> - move towards unifying the multiple inflate implementations
> - save space

Could you also make sure that there is no non-const global variables 
whatsoever in there?  The idea is that on ARM the decompressor was once 
made to be executable directly from flash so the deflated kernel image 
would be stored at its final location in ram directly without first 
copying zImage nor any .data to ram in order to execute it.  This is a 
significant boot time saving (which I think CE Linux is interested in) 
while still preserving a compressed kernel in flash.

Currently:

$ size lib/zlib_inflate/zlib_inflate.o
   text    data     bss     dec     hex filename
  11868      68       0   11936    2ea0 lib/zlib_inflate/zlib_inflate.o

Since this code is probably reentrant already, global variables are most 
probably read-only and declaring them const will store their content 
into .rodata which should be accounted as text above.

Having an empty .data section is the easiest way to be sure the kernel 
decompressor can actually execute from flash without subtle bugs due to 
the enforced read-only nature of global variables.


Nicolas