[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <201011251634.04065.lasse.collin@tukaani.org>
Date: Thu, 25 Nov 2010 16:34:03 +0200
From: Lasse Collin <lasse.collin@...aani.org>
To: Andrew Morton <akpm@...ux-foundation.org>
Cc: linux-kernel@...r.kernel.org, linux-embedded@...r.kernel.org,
"H. Peter Anvin" <hpa@...or.com>, Alain Knaff <alain@...ff.lu>,
Albin Tonnerre <albin.tonnerre@...e-electrons.com>,
Phillip Lougher <phillip@...gher.demon.co.uk>
Subject: Re: [PATCH RFC 1/3] Decompressors: Add XZ decompressor module
On 2010-11-25 Andrew Morton wrote:
> On Wed, 24 Nov 2010 22:51:52 +0200
> Lasse Collin <lasse.collin@...aani.org> wrote:
> > This patch: Add the main decompression code (xz_dec), testing
> > module (xz_dec_test), wrapper script (xz_wrap.sh) for the xz
> > command line tool, and documentation. The xz_dec module is
> > enough to have a usable XZ decompressor e.g. for Squashfs.
>
> I'm not seeing any documentation which tells me how to create,
> install and execute xs-compressed kernels. There are new makefile
> targets?
The last paragraph under "XZ related components in the kernel" in
Documentation/xz.txt mentions xzkern and xzmisc commands available for
makefiles. They are defined in scripts/Makefile.lib and have some
comments as documentation there too.
The Kconfig options to enable XZ-compressed kernel are added in the
second patch. There is already an option to select the kernel
compression method so I only added XZ to that list. I assume that people
will find this option just like they have been able to find the other
compression methods.
> > +#define bcj_x86_test_msbyte(b) ((b) == 0x00 || (b) == 0xFF)
>
> This should be written in C. It looks nicer, and so
> bcj_x86_test_msbyte(*p++) won't explode.
Thanks. It's fixed in XZ Embedded git repository now:
git clone http://git.tukaani.org/xz-embedded.git
(clone only, no WWW for now)
I will post updated patches when needed.
> > +static noinline_for_stack size_t bcj_x86(
>
> hm, but it uses little stack space.
I wrote that code 19 months ago and now it looks odd to me too. I
remember that my idea was to minimize stack usage in this call stack:
xz_dec_run() -> xz_dec_bcj_run() -> xz_dec_lzma2_run() -> ...
I wanted to prevent the BCJ filters from being inlined into
xz_dec_bcj_run() because inlining would have increased the maximum
stack usage by 100-150 bytes.
The BCJ filters are called via bcj_apply() which is called in two places
from xz_dec_bcj_run(). Since there are two calls to bcj_apply(), GCC
won't inline it. It will only inline the BCJ filters into bcj_apply()
and that's OK. So noinline_for_stack does nothing good here. Maybe my
early version had only one call to bcj_apply() or something like that.
Anyway, I removed noinline_for_stacks. I tested it also when only
one BCJ filter is enabled and with a few different GCC versions.
> > +static noinline_for_stack size_t bcj_x86(
> > + struct xz_dec_bcj *s, uint8_t *buf, size_t size)
>
> The preferred style is
>
> static noinline_for_stack size_t bcj_x86(struct xz_dec_bcj *s, uint8_t *buf,
> size_t size)
>
> or
>
> static noinline_for_stack size_t
> bcj_x86(struct xz_dec_bcj *s, uint8_t *buf, size_t size)
>
> (lots of dittoes)
Fixed.
--
Lasse Collin | IRC: Larhzu @ IRCnet & Freenode
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists