From: Rene Herman <rene.herman@keyaccess.nl>
To: Arjan van de Ven <arjan@infradead.org>
Cc: linux-kernel@vger.kernel.org, mingo@elte.hu,
Pavel Machek <pavel@ucw.cz>, "Rafael J. Wysocki" <rjw@sisk.pl>
Subject: Re: [patch 1/3] fastboot: Create a "asynchronous" initlevel
Date: Sun, 20 Jul 2008 09:23:31 +0200 [thread overview]
Message-ID: <4882E7F3.8060605@keyaccess.nl> (raw)
In-Reply-To: <20080719084448.7859afd0@infradead.org>
On 19-07-08 17:44, Arjan van de Ven wrote:
> On Sat, 19 Jul 2008 10:10:09 +0200
> Rene Herman <rene.herman@keyaccess.nl> wrote:
>>> I'm not sure about this comment, being not very sure about the
>>> semantics of late_initcall but shouldn't late_initcall (level 7)
>>> wait for 6s to have completed?
>>
>> Following up on this myself -- see for example kernel/power/disk.c:
>> power_suspend(). It's a late intitcall so that, as it comments,
>> "all devices are discovered and initialized". However, your first
>> followup patch makes the USB HCI init async meaning that any USB
>> storage device might not be ready yet when it runs, no?
>
> good spotting/comment.
>
> you would have a valid point... if it weren't for the case where much
> of this actual "end device" probing is in various cases already
> asynchronous... what you do have found is a bug in the suspend code.
> Unless code does:
> /* wait for the known devices to complete their probing */
> while (driver_probe_done() != 0)
> msleep(100);
> (taken from init/do_mounts.c)
>
> ... the assertion in the comment that probing is done is absolutely
> false, with or without my patches.
Yes, I see. Unfortunately, WITH your patches, driver_probe_done() would
also no longer be safe when run from a late_initcall() it would appear.
driver_probe_done() tests a variable that's incremented just before the
driver model calls into the driver .probe method and decremented on
return from it (really_probe).
However, if the entire module_init() is async the probing may not even
have _started_ yet let alone finished. Let's take ehci_hcd_init() as an
example both since you changed that one and since it'll fairly often be
en route to mass storage devices.
Only after ehci_init() calls foo_register_driver() is the driver model
aware of it and will it start calling the probe methods meaning the
driver_probe_done() would be racing.
I have the sneaking suspicion that this is a bit of a fundamental issue.
Turning some of the driver level (6) async basicaly removes the ordering
between drivers and late_initcall (level 7).
I trust it will completely and utterly destroy the point of this patch
to flush level 6a before starting level 7?
> (Not that I want the suspend/resume code to call this, because that
> would make the boot even longer ;( )
Well, yes, but bugs are bugs. CCing Pavel and Rafael as well :-)
Rene.
next prev parent reply other threads:[~2008-07-20 7:21 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-07-18 22:15 [patch 0/3] fastboot patches series 1 Arjan van de Ven
2008-07-18 22:16 ` [patch 1/3] fastboot: Create a "asynchronous" initlevel Arjan van de Ven
2008-07-19 1:22 ` Daniel Walker
2008-07-19 3:44 ` Arjan van de Ven
2008-07-19 4:11 ` Daniel Walker
2008-07-19 4:58 ` Arjan van de Ven
2008-07-19 5:20 ` Arjan van de Ven
2008-07-19 15:24 ` Daniel Walker
2008-07-19 15:35 ` Arjan van de Ven
2008-07-19 16:08 ` Daniel Walker
2008-07-19 16:14 ` Arjan van de Ven
2008-07-19 4:28 ` Daniel Walker
2008-07-19 7:53 ` Rene Herman
2008-07-19 8:10 ` Rene Herman
2008-07-19 15:44 ` Arjan van de Ven
2008-07-20 7:23 ` Rene Herman [this message]
2008-07-20 11:10 ` Arjan van de Ven
2008-07-20 14:20 ` Rene Herman
2008-07-20 15:35 ` Arjan van de Ven
2008-07-18 22:16 ` [patch 2/3] fastboot: turn the USB hostcontroller initcalls into async initcalls Arjan van de Ven
2008-07-18 22:17 ` [patch 3/3] fastboot: convert a few non-critical ACPI drivers to " Arjan van de Ven
2008-07-19 4:51 ` [patch 0/3] fastboot patches series 1 Simon Arlott
2008-07-19 5:16 ` Arjan van de Ven
2008-07-19 5:47 ` Simon Arlott
2008-07-19 10:22 ` Andi Kleen
2008-07-20 8:31 ` Ingo Molnar
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=4882E7F3.8060605@keyaccess.nl \
--to=rene.herman@keyaccess.nl \
--cc=arjan@infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=pavel@ucw.cz \
--cc=rjw@sisk.pl \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.