Messages in this thread | | | Date | Fri, 15 Sep 2017 10:41:06 +1000 | From | Nicholas Piggin <> | Subject | Re: [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
| |