All of lore.kernel.org
 help / color / mirror / Atom feed
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.

  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.