Simple fix to the ARM kexec tools implementation

If we want to assume that the compressed image will expand by a maximum of 4x, we actually need to reserve 5x the space, since we need to keep a copy of the compessed image around while decompressing.

Hi,

Quoted 3 lines · Russell King

If we want to assume that the compressed image will expand by a maximum of 4x, we actually need to reserve 5x the space, since we need to keep a copy of the compessed image around while decompressing.

Looks like 5x is not enough with omap2plus_defconfig at least.. This one needs to be set to 6x for that. Maybe we should just make it go all the way to 11 :)

Quoted 10 lines · Tony Lindgren

Hi,

Quoted 3 lines · Russell King

If we want to assume that the compressed image will expand by a maximum of 4x, we actually need to reserve 5x the space, since we need to keep a copy of the compessed image around while decompressing.

Looks like 5x is not enough with omap2plus_defconfig at least.. This one needs to be set to 6x for that. Maybe we should just make it go all the way to 11 :)

That's not really what this commit is about - it's about making the code consistent with the comments in the file. I could have instead changed the comment saying "4x" to "3x" instead.

There isn't really an answer which will always work for this problem, as I've already detailed in a previous thread discussing the issue. Adding a zImage header don't fix the problem.

Quoted 4 lines · Russell King - ARM Linux

There isn't really an answer which will always work for this problem, as I've already detailed in a previous thread discussing the issue. Adding a zImage header don't fix the problem.

Well how about we change both comments and code to use a value that works?

Quoted 6 lines · Tony Lindgren

Quoted 4 lines · Russell King - ARM Linux

There isn't really an answer which will always work for this problem, as I've already detailed in a previous thread discussing the issue. Adding a zImage header don't fix the problem.

Quoted 2 lines

Well how about we change both comments and code to use a value that works?

Maybe zImage size + MAX_RODATA_SZ + 4x zImage size?

Then the MAX_RODATA_SZ could be 2 or 4 MB or whatever we think is sufficient to kick the can until we have a better solution.

Quoted 6 lines · Tony Lindgren

Maybe zImage size + MAX_RODATA_SZ + 4x zImage size?

Then the MAX_RODATA_SZ could be 2 or 4 MB or whatever we think is sufficient to kick the can until we have a better solution.

Maybe if you give some details about your failing case - you omitted any details about the uncompressed image size...

Quoted 10 lines · Russell King - ARM Linux

Quoted 6 lines · Tony Lindgren

Maybe zImage size + MAX_RODATA_SZ + 4x zImage size?

Then the MAX_RODATA_SZ could be 2 or 4 MB or whatever we think is sufficient to kick the can until we have a better solution.

Maybe if you give some details about your failing case - you omitted any details about the uncompressed image size...

Looks like we currently have this:

Memory: 2033088K/2095100K available (7376K kernel code, 784K rwdata, 2312K rodata, 1024K init, 8117K bss, 45628K reserved, 16384K cma-reserved, 1292284K highmem)
Virtual kernel memory layout:

    vector  : 0xffff0000 - 0xffff1000   (   4 kB)
    fixmap  : 0xffc00000 - 0xfff00000   (3072 kB)
    vmalloc : 0xf0800000 - 0xff800000   ( 240 MB)
    lowmem  : 0xc0000000 - 0xf0000000   ( 768 MB)
    pkmap   : 0xbfe00000 - 0xc0000000   (   2 MB)
    modules : 0xbf000000 - 0xbfe00000   (  14 MB)
      .text : 0xc0008000 - 0xc0a762a4   (10681 kB)
      .init : 0xc0b00000 - 0xc0c00000   (1024 kB)
      .data : 0xc0c00000 - 0xc0cc411c   ( 785 kB)
       .bss : 0xc0cc6000 - 0xc14b37f8   (8118 kB)

So that's 20.7MB, and the zImage was 3.6MB. You're getting an expansion ratio of 5.7x.

To fix that, we'd need to up it to 7x, but the problem with upping it in this way is that it increases the requirements for the crashdump region too. We can play with this number, but there will always be cases where it doesn't work - either because the ratio is too big or too small.

By way of illustration, "zImage size + MAX_RODATA_SZ + 4x zImage size" doesn't even work for this case, since you need about 25MB. Your calculation comes out at 22MB, which is 3MB short. Not only that, but it introduces (from what I can see) an irrelevant fudge factor of "MAX_RODATA_SZ" which has no basis in what's really going on here, and I'm left wondering what "MAX_RODATA_SZ" is actually referring to.

So... I'm of the opinion that we shouldn't play with it - but instead try to come up with a solution which *doesn't* involve teaching kexec a whole load of internals about how the ARM kernel booting happens.

What's more worrying to me at the moment, though, is that the kexec code only tries to find memory for the actual "kernel" size, not the actual space required to decompress the kernel. It could find a chunk of memory large enough to fit the kernel image to be loaded, but is not big enough to allow its decompression. kexec is really quite a mess on ARM. :(

Quoted 8 lines · Russell King - ARM Linux

So that's 20.7MB, and the zImage was 3.6MB. You're getting an expansion ratio of 5.7x.

To fix that, we'd need to up it to 7x, but the problem with upping it in this way is that it increases the requirements for the crashdump region too. We can play with this number, but there will always be cases where it doesn't work - either because the ratio is too big or too small.

OK

Quoted 6 lines

By way of illustration, "zImage size + MAX_RODATA_SZ + 4x zImage size" doesn't even work for this case, since you need about 25MB. Your calculation comes out at 22MB, which is 3MB short. Not only that, but it introduces (from what I can see) an irrelevant fudge factor of "MAX_RODATA_SZ" which has no basis in what's really going on here, and I'm left wondering what "MAX_RODATA_SZ" is actually referring to.

Well I thinking the section alignment split might give us some known size for rodata, but looking at it more it's only with DEBUG_ALIGN_RODATA. If we can't make any assumptions about the size of the rodata, then no point adding it.

How about we check the size of RAM available, and if there is plenty of RAM we use a safe compression ratio of 8. If RAM is a problem, we could make the compression ratio smaller and warn about it. And we could also allow passing the compression ratio to kexec as an option.

I forgot if we still have some 128MB limits too..

When CONFIG_DEBUG_RODATA is enabled, we align the text, read-only data, data, and so on to 1MB boundaries so that we can change the permissions of the sections to enforce the properties of the various ELF segments. This padding massively inflates the size of the run-time kernel image, and can result in megabytes of wastage (if we're as little as one byte into 1MB, we have to round up to the next megabyte.)

The padding, being a string of zeros, is very compressable, and this increases the compression ratio.

The BSS is not included in the compressed image, but needs to be taken account of when avoiding overwriting the DTB.

Quoted 5 lines

How about we check the size of RAM available, and if there is plenty of RAM we use a safe compression ratio of 8. If RAM is a problem, we could make the compression ratio smaller and warn about it. And we could also allow passing the compression ratio to kexec as an option.

You do know that you can already pass the "size" of the kernel to be reserved to kexec. --image-size. It's not documented in --help, but it is there (which is another annoying thing about ARM kexec...)

If we want to assume that the compressed image will expand by a maximum of 4x, we actually need to reserve 5x the space, since we need to keep a copy of the compessed image around while decompressing.

This is the "simple" fix for the ARM kexec tools code, which makes the code actually do what the comments describe.

There are two problems:

1. When placing the initrd, we do not take account of the 32k offset that is required for the kernel image.

2. The code claims to allow the zImage decompressor to expand by a factor of 4, but only allows space for a factor of 3, since the decompressor itself has to fit in this memory space as well.

Changing the code to follow the comments allows Keystone II to kexec with DEBUG_RODATA enabled, at least with the kernel I've been testing with.

This is a stop-gap solution until a better solution can be implemented.

 kexec/arch/arm/kexec-zImage-arm.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

Signature · 3 lines

RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Yes, so until we have proper header for zImage, these patches looks fine to me.

For both of the patches:

Reviewed-by Pratyush Anand <[email protected]>

Quoted 2 lines · Pratyush Anand

Yes, so until we have proper header for zImage, these patches looks fine to me.

If you read my last email in the "kexec failures with DEBUG_RODATA" thread, you'll see that I'm unhappy with this idea, because adding a "proper header" still results in some kind of assumption about how the zImage decompressor works being needed by the boot environment.

Quoted 3 lines

For both of the patches:

Reviewed-by Pratyush Anand <[email protected]>

Thanks.

Simon, do you need me to resend with the r-b tag, or will you pick these two up and the documentation fix patch you asked for?