Re: udev breakages - was: Re: Need of an ".async_probe()" type of callback at driver's core - Was: Re: [PATCH] [media] drxk: change it to use request_firmware_nowait()
From: Mauro Carvalho Chehab <hidden>
Date: 2012-10-03 14:13:05
Also in:
lkml
Em 02-10-2012 19:23, Greg KH escreveu:
quoted hunk ↗ jump to hunk
On Tue, Oct 02, 2012 at 03:12:39PM -0700, Greg KH wrote:quoted
On Tue, Oct 02, 2012 at 09:33:03AM -0700, Linus Torvalds wrote:quoted
I don't know where the problem started in udev, but the report I saw was that udev175 was fine, and udev182 was broken, and would deadlock if module_init() did a request_firmware(). That kind of nested behavior is absolutely *required* to work, in order to not cause idiotic problems for the kernel for no good reason. What kind of insane udev maintainership do we have? And can we fix it? Greg, I think you need to step up here too. You were the one who let udev go. If the new maintainers are causing problems, they need to be fixed some way.I've talked about this with Kay in the past (Plumbers conference I think) and I thought he said it was all fixed in the latest version of udev so there shouldn't be any problems anymore with this. Mauro, what version of udev are you using that is still showing this issue? Kay, didn't you resolve this already? If not, what was the reason why?Hm, in digging through the udev tree, the only change I found was this one: commit 39177382a4f92a834b568d6ae5d750eb2a5a86f9 Author: Kay Sievers [off-list ref] Date: Thu Jul 19 12:32:24 2012 +0200 udev: firmware - do not cancel requests in the initrddiff --git a/src/udev/udev-builtin-firmware.c b/src/udev/udev-builtin-firmware.c index 56dc8fc..de93d7b 100644 --- a/src/udev/udev-builtin-firmware.c +++ b/src/udev/udev-builtin-firmware.c@@ -129,7 +129,13 @@ static int builtin_firmware(struct udev_device *dev, int argc, char *argv[], boo err = -errno; } while (err == -ENOENT); rc = EXIT_FAILURE; - set_loading(udev, loadpath, "-1"); + /* + * Do not cancel the request in the initrd, the real root might have + * the firmware file and the 'coldplug' run in the real root will find + * this pending request and fulfill or cancel it. + * */ + if (!in_initrd()) + set_loading(udev, loadpath, "-1"); goto exit; }which went into udev release 187 which I think corresponds to the place when people started having problems, right Mauro?
I'm using here udev-182.
If so, Mauro, is the solution just putting the firmware into the initrd?
I don't think that putting firmware on initrd is something that we want to for media drivers. None of the webcam drivers currently need a firmware; those are required on more complex devices (typically digital TV ones). While there are a number of PCI devices that require firmware, in practice, we're seeing more people using USB devices. IMO, it doesn't make any sense that a hot-pluggable USB device to require a firmware at initrd/initramfs, with is available only at boot time.
No wait, it looks like this change was trying to fix the problem where firmware files were not in the initrd, so it would stick around for the real root to show up so that they could be loaded. So this looks like it was fixing firmware loading problems for people?
I'll run some tests with this patch applied and see what happens.
Kay, am I just looking at the totally wrong place here, and this file in udev didn't have anything to do with the breakage? thanks, greg k-h