Message-Id: <200705091116.06995.duncan.sands@math.u-psud.fr>
Date:	Wed, 9 May 2007 11:16:06 +0200
From:	Duncan Sands <duncan.sands@...h.u-psud.fr>
To:	Cornelia Huck <cornelia.huck@...ibm.com>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Adrian Bunk <bunk@...sta.de>, Greg K-H <greg@...ah.com>,
	linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: Please revert 5adc55da4a7758021bcc374904b0f8b076508a11 (PCI_MULTITHREAD_PROBE)

Hi Cornelia,

> > the usbatm USB ADSL modem drivers have this functionality.  These drivers
> > need to load firmware before they become useful.  The natural place to do
> > this is in the probe() method, but because firmware loading can take quite
> > some time (10 seconds, or even an infinite amount of time if the firmware
> > is not available and the timeout has been turned off) and would block the
> > USB hub thread if done from probe(), it's done in a separate kernel thread.
> > Clients of usbatm, like the speedtch driver, register themselves with usbatm,
> > providing a "bind" and a "heavy_init" method.  "bind" is like probe(), while
> > "heavy_init" is like probe_async().  First bind is called, and if successful
> > and heavy_init has been defined, then heavy_init is run in its own thread.
> > If the device is unplugged, usbatm takes care of making sure that the heavy_init
> > thread has stopped before calling unbind and destroying device related structures.
> > 
> > A bunch of other USB drivers could do with similar functionality for the
> > same reason (slow probe), and there was some discussion about generalizing
> > this functionality to the USB layer but I didn't find time to do anything
> > about it yet.  See http://marc.info/?l=linux-usb-devel&m=116551653026075&w=2
> 
> Would a general split between probe() (check if we can handle the
> device, do very basic stuff) and setup() (get the device up and
> running) make sense? Drivers could stay with today's probe() function
> if they want to (and still be working). setup() could be, but need not
> be async. As an added benefit for huge systems, setup() might only be
> called if explicitly requested (like "only do this heavy lifting
> if/when we really want to use the device"). probe() could call bind()
> and setup() (doing the firmware load etc.) heavy_init(). (This may be
> orthogonal to the probe()/probe_async() idea.)

yes that would be fine.  Like with your setup() suggestion, it is possible
to specify whether heavy_init should be called or not.  This is useful if
the firmware has already been uploaded - the probe() can detect this and
signal that heavy_init is not needed.

The main difficulties encountered with usbatm were: (1) avoiding race
conditions on device disconnect; (2) providing a way to cancel heavy_init.
As mentioned in my original email, heavy_init could run forever depending
on how the system is configured.  In the current implementation a signal
is sent on disconnect(), to indicate that the game is up, then disconnect()
blocks until heavy_init terminates, if it hasn't already.  A better method
would be to allow registration of a cancel() routine, which disconnect
would call before blocking on heavy_init termination.  This could send a
signal, or complete a completion, or whatever.  Sadly, the generic firmware
loading infrastructure doesn't have a way of asynchronously cancelling a
firmware load, so right now cancellation only does something if the firmware
has already been loaded from disk, in which case it cancels the upload to the
modem (which is the most time consuming part).

Best wishes,

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

Powered by blists - more mailing lists