LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: "Éric Piel" <Eric.Piel@tremplin-utc.net>
To: Sergey Vlasov <vsu@altlinux.ru>
Cc: Christoph Hellwig <hch@lst.de>,
	dsdt@gaugusch.at, len.brown@intel.com,
	linux-kernel@vger.kernel.org, Linus Torvalds <torvalds@osdl.org>,
	trenn@suse.de
Subject: Re: acpi dsts loading and populate_rootfs
Date: Tue, 12 Feb 2008 00:41:40 +0100	[thread overview]
Message-ID: <47B0DD34.4010604@tremplin-utc.net> (raw)
In-Reply-To: <20080211164719.d42c618e.vsu@altlinux.ru>

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) {}

  reply	other threads:[~2008-02-11 23:42 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-10  7:12 Christoph Hellwig
2008-02-10  7:14 ` Christoph Hellwig
2008-02-10 11:58   ` Eric Piel
2008-02-11 13:47     ` Sergey Vlasov
2008-02-11 23:41       ` Éric Piel [this message]
2008-02-21 19:02         ` [PATCH] Allow populate_rootfs() to be called early (was: acpi dsts loading and populate_rootfs) Éric Piel
2008-02-21 19:04           ` Christoph Hellwig
2008-02-21 21:27             ` [PATCH] Allow populate_rootfs() to be called early Éric Piel
2008-02-23 19:40           ` [PATCH] Allow populate_rootfs() to be called early (resent, with sob) Éric Piel
2008-02-12  5:37     ` acpi dsts loading and populate_rootfs Christoph Hellwig
2008-02-21 18:46       ` Éric Piel
2008-02-22  8:51         ` Thomas Renninger
2008-02-22  9:05           ` Thomas Renninger
2008-02-22  9:53           ` Andi Kleen
2008-02-23 19:34         ` [PATCH] Use userland-like functions for reading the ACPI table Éric Piel
2008-02-23 20:45           ` Linus Torvalds
2008-02-24  1:31             ` Christoph Hellwig
2008-02-24 19:02               ` Éric Piel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=47B0DD34.4010604@tremplin-utc.net \
    --to=eric.piel@tremplin-utc.net \
    --cc=dsdt@gaugusch.at \
    --cc=hch@lst.de \
    --cc=len.brown@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@osdl.org \
    --cc=trenn@suse.de \
    --cc=vsu@altlinux.ru \
    --subject='Re: acpi dsts loading and populate_rootfs' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).