Message-ID: <47B0DD34.4010604@tremplin-utc.net>
Date:	Tue, 12 Feb 2008 00:41:40 +0100
From:	Éric Piel <Eric.Piel@...mplin-utc.net>
To:	Sergey Vlasov <vsu@...linux.ru>
CC:	Christoph Hellwig <hch@....de>, dsdt@...gusch.at,
	len.brown@...el.com, linux-kernel@...r.kernel.org,
	Linus Torvalds <torvalds@...l.org>, trenn@...e.de
Subject: Re: acpi dsts loading and populate_rootfs

11/02/08 14:47, Sergey Vlasov wrote/a écrit:
> On Sun, 10 Feb 2008 12:58:09 +0100 Eric Piel wrote:
> 
>> I guess the problem that Linus solved by moving populate_rootfs()
>> happens only rarely or on only few configurations. Linus, do you
>> remember what kind of problem it was? How can I reproduce it?
> 
> AFAIR the problem was that the kernel was trying to exec /sbin/hotplug
> before some kernel subsystems (e.g., pipe support) were completely
> initialized, which caused oopses during boot.
> 
> Initramfs images generated by distro tools usually do not contain
> /sbin/hotplug, and therefore avoid the problem.  (The kernel might
> also call /sbin/modprobe by itself, but it either does not do it
> during early boot, or /sbin/modprobe does not use kernel features
> which had not been initialized yet).
Thanks a lot for the pointer. I was able to find the original bug
report. After reading the various fix proposals and the current code I
think I understand what is going on.

Basically, the problem is that one should not run a user land program
too early because not everything is initialized yet (for instance
pipefs). This might happen because at initialization some drivers can
trigger automatically /sbin/hotplug (via call_usermodehelper()). So the
fix which was applied consists in populating the rootfs only once all
the kernel subsystem is initialized. In this way, the hotplugging is
still triggered but as the user space program doesn't exist it can't be
called! (hehe)

>> One different solution that I implemented [1] was to have an
>> "early_populate_rootfs()" before the acpi_early_init(), leaving
>> populate_rootfs() at the normal place. If the early version fails, it's
>> ok: we can't override the DSDT, but the later version will work as usual
>> anyway. This solution also seems to work quite often (it's used in
>> Ubuntu, Mandriva and PCLinuxOS)
> 
> This would not solve the problem, because /sbin/hotplug (if present in
> the initramfs image) will exist too early.
Yes, now that I understand the problem, reading the initramfs twice
would in no way help!

>> Would that seem an acceptable solution? Or what other way exists?
> 
> Disabling call_usermodehelper() until all core initializers had
> completed would fix the problem too; will such change be acceptable?
Yes, that looks like a nice way. Actually, I discovered that for suspend
and resume, there is exactly the same need, and a special flag is
already available: usermodehelper_disabled.

So here is a proposal patch leveraging this flag. The rootfs is
populated early but user land execution is forbidden until we reach a
sufficient initialization state.  I think it even has the bonus of
doing _explicitly_ what we want (avoid user land execution).

Does this look good? (if so, I can probably make it even cleaner by
renaming/removing the rootfs_initcall level)

Eric
--

diff --git a/init/initramfs.c b/init/initramfs.c
index c0b1e05..b74e845 100644
--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -577,10 +577,3 @@ int __init populate_rootfs(void)
 	}
 	return 0;
 }
-#ifndef CONFIG_ACPI_CUSTOM_DSDT_INITRD
-/*
- * if this option is enabled, populate_rootfs() is called _earlier_ in the
- * boot sequence. This insures that the ACPI initialisation can find the file.
- */
-rootfs_initcall(populate_rootfs);
-#endif
diff --git a/init/main.c b/init/main.c
index 8b19820..703ded0 100644
--- a/init/main.c
+++ b/init/main.c
@@ -102,11 +102,7 @@ static inline void mark_rodata_ro(void) { }
 extern void tc_init(void);
 #endif
 
-#ifdef CONFIG_ACPI_CUSTOM_DSDT_INITRD
 extern int populate_rootfs(void);
-#else
-static inline void populate_rootfs(void) {}
-#endif
 
 enum system_states system_state;
 EXPORT_SYMBOL(system_state);
diff --git a/kernel/kmod.c b/kernel/kmod.c
index bb7df2a..d6a2b40 100644
--- a/kernel/kmod.c
+++ b/kernel/kmod.c
@@ -267,14 +267,23 @@ static void __call_usermodehelper(struct work_struct *work)
 	}
 }
 
-#ifdef CONFIG_PM
 /*
  * If set, call_usermodehelper_exec() will exit immediately returning -EBUSY
  * (used for preventing user land processes from being created after the user
  * land has been frozen during a system-wide hibernation or suspend operation).
+ * It is also used at boot up to avoid calling a user land process before all
+ * the kernel subsystems are initialised (such as pipefs).
  */
-static int usermodehelper_disabled;
+static int usermodehelper_disabled = 1;
 
+static int __init enable_usermodehelper(void)
+{
+	usermodehelper_disabled = 0;
+	return 0;
+}
+rootfs_initcall(enable_usermodehelper);
+
+#ifdef CONFIG_PM
 /* Number of helpers running */
 static atomic_t running_helpers = ATOMIC_INIT(0);
 
@@ -342,8 +351,6 @@ static void register_pm_notifier_callback(void)
 	pm_notifier(usermodehelper_pm_callback, 0);
 }
 #else /* CONFIG_PM */
-#define usermodehelper_disabled	0
-
 static inline void helper_lock(void) {}
 static inline void helper_unlock(void) {}
 static inline void register_pm_notifier_callback(void) {}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists