diff mbox

[v2,0/2] Populate rootfs asynchronously and early

Message ID 1260265475-4303-2-git-send-email-surbhi.palande@canonical.com
State Accepted
Delegated to: Andy Whitcroft
Headers show

Commit Message

Surbhi Palande Dec. 8, 2009, 9:44 a.m. UTC
populate_rootfs() is called asynchronously to reduce the boot time.

Signed-off-by: Surbhi Palande <surbhi.palande@canonical.com>
---
 include/linux/init.h |    2 ++
 init/initramfs.c     |   15 ++++++++++++---
 init/main.c          |    6 ++++++
 3 files changed, 20 insertions(+), 3 deletions(-)

Comments

Andy Whitcroft Dec. 8, 2009, 7:13 p.m. UTC | #1
On Tue, Dec 08, 2009 at 11:44:34AM +0200, Surbhi Palande wrote:
> populate_rootfs() is called asynchronously to reduce the boot time.

This comment I made on a previous version, but it got lost in my last
round of comments so I repeat it here:

This could do with some more words of explanation.  We should be telling
them why this is true and why it is safe.

"The expansion of the initramfs is completely independant of other boot
activities.  The original data is already present at boot and the
filesystem is not required until we are ready to start init.  It is
therefore reasonable to populate the rootfs asynchronously.  Move this
processing to an async call."

-apw
Scott James Remnant Dec. 8, 2009, 7:42 p.m. UTC | #2
On Tue, 2009-12-08 at 19:13 +0000, Andy Whitcroft wrote:

> On Tue, Dec 08, 2009 at 11:44:34AM +0200, Surbhi Palande wrote:
> > populate_rootfs() is called asynchronously to reduce the boot time.
> 
> This comment I made on a previous version, but it got lost in my last
> round of comments so I repeat it here:
> 
> This could do with some more words of explanation.  We should be telling
> them why this is true and why it is safe.
> 
> "The expansion of the initramfs is completely independant of other boot
> activities.  The original data is already present at boot and the
> filesystem is not required until we are ready to start init.  It is
> therefore reasonable to populate the rootfs asynchronously.  Move this
> processing to an async call."
> 
It's worth pointing out that the upstream kernel folks probably won't
agree with this straight away.

Part of the original goal of the initramfs was that the kernel might
actually want to call out to it for its own init purposes before
actually passing over to it's real init.

Their usual example is things like firmware loading, or that crazy
kernel-calls-modprobe stuff that won me the Dell Tablet :p


The simple fact is that it never worked, because there's more to making
the initramfs *ready* than just unpacking it.  We need filesystems
like /proc and /sys mounted in it, we need to run depmod inside it (so
that modprobe works), you need udev running to process firmware
requests, etc.


Having populate_rootfs() synchronous makes sense for the *original*
design goals of the initramfs (vs. initrd).  However since no userspace
ever implemented it that way, and things have moved on since then with
regards to kernel/userspace communication, if you were doing it now
you'd make it async.  Which we've done.

This may take some arguing upstream.

Plus you can guarantee that certain Dutch-born Intel people will stick
their oar in about how it's not worth accepting the patch because you
shouldn't have an initramfs anyway.

Scott
Andy Whitcroft Dec. 8, 2009, 9:14 p.m. UTC | #3
On Tue, Dec 08, 2009 at 07:42:47PM +0000, Scott James Remnant wrote:

> Part of the original goal of the initramfs was that the kernel might
> actually want to call out to it for its own init purposes before
> actually passing over to it's real init.
> 
> Their usual example is things like firmware loading, or that crazy
> kernel-calls-modprobe stuff that won me the Dell Tablet :p
> 
> 
> The simple fact is that it never worked, because there's more to making
> the initramfs *ready* than just unpacking it.  We need filesystems
> like /proc and /sys mounted in it, we need to run depmod inside it (so
> that modprobe works), you need udev running to process firmware
> requests, etc.
> 
> 
> Having populate_rootfs() synchronous makes sense for the *original*
> design goals of the initramfs (vs. initrd).  However since no userspace
> ever implemented it that way, and things have moved on since then with
> regards to kernel/userspace communication, if you were doing it now
> you'd make it async.  Which we've done.
> 
> This may take some arguing upstream.
> 
> Plus you can guarantee that certain Dutch-born Intel people will stick
> their oar in about how it's not worth accepting the patch because you
> shouldn't have an initramfs anyway.

Yeah, the nice thing about the _domain approach is that the first
consumer of the initramfs can wait for it then.

The other thing we can do is to make this a configurable option so that
distros can opt-in as appropriate.

-apw
diff mbox

Patch

diff --git a/include/linux/init.h b/include/linux/init.h
index ff8bde5..b57935f 100644
--- a/include/linux/init.h
+++ b/include/linux/init.h
@@ -213,6 +213,8 @@  extern void (*late_time_init)(void);
 	static initcall_t __initcall_##fn \
 	__used __section(.security_initcall.init) = fn
 
+extern struct list_head  populate_rootfs_domain;
+
 struct obs_kernel_param {
 	const char *str;
 	int (*setup_func)(char *);
diff --git a/init/initramfs.c b/init/initramfs.c
index 4c00edc..b8f9e3c 100644
--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -8,6 +8,7 @@ 
 #include <linux/dirent.h>
 #include <linux/syscalls.h>
 #include <linux/utime.h>
+#include <linux/async.h>
 
 static __initdata char *message;
 static void __init error(char *x)
@@ -565,7 +566,9 @@  static void __init clean_rootfs(void)
 }
 #endif
 
-static int __init populate_rootfs(void)
+LIST_HEAD(populate_rootfs_domain);
+
+static void __init async_populate_rootfs(void)
 {
 	char *err = unpack_to_rootfs(__initramfs_start,
 			 __initramfs_end - __initramfs_start);
@@ -579,7 +582,7 @@  static int __init populate_rootfs(void)
 			initrd_end - initrd_start);
 		if (!err) {
 			free_initrd();
-			return 0;
+			return;
 		} else {
 			clean_rootfs();
 			unpack_to_rootfs(__initramfs_start,
@@ -603,6 +606,12 @@  static int __init populate_rootfs(void)
 		free_initrd();
 #endif
 	}
-	return 0;
+	return;
 }
+
+static int __init populate_rootfs(void)
+{
+	async_schedule_domain(async_populate_rootfs, NULL, &populate_rootfs_domain);
+}
+
 rootfs_initcall(populate_rootfs);
diff --git a/init/main.c b/init/main.c
index 28bc963..3b74ddf 100644
--- a/init/main.c
+++ b/init/main.c
@@ -884,6 +884,12 @@  static int __init kernel_init(void * unused)
 	do_basic_setup();
 
 	/*
+	 * We need to ensure that the filesystem is ready by this point, wait for
+	 * async_populate_rootfs to complete.
+	 */
+	async_synchronize_full_domain(&populate_rootfs_domain);
+
+	/*
 	 * check if there is an early userspace init.  If yes, let it do all
 	 * the work
 	 */