lkml.org 
[lkml]   [2017]   [Sep]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] initramfs: Fix disabling of initramfs (and its compression)
On Thu, 14 Sep 2017 15:59:50 -0700
Florian Fainelli <f.fainelli@gmail.com> wrote:

> On 09/07/2017 09:36 PM, Florian Fainelli wrote:
> >
> >
> > On 09/07/2017 06:19 AM, Nicholas Piggin wrote:
> >> On Thu, 7 Sep 2017 05:50:30 -0700
> >> Florian Fainelli <f.fainelli@gmail.com> wrote:
> >>
> >>> 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?
> >>
> >> This rule
> >>
> >> $(obj)/$(datafile_y): $(obj)/gen_init_cpio $(deps_initramfs) klibcdirs
> >> $(Q)$(initramfs) -l $(ramfs-input) > $(obj)/.initramfs_data.cpio.d
> >> $(call if_changed,initfs)
> >>
> >> does not have FORCE on the end, so I guess is not picking up the dependency
> >> properly, perhaps?
> >
> > I tried that:
> >
> > diff --git a/usr/Makefile b/usr/Makefile
> > index 0b87e71c00fc..28a958bebd27 100644
> > --- a/usr/Makefile
> > +++ b/usr/Makefile
> > @@ -51,6 +51,6 @@ $(deps_initramfs): klibcdirs
> > # 2) There are changes in which files are included (added or deleted)
> > # 3) If gen_init_cpio are newer than initramfs_data.cpio
> > # 4) arguments to gen_initramfs.sh changes
> > -$(obj)/$(datafile_y): $(obj)/gen_init_cpio $(deps_initramfs) klibcdirs
> > +$(obj)/$(datafile_y): $(obj)/gen_init_cpio $(deps_initramfs) klibcdirs
> > FORCE
> > $(Q)$(initramfs) -l $(ramfs-input) > $(obj)/.initramfs_data.cpio.d
> > $(call if_changed,initf
> >
> > but this did not change anything.
> >
> > We are re-generating .initramfs_data.cpio.d and its timestamp is clearly
> > updated so it's got to be cmd_initfs which does not get invoked.
> >
> > .initramfs_data.cpio.gz.cmd is correct:
> >
> > cmd_usr/initramfs_data.cpio.gz := /bin/bash
> > ./scripts/gen_initramfs_list.sh -o usr/initramfs_data.cpio.gz -u 1000
> > -g 1000 /home/fainelli/work/uclinux-rootfs/romfs
> > /home/fainelli/work/uclinux-rootfs/misc/initramfs.dev
> >
> > and was generated the first time we did generate the gzip initramfs, so
> > the command has not changed, nor its arguments, so we just don't call it.
> >
> > I did try to use if_changed_dep, which rightfully pointed that we don't
> > have a .initramfs_data.cpio.gz.d, but this gets us very close from the
> > only thing that I could get working thus far. Another way to possibly
> > solve it is to void generating a cmd_$($@) and use the same name
> > irrespective of the compression type used, that should force a change
> > everytime...
> >
> > Thanks Nick!
>
> Nick, is the patch below acceptable to you?

Well it looks reasonable, and it fixes your problem. I'm not maintainer
of this code or a Makefile wizard though :) Perhaps send it up to the
linux-kbuild maintainers?

Thanks,
Nick

\
 
 \ /
  Last update: 2017-09-15 02:41    [W:0.115 / U:1.020 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site