Re: [patch 1/3] fastboot: Create a "asynchronous" initlevel

From: Rene Herman
Date: Sun Jul 20 2008 - 03:21:42 EST


On 19-07-08 17:44, Arjan van de Ven wrote:

On Sat, 19 Jul 2008 10:10:09 +0200
Rene Herman <rene.herman@xxxxxxxxxxxx> 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/