Created attachment 588547 [details] syslog Description of problem: DVB USB device firmware downloading takes 30 seconds. Version-Release number of selected component (if applicable): How reproducible: Steps to Reproduce: 1. 2. 3. Actual results: It takes over 30 seconds until device appears. Expected results: Device should be ready during 2 seconds when plugged. Additional info: errors seen in log: udevd[423]: worker [1581] timeout, kill it udevd[423]: worker [1581] terminated by signal 9 (Killed) Downgrading to Fedora 16 udev packages (udev libudev libgudev1) resolved the problem.
This is very likely a kernel driver issue. Drivers must not load firmware in the module_init() path, or device probe()/bind() path. This creates a deadlock in the event handling. We used to silently try to work around that, but recently started to log this error explicitely. The firmware should in general be requested asynchronously, or at the first open() of the device. Details are here: http://thread.gmane.org/gmane.linux.network/217729
(In reply to comment #1) > This is very likely a kernel driver issue. > > Drivers must not load firmware in the module_init() path, or device > probe()/bind() path. This creates a deadlock in the event handling. > > We used to silently try to work around that, but recently started > to log this error explicitely. > > The firmware should in general be requested asynchronously, or at the first > open() of the device. > > Details are here: > http://thread.gmane.org/gmane.linux.network/217729 Key, There are _several_ media drivers that requires loading the firmware during device probe, as, without a firmware, the driver is not able to detect what ancillary drivers are also required, as, on those drivers, the "main" driver is generally just an USB or PCI bridge for stream, and an I2C bridge for ancillary drivers. The real device control happens at those I2C ancillary drivers, that provide functions for tuning and demodulating the media streams. Without the firmware, it is not possible to know what else is needed.
(In reply to comment #2) > There are _several_ media drivers that requires loading the firmware during > device probe, as, without a firmware, the driver is not able to detect what > ancillary drivers are also required, as, on those drivers, the "main" driver > is generally just an USB or PCI bridge for stream, and an I2C bridge for > ancillary drivers. The real device control happens at those I2C ancillary > drivers, that provide functions for tuning and demodulating the media > streams. Without the firmware, it is not possible to know what else is > needed. That should still work, if the firmware loading request is done asynchronous. What we do not support anymore, and what might entirely stop to work in the future is blocking calls to request_firmware() from module_init() or probe(). It's pretty obvious I think, that the init_module() syscall is not allowed to call-out to a userspace transaction and block until that transaction is fulfilled, or it has run into the 60 second default timeout. That behavior is really broken and needs to be fixed.
In case it sounded wrong, comment#1 should really read: "Drivers must not *synchronously* load firmware in the module_init() path, or device probe()/bind() path." What we need to avoid is the the "modprobe" process get blocked in the kernel, waiting for a callout to userspace, which might not be fulfilled. That creates all sorts of problems, hence udev compalins about that now.
(In reply to comment #4) > In case it sounded wrong, comment#1 should really read: > "Drivers must not *synchronously* load firmware in the module_init() path, or > device probe()/bind() path." > > What we need to avoid is the the "modprobe" process get blocked in the > kernel, > waiting for a callout to userspace, which might not be fulfilled. That > creates > all sorts of problems, hence udev compalins about that now. While it is ok to require that module_init() should not require a firmware, device probe()/bind(), on some media devices, require firmware to happen, as otherwise the i2c bus will not be available, and the media drivers for the I2C drivers can't be probed. A hack to run the probe asynchronously would mean that the probe would return 0 _even_ if the device driver cannot be completely loaded/binded (as the I2C drivers are required for the device to be available will not be loaded without a firmware at the bridge driver).
As Mauro said DVB devices are generally required to download firmware at the very first. This is not only for I2C adapter but remote controller too. I delayed DVB USB framework firmware downloading and bug disappeared. I did that in USB .probe() delaying real .probe() using workqueue and returning success for the ongoing .probe() in every case. We have now quite many drivers to fix, not only bus interface drivers but also some demod and tuner drivers that loads firmware on attach(). I also strongly suspect that same issue has been behind all the firmware downloading problems we have had during the last years. Especially resume from the suspend has been broken: http://www.mail-archive.com/linux-media@vger.kernel.org/msg09974.html I wonder why these udev requirements and changes are never meet relevant persons from the linux-media.
Just signing up to Cc. This change is probably part of the general boot-up optimization from the systemd related changes.
(In reply to comment #5) > (In reply to comment #4) > > In case it sounded wrong, comment#1 should really read: > > "Drivers must not *synchronously* load firmware in the module_init() path, or > > device probe()/bind() path." > > > > What we need to avoid is the the "modprobe" process get blocked in the > > kernel, > > waiting for a callout to userspace, which might not be fulfilled. That > > creates > > all sorts of problems, hence udev compalins about that now. > > While it is ok to require that module_init() should not require a firmware, > device probe()/bind(), on some media devices To make it clearer, this is what Kernel Documentation/driver-model/driver.txt says about the .probe callback: int (*probe) (struct device * dev); The probe() entry is called in task context, with the bus's rwsem locked and the driver partially bound to the device. Drivers commonly use container_of() to convert "dev" to a bus-specific type, both in probe() and other routines. That type often provides device resource data, such as pci_dev.resource[] or platform_device.resources, which is used in addition to dev->platform_data to initialize the driver. This callback holds the driver-specific logic to bind the driver to a given device. That includes verifying that the device is present, that it's a version the driver can handle, that driver data structures can be allocated and initialized, and that any hardware can be initialized. Drivers often store a pointer to their state with dev_set_drvdata(). When the driver has successfully bound itself to that device, then probe() returns zero and the driver model code will finish its part of binding the driver to that device. A driver's probe() may return a negative errno value to indicate that the driver did not bind to this device, in which case it should have released all resources it allocated. On DVB devices, it is not possible to bind, verify that the device is present and that it is a version that the driver can handle without initializing the board's I2C and probing the I2C devices on it. For that to happen, the bridge driver's firmware is required. So, the DVB drivers are doing the right thing. It is an udev bug to block loading a firmware while the driver's probe method is being called.
(In reply to comment #8) > So, the DVB drivers are doing the right thing. It is an udev bug to block > loading a firmware while the driver's probe method is being called. It's a bug in the driver. Requesting firmware in a blocking manner in the module_init() path is a serious bug in the driver. Modprobe must return, not blindly wait for a userspace transaction to be fulfilled, which might never happen. Please fix the driver, or close the bug if you refuse to do it. Udev behaves correctly and will not be changed. The problem this broken driver behaviour causes, is bigger than udev, and udev is just the messenger of the issue, not the cause.
(In reply to comment #9) > (In reply to comment #8) > > So, the DVB drivers are doing the right thing. It is an udev bug to block > > loading a firmware while the driver's probe method is being called. > > It's a bug in the driver. Requesting firmware in a blocking manner in the > module_init() path is a serious bug in the driver. Modprobe must return, > not blindly wait for a userspace transaction to be fulfilled, which might > never happen. Firmware load at DVB is not at driver's module_init(). It is at driver's .probe(), and it is _required_ to fulfill the Linux driver model, as specified at the Kernel's documentation, as shown above. It is _not_possible_ to probe the device without loading the bridge's firmware, as probing the device requires access to the device's internal buses, in order to inspect and load the required drivers. Typically, a DVB device requires several sub-drivers. The simplest driver requires at least 3/4 drivers: - The bridge driver (the one that is modprobed); - The frontend driver (with the remaining logic to get a digital channel); - The tuner driver (with the code to set the PLL to a given frequency - sometimes part of the frontend); - The demod driver (with the code to decode a digital channel). It is not uncommon to also need drivers for IR, for the analog part and some common drivers with a code shared with other drivers. So, in any case, probing a DVB device will make the driver to go to disk, in order to load the required modules and firmwares. So, even from booting speedup PoV, not loading a firmware won't make a significant speedup, as firmware is just one (typically small) part of the data that is needed to be loaded during a DVB driver modprobe. If you want to speed up the boot time, just modprobe those drivers in background, but don't assume that firmware can be loaded later, as it can't. > Please fix the driver, or close the bug if you refuse to do it. There are no bugs at the 64 media drivers that require firmware at device's probe time. They're doing the right thing. The bug is at udev (and/or modprobe/systemd) level by forbidding a driver to properly implement the .probe() callback. > Udev behaves correctly and will not be changed. The problem this broken > driver behaviour causes, is bigger than udev, and udev is just the messenger > of the issue, not the cause. It is stupid to refuse to load a firmware during probe(): even assuming that it would be possible to do it, the firmware size is generally small part (typically 16Kb-64Kb) of what is needed to be load from the disk, when comparing with the size of the driver and media core modules that are required for a DVB device to work.
(In reply to comment #10) > Firmware load at DVB is not at driver's module_init(). It is at driver's > .probe(), Which is usually the same context, unless you delegate that work. > and it is _required_ to fulfill the Linux driver model, as specified at the > Kernel's documentation, as shown above. "fulfill the model"? - that does not make much sense in that context. The current driver behaviour is still broken. > It is _not_possible_ to probe the device without loading the bridge's > firmware, as probing the device requires access to the device's internal > buses, in order to inspect and load the required drivers. Typically, a DVB > device requires several sub-drivers. The simplest driver requires at least > 3/4 drivers: > - The bridge driver (the one that is modprobed); > - The frontend driver (with the remaining logic to get a digital > channel); > - The tuner driver (with the code to set the PLL to a given frequency - > sometimes part of the frontend); > - The demod driver (with the code to decode a digital channel). > > It is not uncommon to also need drivers for IR, for the analog part and some > common drivers with a code shared with other drivers. All that doesn't matter; do whatever you need to do, but load the firmware async, and continue to do the rest of the job when the firmware arrives, and do not block modprobe. > So, in any case, probing a DVB device will make the driver to go to disk, in > order to load the required modules and firmwares. So, even from booting > speedup PoV, not loading a firmware won't make a significant speedup, as > firmware is just one (typically small) part of the data that is needed to be > loaded during a DVB driver modprobe. That's all entirely irrelevant. > If you want to speed up the boot time, just modprobe those drivers in > background, but don't assume that firmware can be loaded later, as it can't. Nobody talks about boot times here. We just talk about a buggy driver. > > Please fix the driver, or close the bug if you refuse to do it. > > There are no bugs at the 64 media drivers that require firmware at device's > probe time. They're doing the right thing. The bug is at udev (and/or > modprobe/systemd) level by forbidding a driver to properly implement the > .probe() callback. Nope. Don't block in the firmware request, there is an async API for that, and you need to use that, or run your own workqueue in whatever way. > It is stupid to refuse to load a firmware during probe() The only thing stupid here is to block in a syscall with a call from inside the kernel out to userspace, and block until that is finished. You can argue as much as you want here, the problem will not go away that way.
Mauro, please stop assigning YOUR bugs to me. I can not fix YOUR buggy drivers!
Created attachment 621017 [details] Possible fix by doing direct filesystem firmware loading This is not in any kind of final form, and may be horribly buggy. It also doesn't participate in the nice firmware caching logic. So consider this an RFC and a call for testing
Created attachment 621061 [details] Improved version of previous patch This patch should work even if you haven't enabled user-space FW loading support (CONFIG_FW_LOADER), and fixes up some VFS complaints that Al had about the first version of the patch. It still doesn't participate in firmware caching etc, and we should possibly consider throwing out the udev case entirely, but this should be sufficient for testing.
(In reply to comment #12) > I can not fix YOUR buggy drivers! A billion "thank you", Kay, and another one to Cormier. Move your titanic on.
I just tested latest Kernel master and it works. Last commit: commit 612a9aab56a93533e76e3ad91642db7033e03b69 Merge: 3a49431 268d283 Author: Linus Torvalds <torvalds> Date: Wed Oct 3 23:29:23 2012 -0700 Merge branch 'drm-next' of git://people.freedesktop.org/~airlied/linux Linux localhost.localdomain 3.6.0+ #1 SMP Thu Oct 4 11:56:13 EEST 2012 x86_64 x86_64 x86_64 GNU/Linux ** I also tested Fedora 17 latest Kernel and it was still broken. Linux localhost.localdomain 3.5.4-2.fc17.x86_64 #1 SMP Wed Sep 26 21:58:50 UTC 2012 x86_64 x86_64 x86_64 GNU/Linux
(In reply to comment #16) > I just tested latest Kernel master and it works. > Last commit: > commit 612a9aab56a93533e76e3ad91642db7033e03b69 > Merge: 3a49431 268d283 > Author: Linus Torvalds <torvalds> > Date: Wed Oct 3 23:29:23 2012 -0700 > > Merge branch 'drm-next' of git://people.freedesktop.org/~airlied/linux > > Linux localhost.localdomain 3.6.0+ #1 SMP Thu Oct 4 11:56:13 EEST 2012 > x86_64 x86_64 x86_64 GNU/Linux > > ** That contains the revised version of the patch Linus attached here (commit abb139e75c2cdbb95). Thank you very much for testing. > I also tested Fedora 17 latest Kernel and it was still broken. That's to be expected at this point.
Still expected to be broken with 3.6.3-1.fc17.x86_64?
(In reply to comment #18) > Still expected to be broken with 3.6.3-1.fc17.x86_64? Yes. Patch is in 3.7-rc1 and hasn't been brought back to stable yet.
F17 will be rebased to the 3.7 kernel within the next couple of weeks. A backport of the patches never made it to the 3.6 stable series. I'll mark this bug as POST for now.
kernel-3.7.3-101.fc17 has been submitted as an update for Fedora 17. https://admin.fedoraproject.org/updates/FEDORA-2013-1025/kernel-3.7.3-101.fc17
kernel-3.7.3-101.fc17 has been pushed to the Fedora 17 stable repository. If problems still persist, please make note of it in this bug report.