From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751300AbdH2DJd (ORCPT ); Mon, 28 Aug 2017 23:09:33 -0400 Received: from mail-pf0-f193.google.com ([209.85.192.193]:36120 "EHLO mail-pf0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751218AbdH2DJc (ORCPT ); Mon, 28 Aug 2017 23:09:32 -0400 Date: Tue, 29 Aug 2017 13:09:16 +1000 From: Nicholas Piggin To: Florian Fainelli Cc: "Francisco Blas Izquierdo Riera (klondike)" , linux-kernel@vger.kernel.org, ppandit@redhat.com, pebolle@tiscali.nl, mmarek@suse.cz, Andrew Morton , Daniel Thompson Subject: Re: [PATCH] initramfs: Fix disabling of initramfs (and its compression) Message-ID: <20170829130916.27054261@roar.ozlabs.ibm.com> In-Reply-To: <65695e3a-e471-5bdb-868c-63adc8b0ddf8@gmail.com> References: <20170521033337.6197-1-f.fainelli@gmail.com> <20170522124652.1bcd7749@roar.ozlabs.ibm.com> <65695e3a-e471-5bdb-868c-63adc8b0ddf8@gmail.com> Organization: IBM X-Mailer: Claws Mail 3.15.0-dirty (GTK+ 2.24.31; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 28 Aug 2017 13:03:31 -0700 Florian Fainelli wrote: > 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? 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. Thanks, Nick