From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932787AbdEVCrJ (ORCPT ); Sun, 21 May 2017 22:47:09 -0400 Received: from mail-pf0-f194.google.com ([209.85.192.194]:36037 "EHLO mail-pf0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932173AbdEVCrH (ORCPT ); Sun, 21 May 2017 22:47:07 -0400 Date: Mon, 22 May 2017 12:46:52 +1000 From: Nicholas Piggin To: Florian Fainelli Cc: linux-kernel@vger.kernel.org, ppandit@redhat.com, pebolle@tiscali.nl, mmarek@suse.cz, Andrew Morton , "Francisco Blas Izquierdo Riera (klondike)" , Daniel Thompson Subject: Re: [PATCH] initramfs: Fix disabling of initramfs (and its compression) Message-ID: <20170522124652.1bcd7749@roar.ozlabs.ibm.com> In-Reply-To: <20170521033337.6197-1-f.fainelli@gmail.com> References: <20170521033337.6197-1-f.fainelli@gmail.com> Organization: IBM X-Mailer: Claws Mail 3.14.1 (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 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 > --- > 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