public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Luis R. Rodriguez" <mcgrof@suse.com>
To: Casey Leedom <leedom@chelsio.com>
Cc: "Luis R. Rodriguez" <mcgrof@do-not-panic.com>,
	hariprasad@chelsio.com, poswald@suse.com, santosh@chelsio.com,
	jcheung@suse.com, dchang@suse.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [RFT 0/3] cxgb4: use request_firmware_nowait()
Date: Tue, 24 Jun 2014 02:29:41 +0200	[thread overview]
Message-ID: <20140624002941.GD27687@wotan.suse.de> (raw)
In-Reply-To: <53A87AC8.7010305@chelsio.com>

On Mon, Jun 23, 2014 at 12:06:48PM -0700, Casey Leedom wrote:
>   I've looked through the patch and I might be wrong, but it appears that 
> all the uses of the asynchronous request_firmware_nowait() are followed 
> immediately by wait_for_completion() calls which essentially would be the 
> same as the previous code with an added layer of mechanism.  Am I missing 
> something?

No you're right and I frailed to mention my original goal really is to
see if we can instead move that to ndo_init().

>   We do have a problem with initialization of multiple adapters with 
> external PHYs since, for each adapter we can check to see if the main 
> adapter firmware needs updating, and then load the PHY firmware.  If the 
> main firmware needs updating on more than one adapter, the combined time to 
> update each adapter's main firmware plus load the PHY firmware can exceed 
> some Distribution's default limits for a driver module's load time (since 
> the kernel seems to be processing the PCI Probe of each device 
> sequentially).

I noticed that for configuration updates it is optional for these configuration
updates to exist, in which case then if udev is enabled the driver will wait
quite a long time unncessarily. To fix that it seems request_firmware_direct()
would be better.

>   It seems to me that it's unfortunate that the limit isn't on a per device 
> basis since a system could have an arbitrary number of devices managed by a 
> driver module. 

The timeout is for the amount of time it takes the kernel to get the firemware,
not for device initialization, so its unclear to me that the 60 timeout thing
is actually causing an issue here.

> Also, it might be useful if there was a way for the driver 
> module to "tell" the timeout mechanism that forward progress _is_ being 
> made so it doesn't blow away the driver module load.

Indeed if this is actually needed, but believe the issue here for the
huge delays might be instead the lack of not using request_firmware_direct()
and actual device initialization time, which I do not believe we penalize,
we should be penalizing only the amount of time it takes either the
kernel or udev to read the firmware from the filesystem.

>  And maybe, if I'm 
> right regarding the sequential nature of the introduction of devices to 
> driver modules, it might make sense for a driver module to be able to 
> "tell" the kernel that it has no per-device dependencies and multiple 
> devices may be probed simultaneously ...

What if just configuration updates use request_firmware_direct() and
for the actual firmware which is required a request_firmware_nowait()
with a proper wait_for_completion() on the ndo_init() ?

  Luis

  reply	other threads:[~2014-06-24  0:29 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-21  0:39 [RFT 0/3] cxgb4: use request_firmware_nowait() Luis R. Rodriguez
2014-06-21  0:39 ` [RFT 1/3] cxgb4: make ethtool set_flash " Luis R. Rodriguez
2014-06-21  0:39 ` [RFT 2/3] cxgb4: make configuration load " Luis R. Rodriguez
2014-06-21  0:39 ` [RFT 3/3] cxgb4: make device firmware " Luis R. Rodriguez
2014-06-23 19:06 ` [RFT 0/3] cxgb4: " Casey Leedom
2014-06-24  0:29   ` Luis R. Rodriguez [this message]
2014-06-24 15:55     ` Casey Leedom
2014-06-24 16:34       ` Casey Leedom
2014-06-24 23:39         ` Luis R. Rodriguez
2014-06-25  2:00           ` Luis R. Rodriguez
2014-06-25 21:51             ` Luis R. Rodriguez
2014-06-26  0:08               ` Luis R. Rodriguez

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=20140624002941.GD27687@wotan.suse.de \
    --to=mcgrof@suse.com \
    --cc=dchang@suse.com \
    --cc=hariprasad@chelsio.com \
    --cc=jcheung@suse.com \
    --cc=leedom@chelsio.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mcgrof@do-not-panic.com \
    --cc=netdev@vger.kernel.org \
    --cc=poswald@suse.com \
    --cc=santosh@chelsio.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox