Okay, I'll leave the whole request_firmware{,_direct,_nowait}() thing 
alone.  The request_firmware_direct() will "solve" a non-problem (since 
all of our "firmware" files are _supposed to be_ always present.  (And 
the 60 second timeout for udev to confirm that a file doesn't exist 
seems like udev is just basically broken.)

   That aside, we still need to solve the real problem that we're 
experiencing in that the boot-time load of cxgb4 is timing out on SLE12 
because a maximum load timeout has been instituted in that distribution 
for driver modules and if there are two 10Gb/s-BT adapters present in a 
system which need both base firmware and BT PHY firmware, we exceed that 
timeout.  The timeout really should be per device (since there ~could~ 
be an arbitrary number of devices in a system) and there probably should 
be a way for the driver to notify the kernel timeout mechanism that 
forward progress is being made ...

Casey

On 06/25/14 10:31, Luis R. Rodriguez wrote:

On Wed, Jun 25, 2014 at 10:12:20AM -0700, Casey Leedom wrote:

quoted

On 06/24/14 18:50, Luis R. Rodriguez wrote:

quoted

On Tue, Jun 24, 2014 at 03:54:44PM -0700, Casey Leedom wrote:

quoted

[[ Hopefully this makes it through to the kernel.org lists -- I’m using the
    Mac OS/X Mailer and it’s not clear how to force it not to use HTML format.
    -- Casey ]]

    So does request_firmware_direct() only fail if the requested file is not
    present on the file system or does it fail in other cases as well?

Same as before they are the same exact call with the only difference
being udev is not used as an extra helper, so it saves the extra
delay caused by udev. That's all.

quoted

    If it’s the former, then the change to cxgb4 is fine.

    But if it’s the latter, then it’s definitely not okay.  While the driver
    _can_ continue running without the local on-disk Firmware Configuration
    File, that file can be used to significantly change the behavior and
    capabilities of the adapter and is user-customizable.  If a user makes
    changes to the local on-disk Firmware Configuration File and these are
    randomly silently ignored this will lead to highly annoying support issues.

This just avoids udev, the request goes directly to the filesystem. The
failure will happen when the file is not present just as before, the
only difference here is skipping udev, it doesn't suffer from the extra
60 second timeout. There's another possible failure, when
usermodehelper_read_trylock() fails but that is just as the code was before
so this change doesn't introduce that as a new false check. When that
triggers yout get a nasty WARN_ON() just as before.

   Huh, okay.  I guess I'm confused about the value of request_firmware()
and the User Device helper.  If request_firmware_direct() just goes to the
file system to grab the file and returns with ENOENT if it's not there,
then you could replace every usage of request_firmware() in the cxgb4
driver as far as I can see ...  Either the files are there and we'll use
them or they won't be and we'll have to cope with that.  Am I missing
something?

You're actually right specially given that udev firmware uploading will
hopefully eventually be removed eventually (it seems it was just one driver
that caused to consider waiting on the removal, some driver that required
looking for firmware on some custom path I think or used a custom loader), for
now however its best to keep things consistent otherwise we'd replace
everything already. The _direct() call then is best used for optional firmware
for now. Perhaps in the end will be that the non _direct() call will have an
explicit print to the ring buffer if the file was not found.

quoted

   And again, this definitely isn't going to solve the problem that started
this whole line of research:

I consider this research part of understanding and optimizing firmware
loading on cxgb4, in this case this would save 60 seconds for each
optional configuration file not present when loading, its not clear to
me how often devices don't have optional configs so its unclear to me the
exact savings in general, but if there's at least one user that should
speed things up.

quoted

we're still going to time out the load of
cxgb4 if there are multiple 10Gb/s BT adapters in a system and we need to
load each one with both base firmware and PHY firmware.

Again, the timeout is *within* firmware_request(), firmware_release() does
not tell the firmware loader the timeout is over. The timeout is for the
kernel doing the hunt for the file.

As I see it the next steps on the evaluation on firmware loading on cxgb4
would be to evaluate a clean strategy to split things up, and also would
appreciate feedback on the bus master thing. Perhaps best we continue that
discussoin on that thread?

   Luis