From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751385AbdH1UDk (ORCPT ); Mon, 28 Aug 2017 16:03:40 -0400 Received: from mail-wr0-f193.google.com ([209.85.128.193]:33093 "EHLO mail-wr0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751190AbdH1UDi (ORCPT ); Mon, 28 Aug 2017 16:03:38 -0400 Subject: Re: [PATCH] initramfs: Fix disabling of initramfs (and its compression) To: Nicholas Piggin , "Francisco Blas Izquierdo Riera (klondike)" Cc: linux-kernel@vger.kernel.org, ppandit@redhat.com, pebolle@tiscali.nl, mmarek@suse.cz, Andrew Morton , Daniel Thompson References: <20170521033337.6197-1-f.fainelli@gmail.com> <20170522124652.1bcd7749@roar.ozlabs.ibm.com> From: Florian Fainelli Message-ID: <65695e3a-e471-5bdb-868c-63adc8b0ddf8@gmail.com> Date: Mon, 28 Aug 2017 13:03:31 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <20170522124652.1bcd7749@roar.ozlabs.ibm.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 05/21/2017 07:46 PM, Nicholas Piggin wrote: > On Sat, 20 May 2017 20:33:35 -0700 > Florian Fainelli 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 > > 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 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? > >> --- >> usr/Kconfig | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/usr/Kconfig b/usr/Kconfig >> index c0c48507e44e..ad0543e21760 100644 >> --- a/usr/Kconfig >> +++ b/usr/Kconfig >> @@ -220,6 +220,7 @@ config INITRAMFS_COMPRESSION_LZ4 >> endchoice >> >> config INITRAMFS_COMPRESSION >> + depends on INITRAMFS_SOURCE!="" >> string >> default "" if INITRAMFS_COMPRESSION_NONE >> default ".gz" if INITRAMFS_COMPRESSION_GZIP > -- Florian