2006-02-24 20:12:46

by Matt Mackall

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

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.)


2006-02-24 20:56:36

by Russell King

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

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

2006-02-24 21:30:36

by Matt Mackall

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

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.

2006-02-24 22:07:22

by Nicolas Pitre

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

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

2006-02-24 22:15:06

by Nicolas Pitre

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

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