Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 2 Mar 2021 16:26:53 +0000
From:   Luis Chamberlain <mcgrof@kernel.org>
To:     Rasmus Villemoes <linux@rasmusvillemoes.dk>,
        Borislav Petkov <bp@alien8.de>, Jessica Yu <jeyu@kernel.org>,
        Takashi Iwai <tiwai@suse.de>
Cc:     linux-kernel@vger.kernel.org,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Jonathan Corbet <corbet@lwn.net>, linux-doc@vger.kernel.org,
        Nick Desaulniers <ndesaulniers@google.com>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Peter Zijlstra <peterz@infradead.org>
Subject: Re: [PATCH/RFC 1/2] init/initramfs.c: allow asynchronous unpacking
Message-ID: <20210302162653.GO4332@42.do-not-panic.com>
References: <20210224142909.2092914-1-linux@rasmusvillemoes.dk>
 <20210224142909.2092914-2-linux@rasmusvillemoes.dk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210224142909.2092914-2-linux@rasmusvillemoes.dk>
Precedence: bulk

On Wed, Feb 24, 2021 at 03:29:08PM +0100, Rasmus Villemoes wrote:
> This is primarily motivated by an embedded ppc target, where unpacking
> even the rather modest sized initramfs takes 0.6 seconds, which is
> long enough that the external watchdog becomes unhappy that it doesn't
> get enough attention soon enough.
> 
> But normal desktops might benefit as well. On my mostly stock Ubuntu
> kernel, my initramfs is a 26M xz-compressed blob, decompressing to
> around 126M. That takes almost two seconds.
> 
> So add an initramfs_async= kernel parameter, allowing the main init
> process to proceed to handling device_initcall()s without waiting for
> populate_rootfs() to finish.
> 
> Should one of those initcalls need something from the initramfs (say,
> a kernel module or a firmware blob), it will simply wait for the
> initramfs unpacking to be done before proceeding, which should in
> theory make this completely safe to always enable. But if some driver
> pokes around in the filesystem directly and not via one of the
> official kernel interfaces (i.e. request_firmware*(),
> call_usermodehelper*) that theory may not hold - also, I certainly
> might have missed a spot when sprinkling wait_for_initramfs().
> 
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
>  .../admin-guide/kernel-parameters.txt         | 12 +++++
>  drivers/base/firmware_loader/main.c           |  2 +
>  include/linux/initrd.h                        |  7 +++
>  init/initramfs.c                              | 51 ++++++++++++++++++-
>  init/main.c                                   |  1 +
>  kernel/umh.c                                  |  2 +
>  usr/Kconfig                                   | 10 ++++
>  7 files changed, 84 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 0ac883777318..e9aca86d429b 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -1820,6 +1820,18 @@
>  			initcall functions.  Useful for debugging built-in
>  			modules and initcalls.
>  
> +	initramfs_async= [KNL] Normally, the initramfs image is
> +			unpacked synchronously, before most devices
> +			are initialized. When the initramfs is huge,
> +			or on slow CPUs, this can take a significant
> +			amount of time. Setting this option means the
> +			unpacking is instead done in a background
> +			thread, allowing the main init process to
> +			begin calling device_initcall()s while the
> +			initramfs is being unpacked.
> +			Format: <bool>
> +			Default set by CONFIG_INITRAMFS_ASYNC.
> +
>  	initrd=		[BOOT] Specify the location of the initial ramdisk
>  
>  	initrdmem=	[KNL] Specify a physical address and size from which to
> diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
> index 78355095e00d..4fdb8219cd08 100644
> --- a/drivers/base/firmware_loader/main.c
> +++ b/drivers/base/firmware_loader/main.c
> @@ -15,6 +15,7 @@
>  #include <linux/kernel_read_file.h>
>  #include <linux/module.h>
>  #include <linux/init.h>
> +#include <linux/initrd.h>
>  #include <linux/timer.h>
>  #include <linux/vmalloc.h>
>  #include <linux/interrupt.h>
> @@ -504,6 +505,7 @@ fw_get_filesystem_firmware(struct device *device, struct fw_priv *fw_priv,
>  	if (!path)
>  		return -ENOMEM;
>  
> +	wait_for_initramfs();

Some folks might want this to not wait, say for folks who use built-in
firmware, but for such use cases a new API which *purposely* only look
for builtin-firmware would resolve that. The only case I think think of
that folks may explicitly want this today is in
arch/x86/kernel/cpu/microcode/, see get_builtin_firmware() calls, those
should use a proper API, not a hack-in solution like that.
I think Boris was working on this long ago, but he's as usual busy.

But since this use the builtin stuff directly it is not affected. And
even if it was affected by this delay, it would have been before.

Other than what Linus pointed out, I see no reason why folks could
experiment with this, in fact I welcome it.

  Luis