Re: [PATCH] initramfs: Fix disabling of initramfs (and its compression)

[Florian Fainelli <f.fainelli@gmail.com>]

On 08/28/2017 08:09 PM, Nicholas Piggin wrote:
> On Mon, 28 Aug 2017 13:03:31 -0700
> Florian Fainelli <f.fainelli@gmail.com> wrote:
> 
>> On 05/21/2017 07:46 PM, Nicholas Piggin wrote:
>>> On Sat, 20 May 2017 20:33:35 -0700
>>> Florian Fainelli <f.fainelli@gmail.com> wrote:
>>>   
>>>> Commit db2aa7fd15e8 ("initramfs: allow again choice of the embedded
>>>> initram compression algorithm") introduced the possibility to select the
>>>> initramfs compression algorithm from Kconfig and while this is a nice
>>>> feature it broke the use case described below.
>>>>
>>>> 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 option
>>>> (CONFIG_INITRAMFS_COMPRESSION_NONE)
>>>> - kernel is re-built with these options -> kernel+initramfs image is
>>>>   copied
>>>> - kernel is re-built again without these options -> kernel image is
>>>>   copied
>>>>
>>>> Building a kernel without an initramfs means setting this option:
>>>>
>>>> CONFIG_INITRAMFS_SOURCE="" (and this one only)
>>>>
>>>> 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=""
>>>>
>>>> Commit db2aa7fd15e857891cefbada8348c8d938c7a2bc ("initramfs: allow again
>>>> choice of the embedded initram compression algorithm") is problematic
>>>> because CONFIG_INITRAMFS_COMPRESSION which is used to determine the
>>>> initramfs_data.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!="" imposed on CONFIG_INITRAMFS_COMPRESSION ) yet we
>>>> still get CONFIG_INITRAMFS_COMPRESSION assigned to ".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 anymore,
>>>> that is also largely problematic after
>>>> 9e3596b0c6539e28546ff7c72a06576627068353 ("kbuild: initramfs cleanup,
>>>> set target from Kconfig") because we used to track all possible
>>>> initramfs_data files in the $(targets) variable before that commit.
>>>>
>>>> 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 it, and we keep re-generating an initramfs_data.cpio.gz file
>>>> which is not the one that we want to include in the kernel image proper.
>>>>
>>>> The fix consists in hiding CONFIG_INITRAMFS_COMPRESSION when
>>>> CONFIG_INITRAMFS_SOURCE="". This puts us back in a state to the pre-4.10
>>>> behavior where we can properly disable and re-enable initramfs within
>>>> the same kernel .config file, and be in control of what
>>>> CONFIG_INITRAMFS_COMPRESSION is set to.
>>>>
>>>> Fixes: db2aa7fd15e8 ("initramfs: allow again choice of the embedded initram compression algorithm")
>>>> Fixes: 9e3596b0c653 ("kbuild: initramfs cleanup, set target from Kconfig")
>>>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>  
>>>
>>> This is very thorough, thank you for tracking it down and fixing it.
>>>
>>> I can't say I've worked through the problem in the code, but your
>>> changelog and the proposed fix seem reasonable to me. So for what
>>> it's worth:
>>>
>>> Acked-by: Nicholas Piggin <npiggin@gmail.com>  
>>
>> Well, I am looking at this again, and it's still broken, the same test
>> case is involved, except this time, I am switching beween no-initramfs
>> and initramfs with gzip compression (the key thing is using a
>> compression of some sort). The end result is the following:
>>
>> - change stuff in the rootfs
>> - build the kernel with initramfs, CONFIG_INITRAMFS_COMPRESSION_GZIP=y,
>> usr/initramfs_data.cpio.gz gets generated correctly the first time
>> - build the kernel without initramfs,
>> CONFIG_INITRAMFS_COMPRESSION_NONE=y, usr/initramfs_data.cpio gets generated
>>
>> Now back to step 1 add some files, and we can see that
>> usr/initramfs_data.cpio.gz is now stale from before...
>>
>> So while my earlier fix switched the initramfs w/o compression to no
>> initramfs rebuild, now this does not work because we still have two
>> files left to be tracked:
>>
>> usr/initramfs_data.cpio (no compression, or when initramfs is disabled)
>> and usr/initramfs_data.cpio.$(suffix_y)
>>
>> How would you go about solving this?
> 
> I don't see the problem. When I change back to .gz, modify the source
> directory, and rebuild, it rebuilds a new initramfs_data.cpio.gz here.

This is really puzzling me, so here is what is going on my side:

First time build w/ initramfs enabled:

grep INITRAMFS .config
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 is not set
CONFIG_INITRAMFS_COMPRESSION_GZIP=y
# 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=".gz"

fainelli@fainelli-laptop:[~/../linux]$ ls -la usr
total 6.8M
drwxrwxr-x  2 fainelli fainelli 4.0K Sep  7 05:37 ./
drwxrwxr-x 27 fainelli fainelli  16K Sep  7 05:38 ../
-rw-rw-r--  1 fainelli fainelli  146 Sep  7 05:37 built-in.o
-rw-rw-r--  1 fainelli fainelli  112 Sep  7 05:37 .built-in.o.cmd
-rwxrwxr-x  1 fainelli fainelli  23K Sep  7 05:36 gen_init_cpio*
-rw-rw-r--  1 fainelli fainelli  13K Oct 13  2016 gen_init_cpio.c
-rw-rw-r--  1 fainelli fainelli 3.3K Sep  7 05:36 .gen_init_cpio.cmd
-rw-rw-r--  1 fainelli fainelli  151 Aug 28  2016 .gitignore
-rw-rw-r--  1 fainelli fainelli 9.6K Sep  7 05:36 .initramfs_data.cpio.d
-rw-rw-r--  1 fainelli fainelli 3.3M Sep  7 05:37 initramfs_data.cpio.gz
-rw-rw-r--  1 fainelli fainelli  220 Sep  7 05:37
.initramfs_data.cpio.gz.cmd
-rw-rw-r--  1 fainelli fainelli 3.3M Sep  7 05:37 initramfs_data.o
-rw-rw-r--  1 fainelli fainelli 2.7K Sep  7 05:37 .initramfs_data.o.cmd
-rw-rw-r--  1 fainelli fainelli 1.3K Aug 28  2016 initramfs_data.S
-rw-rw-r--  1 fainelli fainelli 7.8K Aug 18 19:25 Kconfig
-rw-rw-r--  1 fainelli fainelli 2.0K Aug 31 20:46 Makefile
-rw-rw-r--  1 fainelli fainelli    0 Sep  7 05:34 modules.builtin
-rw-rw-r--  1 fainelli fainelli    0 Sep  7 05:35 modules.order

Now let's reconfigure this kernel w/o initramfs and rebuild:

grep INITRAMFS .config
CONFIG_INITRAMFS_SOURCE=""

fainelli@fainelli-laptop:[~/../linux]$ ls -la usr
total 3.6M
drwxrwxr-x  2 fainelli fainelli 4.0K Sep  7 05:44 ./
drwxrwxr-x 27 fainelli fainelli  16K Sep  7 05:44 ../
-rw-rw-r--  1 fainelli fainelli  146 Sep  7 05:44 built-in.o
-rw-rw-r--  1 fainelli fainelli  112 Sep  7 05:44 .built-in.o.cmd
-rwxrwxr-x  1 fainelli fainelli  23K Sep  7 05:36 gen_init_cpio*
-rw-rw-r--  1 fainelli fainelli  13K Oct 13  2016 gen_init_cpio.c
-rw-rw-r--  1 fainelli fainelli 3.3K Sep  7 05:36 .gen_init_cpio.cmd
-rw-rw-r--  1 fainelli fainelli  151 Aug 28  2016 .gitignore
-rw-rw-r--  1 fainelli fainelli  512 Sep  7 05:44 initramfs_data.cpio
-rw-rw-r--  1 fainelli fainelli  105 Sep  7 05:44 .initramfs_data.cpio.cmd
-rw-rw-r--  1 fainelli fainelli   52 Sep  7 05:44 .initramfs_data.cpio.d
-rw-rw-r--  1 fainelli fainelli 3.3M Sep  7 05:37 initramfs_data.cpio.gz
-rw-rw-r--  1 fainelli fainelli  220 Sep  7 05:37
.initramfs_data.cpio.gz.cmd
-rw-rw-r--  1 fainelli fainelli 1.3K Sep  7 05:44 initramfs_data.o
-rw-rw-r--  1 fainelli fainelli 2.7K Sep  7 05:44 .initramfs_data.o.cmd
-rw-rw-r--  1 fainelli fainelli 1.3K Aug 28  2016 initramfs_data.S
-rw-rw-r--  1 fainelli fainelli 7.8K Aug 18 19:25 Kconfig
-rw-rw-r--  1 fainelli fainelli 2.0K Aug 31 20:46 Makefile
-rw-rw-r--  1 fainelli fainelli    0 Sep  7 05:34 modules.builtin
-rw-rw-r--  1 fainelli fainelli    0 Sep  7 05:35 modules.order

Now let's rebuild with a new application added to the root filesystem:

grep INITRAMFS .config
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 is not set
CONFIG_INITRAMFS_COMPRESSION_GZIP=y
# 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=".gz"

We are not fine here:
scripts/kconfig/conf  --silentoldconfig Kconfig
#
# configuration written to .config
#
  CHK     include/config/kernel.release
  CHK     include/generated/uapi/linux/version.h
  CHK     scripts/mod/devicetable-offsets.h
  CHK     include/generated/utsrelease.h
  CHK     include/generated/timeconst.h
  CHK     include/generated/bounds.h
  CHK     include/generated/asm-offsets.h
  CALL    scripts/checksyscalls.sh
  CHK     include/generated/compile.h
  AS      usr/initramfs_data.o

^===== we should have called gzip to re-generate the initramfs, we did not

  AR      usr/built-in.o
  GEN     .version
  CHK     include/generated/compile.h
  UPD     include/generated/compile.h
  CC      init/version.o
  AR      init/built-in.o
  AR      built-in.o
  LD      vmlinux.o
  MODPOST vmlinux.o

And the timestamps here confirm that:

fainelli@fainelli-laptop:[~/../linux]$ ls -la usr
total 6.9M
drwxrwxr-x  2 fainelli fainelli 4.0K Sep  7 05:46 ./
drwxrwxr-x 27 fainelli fainelli  16K Sep  7 05:46 ../
-rw-rw-r--  1 fainelli fainelli  146 Sep  7 05:46 built-in.o
-rw-rw-r--  1 fainelli fainelli  112 Sep  7 05:46 .built-in.o.cmd
-rwxrwxr-x  1 fainelli fainelli  23K Sep  7 05:36 gen_init_cpio*
-rw-rw-r--  1 fainelli fainelli  13K Oct 13  2016 gen_init_cpio.c
-rw-rw-r--  1 fainelli fainelli 3.3K Sep  7 05:36 .gen_init_cpio.cmd
-rw-rw-r--  1 fainelli fainelli  151 Aug 28  2016 .gitignore
-rw-rw-r--  1 fainelli fainelli  512 Sep  7 05:44 initramfs_data.cpio
-rw-rw-r--  1 fainelli fainelli  105 Sep  7 05:44 .initramfs_data.cpio.cmd
-rw-rw-r--  1 fainelli fainelli  11K Sep  7 05:46 .initramfs_data.cpio.d
-rw-rw-r--  1 fainelli fainelli 3.3M Sep  7 05:37 initramfs_data.cpio.gz
-rw-rw-r--  1 fainelli fainelli  220 Sep  7 05:37
.initramfs_data.cpio.gz.cmd
-rw-rw-r--  1 fainelli fainelli 3.3M Sep  7 05:46 initramfs_data.o
-rw-rw-r--  1 fainelli fainelli 2.7K Sep  7 05:46 .initramfs_data.o.cmd
-rw-rw-r--  1 fainelli fainelli 1.3K Aug 28  2016 initramfs_data.S
-rw-rw-r--  1 fainelli fainelli 7.8K Aug 18 19:25 Kconfig
-rw-rw-r--  1 fainelli fainelli 2.0K Aug 31 20:46 Makefile
-rw-rw-r--  1 fainelli fainelli    0 Sep  7 05:46 modules.builtin
-rw-rw-r--  1 fainelli fainelli    0 Sep  7 05:46 modules.order

Worse yet, usr/.initramfs_data.cpio.d does list the binary that was just
added, but for some reason, this did not trigger a rebuild of
initramfs_data.cpio.gz do you see any reasons why?

Thanks!
-- 
Florian