Message-ID: <48834996.9060804@keyaccess.nl>
Date: Sun, 20 Jul 2008 16:20:06 +0200
From: Rene Herman <rene.herman@keyaccess.nl>
User-Agent: Thunderbird 2.0.0.14 (X11/20080421)
MIME-Version: 1.0
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>,
       Paul Diefenbaugh <paul.s.diefenbaugh@intel.com>,
       Andy Grover <andrew.grover@intel.com>, Len Brown <lenb@kernel.org>
Subject: Re: [patch 1/3] fastboot: Create a "asynchronous" initlevel
References: <20080718151524.5ef9e29b@infradead.org>	<20080718151608.46dc17c0@infradead.org>	<48819D92.8050904@keyaccess.nl>	<4881A161.7050902@keyaccess.nl>	<20080719084448.7859afd0@infradead.org>	<4882E7F3.8060605@keyaccess.nl> <20080720041039.275c57f9@infradead.org>
In-Reply-To: <20080720041039.275c57f9@infradead.org>
Content-Type: text/plain; charset=ISO-8859-15; format=flowed
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3003
Lines: 73

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.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/