Messages in this thread | | | Date | Sun, 20 Jul 2008 16:20:06 +0200 | From | Rene Herman <> | Subject | Re: [patch 1/3] fastboot: Create a "asynchronous" initlevel |
| |
On 20-07-08 13:10, Arjan van de Ven wrote:
> On Sun, 20 Jul 2008 09:23:31 +0200 > Rene Herman <rene.herman@keyaccess.nl> wrote: > >> 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. > > true for now (but see below) > >> 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 was hoping to not need this ordering.
May have found an issue with 3/3 for this same reason. You make the ACPI button driver async but acpi_wakeup_device_init() is a late_initcall and comments that it interacts with the button driver.
The button driver could be a module so a complete reversal of ordering between acpi_wakeup_device() and acpi_button_init() might in itself not be a problem (undeterministic order even with the button driver builtin might be undesireable I guess) but ...
Correct me if I'm wrong but I believe your patch implies that we could be racing between acpi_wakeup_device() and acpi_button_init()? If yes, do bad things happen when we race checking dev->wakeup.state.enabled?
As far as I can see, the acpi_device_lock isn't serialising here so if we have just done the acpi_enable_gpe() in acpi_button_add() but haven't set the enabled flag yet we could do it again here it seems.
The ACPI button driver doesn't appear to have a specific maintainer and Len Brown was on vacation I believe but this would ideally like a comment from that side...
>> I trust it will completely and utterly destroy the point of this >> patch to flush level 6a before starting level 7? > > Thankfully it doesn't destroy it, the reason for this is that level 6 > itself tends to take long enough to get benefits. It's just that if we > can get both 6 and 7 it's nicer. But if we end up needing to sync, so > be it.
I worry...
> Note: syncing on a driver_probe_done() from level 7 is not going to be > pretty (think "multi-second extra boot time). > Part of me wants to only sync level 6a from the first > driver_probe_done() so that only people who already pay these extra > seconds suffer this one as well ;-)
Makes sense in this specific case. Generally, utility of late_initcall() does seem to be impacted by this. Unless you can be sure that every device you depend on is and always will be sync you might as well be device_initcall() yourself after all.
Yes, I did note the bit about the endpoint probing already being async for example for USB but now you can't even be sure that it _started_ meaning you also couldn't really devise some private synchronization mechanism either.
Rene.
| |