Re: [systemd-devel] [PATCH] Change CONFIG_FW_LOADER_USER_HELPER to default n and don't select it
On 7 Aug 2013 02:26, "Andy Lutomirski" wrote: > > On Tue, Aug 6, 2013 at 5:24 PM, Tom Gundersen wrote: > > > > On 6 Aug 2013 18:32, "Bryan Kadzban" wrote: > >> > >> On Tue, Aug 06, 2013 at 11:17:17AM +0200, Tom Gundersen wrote: > >> > On Tue, Aug 6, 2013 at 11:11 AM, Tom Gundersen wrote: > >> > > On Tue, Aug 6, 2013 at 10:20 AM, Maarten Lankhorst > >> > > wrote: > >> > >> Op 05-08-13 18:29, Andy Lutomirski schreef: > >> > >>> The systemd commit below can delay firmware loading by multiple > >> > >>> minutes if CONFIG_FW_LOADER_USER_HELPER=y. Unfortunately no one > >> > >>> noticed that the systemd-udev change would break new kernels as well > >> > >>> as old kernels. > >> > >>> > >> > >>> Since the kernel apparently can't count on reasonable userspace > >> > >>> support, turn this thing off by default. > >> > >>> > >> > >>> commit a3bd8447be4ea2ce230eb8ae0e815c04d85fa15a > >> > >>> Author: Tom Gundersen > >> > >>> Date: Mon Mar 18 15:12:18 2013 +0100 > >> > >>> > >> > >>> udev: make firmware loading optional and disable by default > >> > >>> > >> > >>> Distros that whish to support old kernels should set > >> > >>> > >> > >>> --with-firmware-dirs="/usr/lib/firmware/updates:/usr/lib/firmware" > >> > >>> to retain the old behaviour. > >> > >>> > >> > >> methinks this patch should be reverted then, > >> > > > >> > > Well, all the code is still there, so it can be enabled if anyone > >> > > wants it. > >> > > > >> > >> or a stub should be added to udev to always fail firmware loading so > >> > >> timeouts don't occur. > >> > > > >> > > I think the only use (if any) of a userspace firmware loader would be > >> > > for anyone who wants a custom one (i.e., not udev), so we shouldn't > >> > > just fail the loading from udev unconditionally. > >> > > > >> > > How about we just improve the udev documentation a bit, similar to > >> > > Andy's kernel patch? > >> > > >> > Sorry, I should first have checked. We already document this in the > >> > README: > >> > > >> > >Userspace firmware loading is deprecated, will go away, and > >> > >sometimes causes problems: > >> > > CONFIG_FW_LOADER_USER_HELPER=n > >> > >> ...And this patch is making the kernel default to the correct behavior, > >> instead of the now-broken-by-udev behavior. > >> > >> I'm not sure I see the issue with it? :-) > > > > Oh yeah this patch is totally the right thing to do, I was just arguing that > > there is nothing to be done on the udev side. > > > >> (Add me to the list of people that think udev is broken too, fwiw. But > >> let's at least not leave *both* sides in a broken-by-default state.) > > > > Well I don't think it is too much to ask that the kernel and udev should be > > configured in a consistent way. Especially as thing still work even if you > > get it wrong, albeit with a delay. > > Except that the current defaults are inconsistent Which your patch fixes. I don't see the problem here... > and there is no > explanation anywhere in the logs when this is screwed up. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] Change CONFIG_FW_LOADER_USER_HELPER to default n and don't select it
On Tue, Aug 6, 2013 at 5:24 PM, Tom Gundersen wrote: > > On 6 Aug 2013 18:32, "Bryan Kadzban" wrote: >> >> On Tue, Aug 06, 2013 at 11:17:17AM +0200, Tom Gundersen wrote: >> > On Tue, Aug 6, 2013 at 11:11 AM, Tom Gundersen wrote: >> > > On Tue, Aug 6, 2013 at 10:20 AM, Maarten Lankhorst >> > > wrote: >> > >> Op 05-08-13 18:29, Andy Lutomirski schreef: >> > >>> The systemd commit below can delay firmware loading by multiple >> > >>> minutes if CONFIG_FW_LOADER_USER_HELPER=y. Unfortunately no one >> > >>> noticed that the systemd-udev change would break new kernels as well >> > >>> as old kernels. >> > >>> >> > >>> Since the kernel apparently can't count on reasonable userspace >> > >>> support, turn this thing off by default. >> > >>> >> > >>> commit a3bd8447be4ea2ce230eb8ae0e815c04d85fa15a >> > >>> Author: Tom Gundersen >> > >>> Date: Mon Mar 18 15:12:18 2013 +0100 >> > >>> >> > >>> udev: make firmware loading optional and disable by default >> > >>> >> > >>> Distros that whish to support old kernels should set >> > >>> >> > >>> --with-firmware-dirs="/usr/lib/firmware/updates:/usr/lib/firmware" >> > >>> to retain the old behaviour. >> > >>> >> > >> methinks this patch should be reverted then, >> > > >> > > Well, all the code is still there, so it can be enabled if anyone >> > > wants it. >> > > >> > >> or a stub should be added to udev to always fail firmware loading so >> > >> timeouts don't occur. >> > > >> > > I think the only use (if any) of a userspace firmware loader would be >> > > for anyone who wants a custom one (i.e., not udev), so we shouldn't >> > > just fail the loading from udev unconditionally. >> > > >> > > How about we just improve the udev documentation a bit, similar to >> > > Andy's kernel patch? >> > >> > Sorry, I should first have checked. We already document this in the >> > README: >> > >> > >Userspace firmware loading is deprecated, will go away, and >> > >sometimes causes problems: >> > > CONFIG_FW_LOADER_USER_HELPER=n >> >> ...And this patch is making the kernel default to the correct behavior, >> instead of the now-broken-by-udev behavior. >> >> I'm not sure I see the issue with it? :-) > > Oh yeah this patch is totally the right thing to do, I was just arguing that > there is nothing to be done on the udev side. > >> (Add me to the list of people that think udev is broken too, fwiw. But >> let's at least not leave *both* sides in a broken-by-default state.) > > Well I don't think it is too much to ask that the kernel and udev should be > configured in a consistent way. Especially as thing still work even if you > get it wrong, albeit with a delay. Except that the current defaults are inconsistent and there is no explanation anywhere in the logs when this is screwed up. --Andy ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] Change CONFIG_FW_LOADER_USER_HELPER to default n and don't select it
On 6 Aug 2013 18:32, "Bryan Kadzban" wrote: > > On Tue, Aug 06, 2013 at 11:17:17AM +0200, Tom Gundersen wrote: > > On Tue, Aug 6, 2013 at 11:11 AM, Tom Gundersen wrote: > > > On Tue, Aug 6, 2013 at 10:20 AM, Maarten Lankhorst > > > wrote: > > >> Op 05-08-13 18:29, Andy Lutomirski schreef: > > >>> The systemd commit below can delay firmware loading by multiple > > >>> minutes if CONFIG_FW_LOADER_USER_HELPER=y. Unfortunately no one > > >>> noticed that the systemd-udev change would break new kernels as well > > >>> as old kernels. > > >>> > > >>> Since the kernel apparently can't count on reasonable userspace > > >>> support, turn this thing off by default. > > >>> > > >>> commit a3bd8447be4ea2ce230eb8ae0e815c04d85fa15a > > >>> Author: Tom Gundersen > > >>> Date: Mon Mar 18 15:12:18 2013 +0100 > > >>> > > >>> udev: make firmware loading optional and disable by default > > >>> > > >>> Distros that whish to support old kernels should set > > >>> --with-firmware-dirs="/usr/lib/firmware/updates:/usr/lib/firmware" > > >>> to retain the old behaviour. > > >>> > > >> methinks this patch should be reverted then, > > > > > > Well, all the code is still there, so it can be enabled if anyone wants it. > > > > > >> or a stub should be added to udev to always fail firmware loading so timeouts don't occur. > > > > > > I think the only use (if any) of a userspace firmware loader would be > > > for anyone who wants a custom one (i.e., not udev), so we shouldn't > > > just fail the loading from udev unconditionally. > > > > > > How about we just improve the udev documentation a bit, similar to > > > Andy's kernel patch? > > > > Sorry, I should first have checked. We already document this in the README: > > > > >Userspace firmware loading is deprecated, will go away, and > > >sometimes causes problems: > > > CONFIG_FW_LOADER_USER_HELPER=n > > ...And this patch is making the kernel default to the correct behavior, > instead of the now-broken-by-udev behavior. > > I'm not sure I see the issue with it? :-) Oh yeah this patch is totally the right thing to do, I was just arguing that there is nothing to be done on the udev side. > (Add me to the list of people that think udev is broken too, fwiw. But > let's at least not leave *both* sides in a broken-by-default state.) Well I don't think it is too much to ask that the kernel and udev should be configured in a consistent way. Especially as thing still work even if you get it wrong, albeit with a delay. Cheers, Tom ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] Change CONFIG_FW_LOADER_USER_HELPER to default n and don't select it
On Tue, Aug 06, 2013 at 11:17:17AM +0200, Tom Gundersen wrote: > On Tue, Aug 6, 2013 at 11:11 AM, Tom Gundersen wrote: > > On Tue, Aug 6, 2013 at 10:20 AM, Maarten Lankhorst > > wrote: > >> Op 05-08-13 18:29, Andy Lutomirski schreef: > >>> The systemd commit below can delay firmware loading by multiple > >>> minutes if CONFIG_FW_LOADER_USER_HELPER=y. Unfortunately no one > >>> noticed that the systemd-udev change would break new kernels as well > >>> as old kernels. > >>> > >>> Since the kernel apparently can't count on reasonable userspace > >>> support, turn this thing off by default. > >>> > >>> commit a3bd8447be4ea2ce230eb8ae0e815c04d85fa15a > >>> Author: Tom Gundersen > >>> Date: Mon Mar 18 15:12:18 2013 +0100 > >>> > >>> udev: make firmware loading optional and disable by default > >>> > >>> Distros that whish to support old kernels should set > >>> --with-firmware-dirs="/usr/lib/firmware/updates:/usr/lib/firmware" > >>> to retain the old behaviour. > >>> > >> methinks this patch should be reverted then, > > > > Well, all the code is still there, so it can be enabled if anyone wants it. > > > >> or a stub should be added to udev to always fail firmware loading so > >> timeouts don't occur. > > > > I think the only use (if any) of a userspace firmware loader would be > > for anyone who wants a custom one (i.e., not udev), so we shouldn't > > just fail the loading from udev unconditionally. > > > > How about we just improve the udev documentation a bit, similar to > > Andy's kernel patch? > > Sorry, I should first have checked. We already document this in the README: > > >Userspace firmware loading is deprecated, will go away, and > >sometimes causes problems: > > CONFIG_FW_LOADER_USER_HELPER=n ...And this patch is making the kernel default to the correct behavior, instead of the now-broken-by-udev behavior. I'm not sure I see the issue with it? :-) (Add me to the list of people that think udev is broken too, fwiw. But let's at least not leave *both* sides in a broken-by-default state.) ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] Change CONFIG_FW_LOADER_USER_HELPER to default n and don't select it
On Tue, Aug 6, 2013 at 2:17 AM, Tom Gundersen wrote: > On Tue, Aug 6, 2013 at 11:11 AM, Tom Gundersen wrote: >> On Tue, Aug 6, 2013 at 10:20 AM, Maarten Lankhorst >> wrote: >>> Op 05-08-13 18:29, Andy Lutomirski schreef: The systemd commit below can delay firmware loading by multiple minutes if CONFIG_FW_LOADER_USER_HELPER=y. Unfortunately no one noticed that the systemd-udev change would break new kernels as well as old kernels. Since the kernel apparently can't count on reasonable userspace support, turn this thing off by default. commit a3bd8447be4ea2ce230eb8ae0e815c04d85fa15a Author: Tom Gundersen Date: Mon Mar 18 15:12:18 2013 +0100 udev: make firmware loading optional and disable by default Distros that whish to support old kernels should set --with-firmware-dirs="/usr/lib/firmware/updates:/usr/lib/firmware" to retain the old behaviour. >>> methinks this patch should be reverted then, >> >> Well, all the code is still there, so it can be enabled if anyone wants it. >> >>> or a stub should be added to udev to always fail firmware loading so >>> timeouts don't occur. >> >> I think the only use (if any) of a userspace firmware loader would be >> for anyone who wants a custom one (i.e., not udev), so we shouldn't >> just fail the loading from udev unconditionally. >> >> How about we just improve the udev documentation a bit, similar to >> Andy's kernel patch? > > Sorry, I should first have checked. We already document this in the README: > >>Userspace firmware loading is deprecated, will go away, and >>sometimes causes problems: >> CONFIG_FW_LOADER_USER_HELPER=n TBH, the udev README is the last thing I'm going to check to figure out why I don't have wifi for a couple minutes after boot. Also, the message is missing the point. It's not that it's deprecated and sometimes causes problems -- it's that udev *changed behavior* and breaks your system if you have CONFIG_FW_LOADER_USER_HELPER=y. If udev logged something (for a couple of years), that would be a different story. --Andy ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] Change CONFIG_FW_LOADER_USER_HELPER to default n and don't select it
On Tue, Aug 6, 2013 at 11:11 AM, Tom Gundersen wrote: > On Tue, Aug 6, 2013 at 10:20 AM, Maarten Lankhorst > wrote: >> Op 05-08-13 18:29, Andy Lutomirski schreef: >>> The systemd commit below can delay firmware loading by multiple >>> minutes if CONFIG_FW_LOADER_USER_HELPER=y. Unfortunately no one >>> noticed that the systemd-udev change would break new kernels as well >>> as old kernels. >>> >>> Since the kernel apparently can't count on reasonable userspace >>> support, turn this thing off by default. >>> >>> commit a3bd8447be4ea2ce230eb8ae0e815c04d85fa15a >>> Author: Tom Gundersen >>> Date: Mon Mar 18 15:12:18 2013 +0100 >>> >>> udev: make firmware loading optional and disable by default >>> >>> Distros that whish to support old kernels should set >>> --with-firmware-dirs="/usr/lib/firmware/updates:/usr/lib/firmware" >>> to retain the old behaviour. >>> >> methinks this patch should be reverted then, > > Well, all the code is still there, so it can be enabled if anyone wants it. > >> or a stub should be added to udev to always fail firmware loading so >> timeouts don't occur. > > I think the only use (if any) of a userspace firmware loader would be > for anyone who wants a custom one (i.e., not udev), so we shouldn't > just fail the loading from udev unconditionally. > > How about we just improve the udev documentation a bit, similar to > Andy's kernel patch? Sorry, I should first have checked. We already document this in the README: >Userspace firmware loading is deprecated, will go away, and >sometimes causes problems: > CONFIG_FW_LOADER_USER_HELPER=n -t ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] Change CONFIG_FW_LOADER_USER_HELPER to default n and don't select it
On Tue, Aug 6, 2013 at 10:20 AM, Maarten Lankhorst wrote: > Op 05-08-13 18:29, Andy Lutomirski schreef: >> The systemd commit below can delay firmware loading by multiple >> minutes if CONFIG_FW_LOADER_USER_HELPER=y. Unfortunately no one >> noticed that the systemd-udev change would break new kernels as well >> as old kernels. >> >> Since the kernel apparently can't count on reasonable userspace >> support, turn this thing off by default. >> >> commit a3bd8447be4ea2ce230eb8ae0e815c04d85fa15a >> Author: Tom Gundersen >> Date: Mon Mar 18 15:12:18 2013 +0100 >> >> udev: make firmware loading optional and disable by default >> >> Distros that whish to support old kernels should set >> --with-firmware-dirs="/usr/lib/firmware/updates:/usr/lib/firmware" >> to retain the old behaviour. >> > methinks this patch should be reverted then, Well, all the code is still there, so it can be enabled if anyone wants it. > or a stub should be added to udev to always fail firmware loading so timeouts > don't occur. I think the only use (if any) of a userspace firmware loader would be for anyone who wants a custom one (i.e., not udev), so we shouldn't just fail the loading from udev unconditionally. How about we just improve the udev documentation a bit, similar to Andy's kernel patch? Cheers, Tom ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] Change CONFIG_FW_LOADER_USER_HELPER to default n and don't select it
Op 05-08-13 18:29, Andy Lutomirski schreef: > The systemd commit below can delay firmware loading by multiple > minutes if CONFIG_FW_LOADER_USER_HELPER=y. Unfortunately no one > noticed that the systemd-udev change would break new kernels as well > as old kernels. > > Since the kernel apparently can't count on reasonable userspace > support, turn this thing off by default. > > commit a3bd8447be4ea2ce230eb8ae0e815c04d85fa15a > Author: Tom Gundersen > Date: Mon Mar 18 15:12:18 2013 +0100 > > udev: make firmware loading optional and disable by default > > Distros that whish to support old kernels should set > --with-firmware-dirs="/usr/lib/firmware/updates:/usr/lib/firmware" > to retain the old behaviour. > methinks this patch should be reverted then, or a stub should be added to udev to always fail firmware loading so timeouts don't occur. ~Maarten ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] Change CONFIG_FW_LOADER_USER_HELPER to default n and don't select it
The systemd commit below can delay firmware loading by multiple minutes if CONFIG_FW_LOADER_USER_HELPER=y. Unfortunately no one noticed that the systemd-udev change would break new kernels as well as old kernels. Since the kernel apparently can't count on reasonable userspace support, turn this thing off by default. commit a3bd8447be4ea2ce230eb8ae0e815c04d85fa15a Author: Tom Gundersen Date: Mon Mar 18 15:12:18 2013 +0100 udev: make firmware loading optional and disable by default Distros that whish to support old kernels should set --with-firmware-dirs="/usr/lib/firmware/updates:/usr/lib/firmware" to retain the old behaviour. --- drivers/base/Kconfig | 15 +++ drivers/firmware/Kconfig | 1 - 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig index 5daa259..de3903e 100644 --- a/drivers/base/Kconfig +++ b/drivers/base/Kconfig @@ -146,13 +146,20 @@ config EXTRA_FIRMWARE_DIR config FW_LOADER_USER_HELPER bool "Fallback user-helper invocation for firmware loading" depends on FW_LOADER - default y + default n help This option enables / disables the invocation of user-helper (e.g. udev) for loading firmware files as a fallback after the - direct file loading in kernel fails. The user-mode helper is - no longer required unless you have a special firmware file that - resides in a non-standard path. + direct file loading in kernel fails. + + Since March 2013, a default udev build does not understand + firmware loading requests. These udev versions will not + even indicate failure; instead they cause long timeouts. + This can dramatically slow down the boot process. + + Say Y only if you have special firmware-loading requirements + and if you have a non-standard helper that will handle these + requests. config DEBUG_DRIVER bool "Driver Core verbose debug messages" diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig index 07478728..9387630 100644 --- a/drivers/firmware/Kconfig +++ b/drivers/firmware/Kconfig @@ -64,7 +64,6 @@ config DELL_RBU tristate "BIOS update support for DELL systems via sysfs" depends on X86 select FW_LOADER - select FW_LOADER_USER_HELPER help Say m if you want to have the option of updating the BIOS for your DELL system. Note you need a Dell OpenManage or Dell Update package (DUP) -- 1.8.3.1 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
