Re: [PATCH] firmware loader: don't cancel _nowait requests whenhelper is not yet available

[Linus Torvalds]

On Tue, Mar 13, 2012 at 6:55 PM, Kay Sievers <kay@xxxxxxxx> wrote:
>
> I do NOT talk about exec, I talk about a REQUEST which can be QUEUED
> just fine, but which the kernel refuses to QUEUE, even when it will
> not harm anything. That check and warning is wrong.

No. request_firmware() is synchronous. People call it, and then use
the loaded firmware immediately after success.

And no, it's not at all clear that you can just say "we'll just wait".
The device that wants the firmware may be something that can be
brought up asynchronously (so "wait until everything is ok" can work),
BUT IT IS NOT AT ALL GUARANTEED.

Seriously, you don't know what you are talking about. We *used* to
just wait, and have a thirty-second timeout etc instead of failing
aggressively. It often worked.

And then quite often it DID NOT WORK, and it caused 30-second delays
at resume time (maybe it was 60s, I forget). Because it turns out that
people were waiting for that device to get through its resume until
the resume was completed. And the system wasn't up. Deadlock.

So a 30-second "let's try delaying this" has been done. It didn't
work. Stop arguing for it, when you clearly don't know what the
problems were. The 30-second delay is when the user just says "resume
didn't work" and has long since pressed the power button.

So fail fast, fail hard. DO NOT REQUEST FIRMWARE WHILE THE SYSTEM IS SUSPENDED.

It really is that simple. Your "let's just queue it up" is crap. Stop
arguing for it, we used to do it, it "worked" much of the time, and
when it didn't work it killed the system.

Which is why it now fails with a big warning. EXACTLY SO THAT DRIVER
WRITERS WOULD KNOW NOT TO DO THAT BROKEN THING.

I'm shouting at you, because you are arguing from ignorance. Stop
doing it. "Works most of the time" just isn't good enough. It needs to
*always* work, and the way to do that is to just do firmware load when
the system is up and running, not when it is suspended.

If a driver is ok with being delayed for a firmware load, it should
just do its resume *without* doing the firmware load at all, and
schedule one serparately for later. But no, we can not just say "let's
delay request_firmware()", because we used to do that and it was shown
to not work.

Comprende?

              Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/