Date:   Sat, 20 May 2017 15:30:05 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     klondike <klondike@...ndike.es>, linux-kernel@...r.kernel.org,
        npiggin@...il.com
Cc:     P J P <ppandit@...hat.com>, Paul Bolle <pebolle@...cali.nl>,
        Andrew Morton <akpm@...ux-foundation.org>, mmarek@...e.cz
Subject: Re: [v3, 2/2] initramfs: Allow again choice of the embedded initram
 compression algorithm

Hi Francisco, Nicholas,

Nicholas already fixed part of this commit, but there is more breakage,
see below:

On 09/27/2016 01:32 PM, klondike wrote:
> Choosing the appropriate compression option when using an embeded initramfs
> can result in significant size differences in the resulting data.
> 
> This is caused by avoiding double compression of the initramfs contents.
> For example on my tests, choosing CONFIG_INITRAMFS_COMPRESSION_NONE when
> compressing the kernel using XZ) results in up to 500KiB differences (9MiB to
>  8.5MiB) in the kernel size as the dictionary will not get polluted with
> uncomprensible data and may reuse kernel data too.
> 
> Despite embedding an uncompressed initramfs, a user may want to allow for a
> compressed extra initramfs to be passed using the rd system, for example to
> boot a recovery system. Commit 9ba4bcb645898d562498ea66a0df958ef0e7a68c
> ("initramfs: read CONFIG_RD_ variables for initramfs compression") broke
> that behavior by making the choice based on CONFIG_RD_* instead of adding
> CONFIG_INITRAMFS_COMPRESSION_LZ4. Saddly, CONFIG_RD_* is also used to
> choose the supported RD compression algorithms by the kernel and a user may
> want to suppport more than one.
> 
> This patch also reverses 3e4e0f0a8756dade3023d1f47d50fbced7749788
> ("initramfs: remove "compression mode" choice") restoring back the
> "compression mode" choice and includes the CONFIG_INITRAMFS_COMPRESSION_LZ4
> option which was never added.
> 
> As a result the following options are added or readed affecting the embedded
> initramfs compression:
> INITRAMFS_COMPRESSION_NONE Do no compression
> INITRAMFS_COMPRESSION_GZIP Compress using gzip
> INITRAMFS_COMPRESSION_BZIP2 Compress using bzip2
> INITRAMFS_COMPRESSION_LZMA Compress using lzma
> INITRAMFS_COMPRESSION_XZ Compress using xz
> INITRAMFS_COMPRESSION_LZO Compress using lzo
> INITRAMFS_COMPRESSION_LZ4 Compress using lz4
> 
> These depend on the corresponding CONFIG_RD_* option being set (except NONE
> which has no dependencies).
> 
> This patch depends on the previous one (the previous version didn't) to
> simplify the way in which the algorithm is chosen and keep backwards
> compatibility with the behaviour introduced by commit
>  9ba4bcb645898d562498ea66a0df958ef0e7a68c
> 
> Signed-off-by: Francisco Blas Izquierdo Riera (klondike) <klondike@...ndike.es>
> Cc: P J P <ppandit@...hat.com>
> Cc: Paul Bolle <pebolle@...cali.nl>
> Cc: Andrew Morton <akpm@...ux-foundation.org>

Running a bisection against usr/ was not particularly convincing but
here is basically what I am observing which used to work just fine
before as of v4.9 and since I tracked it to this particular
commit/patch. Here is what my build system does:

- kernel is initially configured not to have an initramfs included
- build the user space root file system
- re-configure the kernel to have an initramfs included
(CONFIG_INITRAMFS_SOURCE="/path/to/romfs") and set relevant
CONFIG_INITRAMFS options, in my case, no compression
- kernel is re-built with these options -> kernel+initramfs image is copied
- kernel is re-built again without these options -> kernel image is copied

Now suppose you make changes to your root filesystem, like add/remove
applications, initramfs_data.cpio is now a stale file and go through the
same process again:

- build the kernel without an initramfs
- user space (re)build
- build the kernel with an initramfs

Building a kernel without an initramfs means setting this option:

CONFIG_INITRAMFS_SOURCE=""

whereas building a kernel with an initramfs means setting these options:

CONFIG_INITRAMFS_SOURCE="/home/fainelli/work/uclinux-rootfs/romfs
/home/fainelli/work/uclinux-rootfs/misc/initramfs.dev"
CONFIG_INITRAMFS_ROOT_UID=1000
CONFIG_INITRAMFS_ROOT_GID=1000
CONFIG_INITRAMFS_COMPRESSION_NONE=y
# CONFIG_INITRAMFS_COMPRESSION_GZIP is not set
# CONFIG_INITRAMFS_COMPRESSION_BZIP2 is not set
# CONFIG_INITRAMFS_COMPRESSION_LZMA is not set
# CONFIG_INITRAMFS_COMPRESSION_XZ is not set
# CONFIG_INITRAMFS_COMPRESSION_LZO is not set
# CONFIG_INITRAMFS_COMPRESSION_LZ4 is not set
CONFIG_INITRAMFS_COMPRESSION=""

Commit db2aa7fd15e857891cefbada8348c8d938c7a2bc ("initramfs: allow again
choice of the embedded initram compression algorithm") appears
problematic because CONFIG_INITRAMFS_COMPRESSION which is used to
determine the initramfs cpio extension/compression is a string, and due
to how Kconfig works, it will evaluate, in order, how to assign it.
Setting CONFIG_INITRAMFS_COMPRESSION_NONE with
CONFIG_INITRAMFS_SOURCE="" cannot possibly work (because of the depends
on INITRAMFS_SOURCE!="") yet we still manage to get it assigned to
something: ".gz" because CONFIG_RD_GZIP=y is set in my kernel, even when
there is no initramfs being built.

So we basically end-up generating two initramfs_data.cpio* files, one
without extension, and one with .gz. This causes usr/Makefile to track
usr/initramfs_data.cpio.gz, and not usr/initramfs_data.cpio, that is
also problematic after 9e3596b0c6539e28546ff7c72a06576627068353
("kbuild: initramfs cleanup, set target from Kconfig") because we used
to track everything in $(targets) before that.

The end result is that the kernel with an initramfs clearly does not
contain what we expect it to, it has a stale initramfs_data.cpio file
built into.

So these two specific commits have just rendered the whole thing
unworkable for me, because of how string config options and how they get
assigned they default values work.

So right now, we are completely at the mercy of whichever was the first
CONFIG_RD_* compression option set and the whole process which I
described where we try to build two kernels one after the other is just
badly broken...

Happy to test any patch, otherwise I am just considering making hiding
INITRAMFS_COMPRESSION depends on INITRAMFS_SOURCE!= to make results more
predictable.

Thoughts?

> ---
> I'm sorry for the noise. I'm resending this as a new version because
> Thunderbird likes to word wrap things it shouldn't be word wrapping.
> 
> diff --git a/usr/Kconfig b/usr/Kconfig
> index bf8e8f1..6278f13 100644
> --- a/usr/Kconfig
> +++ b/usr/Kconfig
> @@ -99,8 +99,125 @@ config RD_LZ4
>  	  Support loading of a LZ4 encoded initial ramdisk or cpio buffer
>  	  If unsure, say N.
>  
> +choice
> +	prompt "Built-in initramfs compression mode"
> +	depends on INITRAMFS_SOURCE!=""
> +	optional
> +	help
> +	  This option allows you to decide by which algorithm the builtin
> +	  initramfs will be compressed.  Several compression algorithms are
> +	  available, which differ in efficiency, compression and
> +	  decompression speed.  Compression speed is only relevant
> +	  when building a kernel.  Decompression speed is relevant at
> +	  each boot. Also the memory usage during decompression may become
> +	  relevant on memory constrained systems. This is usually based on the
> +	  dictionary size of the algorithm with algorithms like XZ and LZMA
> +	  featuring large dictionary sizes.
> +
> +	  High compression options are mostly useful for users who are
> +	  low on RAM, since it reduces the memory consumption during
> +	  boot.
> +
> +	  Keep in mind that your build system needs to provide the appropriate
> +	  compression tool to compress the generated initram cpio file for
> +	  embedding.
> +
> +	  If in doubt, select 'None'
> +
> +config INITRAMFS_COMPRESSION_NONE
> +	bool "None"
> +	help
> +	  Do not compress the built-in initramfs at all. This may sound wasteful
> +	  in space, but, you should be aware that the built-in initramfs will be
> +	  compressed at a later stage anyways along with the rest of the kernel,
> +	  on those architectures that support this. However, not compressing the
> +	  initramfs may lead to slightly higher memory consumption during a
> +	  short time at boot, while both the cpio image and the unpacked
> +	  filesystem image will be present in memory simultaneously
> +
> +config INITRAMFS_COMPRESSION_GZIP
> +	bool "Gzip"
> +	depends on RD_GZIP
> +	help
> +	  Use the old and well tested gzip compression algorithm. Gzip provides
> +	  a good balance between compression ratio and decompression speed and
> +	  has a reasonable compression speed. It is also more likely to be
> +	  supported by your build system as the gzip tool is present by default
> +	  on most distros.
> +
> +config INITRAMFS_COMPRESSION_BZIP2
> +	bool "Bzip2"
> +	depends on RD_BZIP2
> +	help
> +	  It's compression ratio and speed is intermediate. Decompression speed
> +	  is slowest among the choices. The initramfs size is about 10% smaller
> +	  with bzip2, in comparison to gzip. Bzip2 uses a large amount of
> +	  memory. For modern kernels you will need at least 8MB RAM or more for
> +	  booting.
> +
> +	  If you choose this, keep in mind that you need to have the bzip2 tool
> +	  available to be able to compress the initram.
> +
> +config INITRAMFS_COMPRESSION_LZMA
> +	bool "LZMA"
> +	depends on RD_LZMA
> +	help
> +	  This algorithm's compression ratio is best but has a large dictionary
> +	  size which might cause issues in memory constrained systems.
> +	  Decompression speed is between the other choices. Compression is
> +	  slowest. The initramfs size is about 33% smaller with LZMA in
> +	  comparison to gzip.
> +
> +	  If you choose this, keep in mind that you may need to install the xz
> +	  or lzma tools to be able to compress the initram.
> +
> +config INITRAMFS_COMPRESSION_XZ
> +	bool "XZ"
> +	depends on RD_XZ
> +	help
> +	  XZ uses the LZMA2 algorithm and has a large dictionary which may cause
> +	  problems on memory constrained systems. The initramfs size is about
> +	  30% smaller with XZ in comparison to gzip. Decompression speed is
> +	  better than that of bzip2 but worse than gzip and LZO. Compression is
> +	  slow.
> +
> +	  If you choose this, keep in mind that you may need to install the xz
> +	  tool to be able to compress the initram.
> +
> +config INITRAMFS_COMPRESSION_LZO
> +	bool "LZO"
> +	depends on RD_LZO
> +	help
> +	  It's compression ratio is the second poorest amongst the choices. The
> +	  kernel size is about 10% bigger than gzip. Despite that, it's
> +	  decompression speed is the second fastest and it's compression speed
> +	  is quite fast too.
> +
> +	  If you choose this, keep in mind that you may need to install the lzop
> +	  tool to be able to compress the initram.
> +
> +config INITRAMFS_COMPRESSION_LZ4
> +	bool "LZ4"
> +	depends on RD_LZ4
> +	help
> +	  It's compression ratio is the poorest amongst the choices. The kernel
> +	  size is about 15% bigger than gzip; however its decompression speed
> +	  is the fastest.
> +
> +	  If you choose this, keep in mind that most distros don't provide lz4
> +	  by default which could cause a build failure.
> +
> +endchoice
> +
>  config INITRAMFS_COMPRESSION
>  	string
> +	default ""      if INITRAMFS_COMPRESSION_NONE
> +	default ".gz"   if INITRAMFS_COMPRESSION_GZIP
> +	default ".bz2"  if INITRAMFS_COMPRESSION_BZIP2
> +	default ".lzma" if INITRAMFS_COMPRESSION_LZMA
> +	default ".xz"   if INITRAMFS_COMPRESSION_XZ
> +	default ".lzo"  if INITRAMFS_COMPRESSION_LZO
> +	default ".lz4"  if INITRAMFS_COMPRESSION_LZ4
>  	default ".gz"   if RD_GZIP
>  	default ".lz4"  if RD_LZ4
>  	default ".lzo"  if RD_LZO
> 

-- 
Florian

Powered by blists - more mailing lists