* [PATCH 1/2] driver core: Fix wait_for_device_probe() & deferred_probe_timeout interaction
2022-06-03 11:28 [GIT PULL] Driver core changes for 5.19-rc1 Greg KH
@ 2022-06-03 11:31 ` Greg Kroah-Hartman
2022-06-03 11:31 ` [PATCH 2/2] driver core: Set default deferred_probe_timeout back to 0 Greg Kroah-Hartman
2022-06-03 18:54 ` [PATCH 1/2] driver core: Fix wait_for_device_probe() & deferred_probe_timeout interaction Rafael J. Wysocki
2022-06-03 19:07 ` [GIT PULL] Driver core changes for 5.19-rc1 pr-tracker-bot
` (2 subsequent siblings)
3 siblings, 2 replies; 10+ messages in thread
From: Greg Kroah-Hartman @ 2022-06-03 11:31 UTC (permalink / raw)
To: Linus Torvalds, Andrew Morton
Cc: linux-kernel, Stephen Rothwell, Saravana Kannan, John Stultz,
David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI,
Jakub Kicinski, Rafael J . Wysocki, Rob Herring,
Geert Uytterhoeven, Yoshihiro Shimoda, Robin Murphy,
Andy Shevchenko, Sudeep Holla, Andy Shevchenko, Naresh Kamboju,
Basil Eljuse, Ferry Toth, Arnd Bergmann, Anders Roxell, linux-pm,
Nathan Chancellor, Sebastian Andrzej Siewior, Geert Uytterhoeven,
Greg Kroah-Hartman
From: Saravana Kannan <saravanak@google.com>
Mounting NFS rootfs was timing out when deferred_probe_timeout was
non-zero [1]. This was because ip_auto_config() initcall times out
waiting for the network interfaces to show up when
deferred_probe_timeout was non-zero. While ip_auto_config() calls
wait_for_device_probe() to make sure any currently running deferred
probe work or asynchronous probe finishes, that wasn't sufficient to
account for devices being deferred until deferred_probe_timeout.
Commit 35a672363ab3 ("driver core: Ensure wait_for_device_probe() waits
until the deferred_probe_timeout fires") tried to fix that by making
sure wait_for_device_probe() waits for deferred_probe_timeout to expire
before returning.
However, if wait_for_device_probe() is called from the kernel_init()
context:
- Before deferred_probe_initcall() [2], it causes the boot process to
hang due to a deadlock.
- After deferred_probe_initcall() [3], it blocks kernel_init() from
continuing till deferred_probe_timeout expires and beats the point of
deferred_probe_timeout that's trying to wait for userspace to load
modules.
Neither of this is good. So revert the changes to
wait_for_device_probe().
[1] - https://lore.kernel.org/lkml/TYAPR01MB45443DF63B9EF29054F7C41FD8C60@TYAPR01MB4544.jpnprd01.prod.outlook.com/
[2] - https://lore.kernel.org/lkml/YowHNo4sBjr9ijZr@dev-arch.thelio-3990X/
[3] - https://lore.kernel.org/lkml/Yo3WvGnNk3LvLb7R@linutronix.de/
Fixes: 35a672363ab3 ("driver core: Ensure wait_for_device_probe() waits until the deferred_probe_timeout fires")
Cc: John Stultz <jstultz@google.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
Cc: Rob Herring <robh@kernel.org>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Sudeep Holla <sudeep.holla@arm.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Naresh Kamboju <naresh.kamboju@linaro.org>
Cc: Basil Eljuse <Basil.Eljuse@arm.com>
Cc: Ferry Toth <fntoth@gmail.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Anders Roxell <anders.roxell@linaro.org>
Cc: linux-pm@vger.kernel.org
Reported-by: Nathan Chancellor <nathan@kernel.org>
Reported-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Acked-by: John Stultz <jstultz@google.com>
Signed-off-by: Saravana Kannan <saravanak@google.com>
Link: https://lore.kernel.org/r/20220526034609.480766-2-saravanak@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/base/dd.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 2fc8507f59ee..91f63cd33b12 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -263,7 +263,6 @@ int driver_deferred_probe_timeout;
#endif
EXPORT_SYMBOL_GPL(driver_deferred_probe_timeout);
-static DECLARE_WAIT_QUEUE_HEAD(probe_timeout_waitqueue);
static int __init deferred_probe_timeout_setup(char *str)
{
@@ -318,7 +317,6 @@ static void deferred_probe_timeout_work_func(struct work_struct *work)
list_for_each_entry(p, &deferred_probe_pending_list, deferred_probe)
dev_info(p->device, "deferred probe pending\n");
mutex_unlock(&deferred_probe_mutex);
- wake_up_all(&probe_timeout_waitqueue);
}
static DECLARE_DELAYED_WORK(deferred_probe_timeout_work, deferred_probe_timeout_work_func);
@@ -736,9 +734,6 @@ int driver_probe_done(void)
*/
void wait_for_device_probe(void)
{
- /* wait for probe timeout */
- wait_event(probe_timeout_waitqueue, !driver_deferred_probe_timeout);
-
/* wait for the deferred probe workqueue to finish */
flush_work(&deferred_probe_work);
--
2.36.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 2/2] driver core: Set default deferred_probe_timeout back to 0.
2022-06-03 11:31 ` [PATCH 1/2] driver core: Fix wait_for_device_probe() & deferred_probe_timeout interaction Greg Kroah-Hartman
@ 2022-06-03 11:31 ` Greg Kroah-Hartman
2022-06-03 18:54 ` [PATCH 1/2] driver core: Fix wait_for_device_probe() & deferred_probe_timeout interaction Rafael J. Wysocki
1 sibling, 0 replies; 10+ messages in thread
From: Greg Kroah-Hartman @ 2022-06-03 11:31 UTC (permalink / raw)
To: Linus Torvalds, Andrew Morton
Cc: linux-kernel, Stephen Rothwell, Saravana Kannan, Mark Brown,
Rob Herring, Nathan Chancellor, Sebastian Andrzej Siewior,
Geert Uytterhoeven, Greg Kroah-Hartman
From: Saravana Kannan <saravanak@google.com>
Since we had to effectively reverted
commit 35a672363ab3 ("driver core: Ensure wait_for_device_probe() waits
until the deferred_probe_timeout fires") in an earlier patch, a non-zero
deferred_probe_timeout will break NFS rootfs mounting [1] again. So, set
the default back to zero until we can fix that.
[1] - https://lore.kernel.org/lkml/TYAPR01MB45443DF63B9EF29054F7C41FD8C60@TYAPR01MB4544.jpnprd01.prod.outlook.com/
Fixes: 2b28a1a84a0e ("driver core: Extend deferred probe timeout on driver registration")
Cc: Mark Brown <broonie@kernel.org>
Cc: Rob Herring <robh@kernel.org>
Reported-by: Nathan Chancellor <nathan@kernel.org>
Reported-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Saravana Kannan <saravanak@google.com>
Link: https://lore.kernel.org/r/20220526034609.480766-3-saravanak@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/base/dd.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 91f63cd33b12..251b5ba1b84a 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -256,12 +256,7 @@ static int deferred_devs_show(struct seq_file *s, void *data)
}
DEFINE_SHOW_ATTRIBUTE(deferred_devs);
-#ifdef CONFIG_MODULES
-int driver_deferred_probe_timeout = 10;
-#else
int driver_deferred_probe_timeout;
-#endif
-
EXPORT_SYMBOL_GPL(driver_deferred_probe_timeout);
static int __init deferred_probe_timeout_setup(char *str)
--
2.36.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 1/2] driver core: Fix wait_for_device_probe() & deferred_probe_timeout interaction
2022-06-03 11:31 ` [PATCH 1/2] driver core: Fix wait_for_device_probe() & deferred_probe_timeout interaction Greg Kroah-Hartman
2022-06-03 11:31 ` [PATCH 2/2] driver core: Set default deferred_probe_timeout back to 0 Greg Kroah-Hartman
@ 2022-06-03 18:54 ` Rafael J. Wysocki
1 sibling, 0 replies; 10+ messages in thread
From: Rafael J. Wysocki @ 2022-06-03 18:54 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Linus Torvalds, Andrew Morton, Linux Kernel Mailing List,
Stephen Rothwell, Saravana Kannan, John Stultz, David S. Miller,
Alexey Kuznetsov, Hideaki YOSHIFUJI, Jakub Kicinski,
Rafael J . Wysocki, Rob Herring, Geert Uytterhoeven,
Yoshihiro Shimoda, Robin Murphy, Andy Shevchenko, Sudeep Holla,
Andy Shevchenko, Naresh Kamboju, Basil Eljuse, Ferry Toth,
Arnd Bergmann, Anders Roxell, Linux PM, Nathan Chancellor,
Sebastian Andrzej Siewior, Geert Uytterhoeven
On Fri, Jun 3, 2022 at 1:32 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> From: Saravana Kannan <saravanak@google.com>
>
> Mounting NFS rootfs was timing out when deferred_probe_timeout was
> non-zero [1]. This was because ip_auto_config() initcall times out
> waiting for the network interfaces to show up when
> deferred_probe_timeout was non-zero. While ip_auto_config() calls
> wait_for_device_probe() to make sure any currently running deferred
> probe work or asynchronous probe finishes, that wasn't sufficient to
> account for devices being deferred until deferred_probe_timeout.
>
> Commit 35a672363ab3 ("driver core: Ensure wait_for_device_probe() waits
> until the deferred_probe_timeout fires") tried to fix that by making
> sure wait_for_device_probe() waits for deferred_probe_timeout to expire
> before returning.
>
> However, if wait_for_device_probe() is called from the kernel_init()
> context:
>
> - Before deferred_probe_initcall() [2], it causes the boot process to
> hang due to a deadlock.
>
> - After deferred_probe_initcall() [3], it blocks kernel_init() from
> continuing till deferred_probe_timeout expires and beats the point of
> deferred_probe_timeout that's trying to wait for userspace to load
> modules.
>
> Neither of this is good. So revert the changes to
> wait_for_device_probe().
>
> [1] - https://lore.kernel.org/lkml/TYAPR01MB45443DF63B9EF29054F7C41FD8C60@TYAPR01MB4544.jpnprd01.prod.outlook.com/
> [2] - https://lore.kernel.org/lkml/YowHNo4sBjr9ijZr@dev-arch.thelio-3990X/
> [3] - https://lore.kernel.org/lkml/Yo3WvGnNk3LvLb7R@linutronix.de/
>
> Fixes: 35a672363ab3 ("driver core: Ensure wait_for_device_probe() waits until the deferred_probe_timeout fires")
> Cc: John Stultz <jstultz@google.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
> Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
> Cc: Sudeep Holla <sudeep.holla@arm.com>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Naresh Kamboju <naresh.kamboju@linaro.org>
> Cc: Basil Eljuse <Basil.Eljuse@arm.com>
> Cc: Ferry Toth <fntoth@gmail.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Anders Roxell <anders.roxell@linaro.org>
> Cc: linux-pm@vger.kernel.org
> Reported-by: Nathan Chancellor <nathan@kernel.org>
> Reported-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Acked-by: John Stultz <jstultz@google.com>
> Signed-off-by: Saravana Kannan <saravanak@google.com>
> Link: https://lore.kernel.org/r/20220526034609.480766-2-saravanak@google.com
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Rafael J. Wysocki <rafael@kernel.org>
> ---
> drivers/base/dd.c | 5 -----
> 1 file changed, 5 deletions(-)
>
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index 2fc8507f59ee..91f63cd33b12 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -263,7 +263,6 @@ int driver_deferred_probe_timeout;
> #endif
>
> EXPORT_SYMBOL_GPL(driver_deferred_probe_timeout);
> -static DECLARE_WAIT_QUEUE_HEAD(probe_timeout_waitqueue);
>
> static int __init deferred_probe_timeout_setup(char *str)
> {
> @@ -318,7 +317,6 @@ static void deferred_probe_timeout_work_func(struct work_struct *work)
> list_for_each_entry(p, &deferred_probe_pending_list, deferred_probe)
> dev_info(p->device, "deferred probe pending\n");
> mutex_unlock(&deferred_probe_mutex);
> - wake_up_all(&probe_timeout_waitqueue);
> }
> static DECLARE_DELAYED_WORK(deferred_probe_timeout_work, deferred_probe_timeout_work_func);
>
> @@ -736,9 +734,6 @@ int driver_probe_done(void)
> */
> void wait_for_device_probe(void)
> {
> - /* wait for probe timeout */
> - wait_event(probe_timeout_waitqueue, !driver_deferred_probe_timeout);
> -
> /* wait for the deferred probe workqueue to finish */
> flush_work(&deferred_probe_work);
>
> --
> 2.36.1
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [GIT PULL] Driver core changes for 5.19-rc1
2022-06-03 11:28 [GIT PULL] Driver core changes for 5.19-rc1 Greg KH
2022-06-03 11:31 ` [PATCH 1/2] driver core: Fix wait_for_device_probe() & deferred_probe_timeout interaction Greg Kroah-Hartman
@ 2022-06-03 19:07 ` pr-tracker-bot
2022-06-03 19:11 ` Linus Torvalds
2022-06-03 22:23 ` Linus Torvalds
3 siblings, 0 replies; 10+ messages in thread
From: pr-tracker-bot @ 2022-06-03 19:07 UTC (permalink / raw)
To: Greg KH
Cc: Linus Torvalds, Andrew Morton, linux-kernel, Stephen Rothwell,
Saravana Kannan
The pull request you sent on Fri, 3 Jun 2022 13:28:39 +0200:
> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git tags/driver-core-5.19-rc1
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/500a434fc593f1fdb274c0e6fe09a0b9c0711a4b
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [GIT PULL] Driver core changes for 5.19-rc1
2022-06-03 11:28 [GIT PULL] Driver core changes for 5.19-rc1 Greg KH
2022-06-03 11:31 ` [PATCH 1/2] driver core: Fix wait_for_device_probe() & deferred_probe_timeout interaction Greg Kroah-Hartman
2022-06-03 19:07 ` [GIT PULL] Driver core changes for 5.19-rc1 pr-tracker-bot
@ 2022-06-03 19:11 ` Linus Torvalds
2022-06-04 9:06 ` Greg KH
2022-06-03 22:23 ` Linus Torvalds
3 siblings, 1 reply; 10+ messages in thread
From: Linus Torvalds @ 2022-06-03 19:11 UTC (permalink / raw)
To: Greg KH
Cc: Andrew Morton, Linux Kernel Mailing List, Stephen Rothwell,
Saravana Kannan
On Fri, Jun 3, 2022 at 4:28 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> It's that last change that I'm the most worried about. It has been
> reported to cause boot problems for a number of systems, and I have a
> tested patch series that resolves this issue.
Ok, pulled and the two patches applied on top.
Linus
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [GIT PULL] Driver core changes for 5.19-rc1
2022-06-03 19:11 ` Linus Torvalds
@ 2022-06-04 9:06 ` Greg KH
0 siblings, 0 replies; 10+ messages in thread
From: Greg KH @ 2022-06-04 9:06 UTC (permalink / raw)
To: Linus Torvalds
Cc: Andrew Morton, Linux Kernel Mailing List, Stephen Rothwell,
Saravana Kannan
On Fri, Jun 03, 2022 at 12:11:29PM -0700, Linus Torvalds wrote:
> On Fri, Jun 3, 2022 at 4:28 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > It's that last change that I'm the most worried about. It has been
> > reported to cause boot problems for a number of systems, and I have a
> > tested patch series that resolves this issue.
>
> Ok, pulled and the two patches applied on top.
Thanks!
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [GIT PULL] Driver core changes for 5.19-rc1
2022-06-03 11:28 [GIT PULL] Driver core changes for 5.19-rc1 Greg KH
` (2 preceding siblings ...)
2022-06-03 19:11 ` Linus Torvalds
@ 2022-06-03 22:23 ` Linus Torvalds
2022-06-04 6:32 ` Takashi Iwai
3 siblings, 1 reply; 10+ messages in thread
From: Linus Torvalds @ 2022-06-03 22:23 UTC (permalink / raw)
To: Greg KH, Takashi Iwai
Cc: Andrew Morton, Linux Kernel Mailing List, Stephen Rothwell,
Saravana Kannan
Augh.
This was very badly done, and I'm not talking about the deferred probe
timeout things that caused problems for people.
On Fri, Jun 3, 2022 at 4:28 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> - firmware_loader reorganization and additions including the
> ability to have XZ compressed firmware images and the ability
> for userspace to initiate the firmware load when it needs to,
> instead of being always initiated by the kernel.
This is actively misleading.
We *always* supported XZ compressed firmware images, and it was
enabled by CONFIG_FW_LOADER_COMPRESS,
What's new is the option to use ZSTD compression.
However, the Kconfig file addition for this was done as badly as the
above explanation was, and the FW_LOADER_COMPRESS_XZ option was added
with a help message and a default value that both are complete
garbage.
So when you do "make oldconfig", you would be expected to say 'N', and
in the process you lose the existing XZ compression.
Only when the resulting kernel doesn't boot, and you spent half an
hour trying to bisect things, and you start looking closer, do you
notice that "ooh, the config changed in bad ways".
Yeah, I'm a bit grumpy. This was *really* annoying.
The commit that does this breakage is literally called "firmware: Add
the support for ZSTD-compressed firmware files", and only when looking
closer do you notice that IT REMOVES SUPPORT FOR XZ COMPRESSION BY
DEFAULT.
Because even when keeping the FW_LOADER_COMPRESS option enabled, the
XZ compression is just gone, gone, gone, unless you realize that it
was implicitly enabled before, and now needs that default disable of
FW_LOADER_COMPRESS_XZ to be enabled.
I've said this before, and I'll say it here again (and I bet I'll have
to say it in the future too): the kernel config is probably the most
annoying part of building a kernel for anybody.
And it damn well does NOT HELP when people then actively break things,
and ask actively bad and misleading questions. In this case, for
example, it's not just that the XZ option is now misleading by
default, it's also that the whole thing has been set up so that you
can say "enable compressed images", but then HAVE NO ACTUAL
COMPRESSION METHOD!
Grr. This was *REALLY* badly done.
Linus
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [GIT PULL] Driver core changes for 5.19-rc1
2022-06-03 22:23 ` Linus Torvalds
@ 2022-06-04 6:32 ` Takashi Iwai
2022-06-04 9:07 ` Greg KH
0 siblings, 1 reply; 10+ messages in thread
From: Takashi Iwai @ 2022-06-04 6:32 UTC (permalink / raw)
To: Linus Torvalds
Cc: Greg KH, Takashi Iwai, Andrew Morton, Linux Kernel Mailing List,
Stephen Rothwell, Saravana Kannan
On Sat, 04 Jun 2022 00:23:18 +0200,
Linus Torvalds wrote:
>
> Augh.
>
> This was very badly done, and I'm not talking about the deferred probe
> timeout things that caused problems for people.
>
> On Fri, Jun 3, 2022 at 4:28 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > - firmware_loader reorganization and additions including the
> > ability to have XZ compressed firmware images and the ability
> > for userspace to initiate the firmware load when it needs to,
> > instead of being always initiated by the kernel.
>
> This is actively misleading.
>
> We *always* supported XZ compressed firmware images, and it was
> enabled by CONFIG_FW_LOADER_COMPRESS,
>
> What's new is the option to use ZSTD compression.
>
> However, the Kconfig file addition for this was done as badly as the
> above explanation was, and the FW_LOADER_COMPRESS_XZ option was added
> with a help message and a default value that both are complete
> garbage.
>
> So when you do "make oldconfig", you would be expected to say 'N', and
> in the process you lose the existing XZ compression.
>
> Only when the resulting kernel doesn't boot, and you spent half an
> hour trying to bisect things, and you start looking closer, do you
> notice that "ooh, the config changed in bad ways".
>
> Yeah, I'm a bit grumpy. This was *really* annoying.
>
> The commit that does this breakage is literally called "firmware: Add
> the support for ZSTD-compressed firmware files", and only when looking
> closer do you notice that IT REMOVES SUPPORT FOR XZ COMPRESSION BY
> DEFAULT.
>
> Because even when keeping the FW_LOADER_COMPRESS option enabled, the
> XZ compression is just gone, gone, gone, unless you realize that it
> was implicitly enabled before, and now needs that default disable of
> FW_LOADER_COMPRESS_XZ to be enabled.
>
> I've said this before, and I'll say it here again (and I bet I'll have
> to say it in the future too): the kernel config is probably the most
> annoying part of building a kernel for anybody.
>
> And it damn well does NOT HELP when people then actively break things,
> and ask actively bad and misleading questions. In this case, for
> example, it's not just that the XZ option is now misleading by
> default, it's also that the whole thing has been set up so that you
> can say "enable compressed images", but then HAVE NO ACTUAL
> COMPRESSION METHOD!
>
> Grr. This was *REALLY* badly done.
Mea culpa, I hesitated to add "default y", but it should have been
added at least for the case with the config takeover case. Now I see
that you've already added "default y" to
CONFIG_FW_LOADER_COMPRESS_XZ. Thanks for taking care of this!
Takashi
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [GIT PULL] Driver core changes for 5.19-rc1
2022-06-04 6:32 ` Takashi Iwai
@ 2022-06-04 9:07 ` Greg KH
0 siblings, 0 replies; 10+ messages in thread
From: Greg KH @ 2022-06-04 9:07 UTC (permalink / raw)
To: Takashi Iwai
Cc: Linus Torvalds, Andrew Morton, Linux Kernel Mailing List,
Stephen Rothwell, Saravana Kannan
On Sat, Jun 04, 2022 at 08:32:29AM +0200, Takashi Iwai wrote:
> On Sat, 04 Jun 2022 00:23:18 +0200,
> Linus Torvalds wrote:
> >
> > Augh.
> >
> > This was very badly done, and I'm not talking about the deferred probe
> > timeout things that caused problems for people.
> >
> > On Fri, Jun 3, 2022 at 4:28 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> > >
> > > - firmware_loader reorganization and additions including the
> > > ability to have XZ compressed firmware images and the ability
> > > for userspace to initiate the firmware load when it needs to,
> > > instead of being always initiated by the kernel.
> >
> > This is actively misleading.
> >
> > We *always* supported XZ compressed firmware images, and it was
> > enabled by CONFIG_FW_LOADER_COMPRESS,
> >
> > What's new is the option to use ZSTD compression.
> >
> > However, the Kconfig file addition for this was done as badly as the
> > above explanation was, and the FW_LOADER_COMPRESS_XZ option was added
> > with a help message and a default value that both are complete
> > garbage.
> >
> > So when you do "make oldconfig", you would be expected to say 'N', and
> > in the process you lose the existing XZ compression.
> >
> > Only when the resulting kernel doesn't boot, and you spent half an
> > hour trying to bisect things, and you start looking closer, do you
> > notice that "ooh, the config changed in bad ways".
> >
> > Yeah, I'm a bit grumpy. This was *really* annoying.
> >
> > The commit that does this breakage is literally called "firmware: Add
> > the support for ZSTD-compressed firmware files", and only when looking
> > closer do you notice that IT REMOVES SUPPORT FOR XZ COMPRESSION BY
> > DEFAULT.
> >
> > Because even when keeping the FW_LOADER_COMPRESS option enabled, the
> > XZ compression is just gone, gone, gone, unless you realize that it
> > was implicitly enabled before, and now needs that default disable of
> > FW_LOADER_COMPRESS_XZ to be enabled.
> >
> > I've said this before, and I'll say it here again (and I bet I'll have
> > to say it in the future too): the kernel config is probably the most
> > annoying part of building a kernel for anybody.
> >
> > And it damn well does NOT HELP when people then actively break things,
> > and ask actively bad and misleading questions. In this case, for
> > example, it's not just that the XZ option is now misleading by
> > default, it's also that the whole thing has been set up so that you
> > can say "enable compressed images", but then HAVE NO ACTUAL
> > COMPRESSION METHOD!
> >
> > Grr. This was *REALLY* badly done.
>
> Mea culpa, I hesitated to add "default y", but it should have been
> added at least for the case with the config takeover case. Now I see
> that you've already added "default y" to
> CONFIG_FW_LOADER_COMPRESS_XZ. Thanks for taking care of this!
Sorry about that, I missed it too as I just selected that option to
ensure it built and worked properly on my systems :(
greg k-h
^ permalink raw reply [flat|nested] 10+ messages in thread