From: Michael Tokarev <mjt@tls.msk.ru>
To: Sam Ravnborg <sam@ravnborg.org>
Cc: Michal Marek <mmarek@suse.cz>,
Michael Guntsche <mike@it-loops.com>,
Oliver Hartkopp <oliver@hartkopp.net>,
linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] kbuild: correct size calculation of bzImgae / fix x86 boot
Date: Sun, 20 Dec 2009 14:19:41 +0300 [thread overview]
Message-ID: <4B2E084D.6080105@msgid.tls.msk.ru> (raw)
In-Reply-To: <20091220100344.GA6614@merkur.ravnborg.org>
Sam Ravnborg wrote:
> We use ... printf \x ... when calculating the size of the
> compressed kernel.
> Unfortunately dash built-in printf does not support this notation
> resulting in a non-bootable kernel.
>
> Fix this by always using the external version of printf.
>
> The commit that introduced this bug was:
> 4a2ff67c88211026afcbdbc190c13f705dae1b59: "kbuild: fix bzImage
> build for x86"
It's not that simple Sam. This commit is a part of the game.
It works neither with printf as after my commit above, nor with
original fix with `/bin/echo -ne'. In neither case the fix is
complete or robust. Because there are two places in the few lines,
around this, both affected and both are non-POSIX-conformant.
> Reported-by: Michael Guntsche <mike@it-loops.com>
> Cc: Oliver Hartkopp <oliver@hartkopp.net>
> Cc: Johannes Stezenbach <js@sig21.net>
> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> ---
>
> This should fix it but it would be great if it is tested.
> Michal Marek, I think this needs to go upstream pretty quickly.
This whole issue only affects debian and ubuntu for now.
But the bug is very difficult to track down, since the prob
happens on early boot.
For now, I think, the best is to ensure we're building with
bash not dash. Ie, SHELL=/bin/bash make ... will work.
Because even with this second fix, it still does not work
correctly with dash's echo and printf behaving differently
than bash's (but still conforming to POSIX). It produces
good kernel, or at least I don't see any obvious probs with
the resulting kernel, but at least `make V=1' output looks
all wrong in this area (since dash's echo interpret \nnn
by default and make uses echo to print commands as they're
executed).
The right fix is to eliminate all this hack altogether.
Imho anyway.
/mjt
> Sam
>
> scripts/Makefile.lib | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index cd815ac..bd201d9 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -213,13 +213,14 @@ cmd_gzip = (cat $(filter-out FORCE,$^) | gzip -f -9 > $@) || \
>
> # Bzip2 and LZMA do not include size in file... so we have to fake that;
> # append the size as a 32-bit littleendian number as gzip does.
> +# Note: dash built-in printf does not support \x so use /usr/bin version
> size_append = printf $(shell \
> dec_size=0; \
> for F in $1; do \
> fsize=$$(stat -c "%s" $$F); \
> dec_size=$$(expr $$dec_size + $$fsize); \
> done; \
> -printf "%08x" $$dec_size | \
> +/usr/bin/printf "%08x" $$dec_size | \
> sed 's/\(..\)\(..\)\(..\)\(..\)/\\\\x\4\\\\x\3\\\\x\2\\\\x\1/g' \
> )
>
next prev parent reply other threads:[~2009-12-20 11:19 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-12-19 23:34 2.6.33-rc1 Reboot right after bootloader Michael Guntsche
2009-12-20 8:46 ` Sam Ravnborg
2009-12-20 9:10 ` Johannes Stezenbach
2009-12-20 10:38 ` Michael Tokarev
2009-12-20 11:10 ` Michael Tokarev
2009-12-20 11:38 ` Michael Tokarev
2009-12-20 9:11 ` Michael Guntsche
2009-12-20 10:03 ` [PATCH] kbuild: correct size calculation of bzImgae / fix x86 boot Sam Ravnborg
2009-12-20 10:20 ` Oliver Hartkopp
2009-12-20 10:28 ` Willy Tarreau
2009-12-20 10:47 ` Michael Tokarev
2009-12-20 13:09 ` Willy Tarreau
2009-12-20 11:19 ` Michael Tokarev [this message]
2009-12-20 12:02 ` Andreas Schwab
2009-12-20 12:18 ` Sam Ravnborg
2009-12-21 12:23 ` Michal Marek
2009-12-21 13:50 ` Andreas Schwab
2009-12-20 14:11 ` Sam Ravnborg
2009-12-21 14:17 ` Michal Marek
2009-12-21 16:28 ` Michael Guntsche
2009-12-21 19:53 ` Michael Tokarev
2009-12-21 20:48 ` Michael Guntsche
2009-12-21 18:28 ` H. Peter Anvin
2009-12-21 19:09 ` H. Peter Anvin
2009-12-20 13:50 ` Arkadiusz Miskiewicz
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4B2E084D.6080105@msgid.tls.msk.ru \
--to=mjt@tls.msk.ru \
--cc=linux-kernel@vger.kernel.org \
--cc=mike@it-loops.com \
--cc=mmarek@suse.cz \
--cc=oliver@hartkopp.net \
--cc=sam@ravnborg.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox