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 <[email protected]> wrote: >> >>> On 08/28/2017 08:09 PM, Nicholas Piggin wrote: >>>> On Mon, 28 Aug 2017 13:03:31 -0700 >>>> Florian Fainelli <[email protected]> wrote: >>>> >>>>> On 05/21/2017 07:46 PM, Nicholas Piggin wrote: >>>>>> On Sat, 20 May 2017 20:33:35 -0700 >>>>>> Florian Fainelli <[email protected]> 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 <[email protected]> >>>>>> >>>>>> 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 <[email protected]> >>>>> >>>>> 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? > > diff --git a/usr/Makefile b/usr/Makefile > index 0b87e71c00fc..2d7d7f4b91f1 100644 > --- a/usr/Makefile > +++ b/usr/Makefile > @@ -7,6 +7,7 @@ PHONY += klibcdirs > > suffix_y = $(subst $\",,$(CONFIG_INITRAMFS_COMPRESSION)) > datafile_y = initramfs_data.cpio$(suffix_y) > +datafile_d_y = .$(datafile_y).d > AFLAGS_initramfs_data.o += -DINITRAMFS_IMAGE="usr/$(datafile_y)" > > > @@ -29,12 +30,12 @@ ramfs-args := \ > $(if $(CONFIG_INITRAMFS_ROOT_UID), -u > $(CONFIG_INITRAMFS_ROOT_UID)) \ > $(if $(CONFIG_INITRAMFS_ROOT_GID), -g $(CONFIG_INITRAMFS_ROOT_GID)) > > -# .initramfs_data.cpio.d is used to identify all files included > +# $(datafile_d_y) is used to identify all files included > # in initramfs and to detect if any files are added/removed. > # Removed files are identified by directory timestamp being updated > # The dependency list is generated by gen_initramfs.sh -l > -ifneq ($(wildcard $(obj)/.initramfs_data.cpio.d),) > - include $(obj)/.initramfs_data.cpio.d > +ifneq ($(wildcard $(obj)/$(datafile_d_y)),) > + include $(obj)/$(datafile_d_y) > endif > > quiet_cmd_initfs = GEN $@ > @@ -52,5 +53,5 @@ $(deps_initramfs): klibcdirs > # 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 > - $(Q)$(initramfs) -l $(ramfs-input) > $(obj)/.initramfs_data.cpio.d > + $(Q)$(initramfs) -l $(ramfs-input) > $(obj)/$(datafile_d_y) > $(call if_changed,initfs) > -- Florian
