Re: [v4 PATCH 1/1] init: fix race between rootfs mount and firmware loading

[Roman Peniaev]

On Sat, Sep 20, 2014 at 11:42 PM, Greg Kroah-Hartman
<gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> On Sat, Sep 20, 2014 at 10:18:39PM +0900, Roman Peniaev wrote:
>> On Sat, Sep 20, 2014 at 6:42 AM, Greg Kroah-Hartman
>> <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>> > On Fri, Sep 19, 2014 at 09:44:24PM +0900, Roman Pen wrote:
>> >> The thing is that built-in modules are being inited before
>> >> rootfs mount.  Some of the modules can request firmware loading
>> >> from another thread using async 'request_firmware_nowait' call
>> >> on inition, so we can catch this kind of race:
>> >>    rootfs does not exist yet, but we are going to open and load
>> >>    firmware file requesting it from the kernel thread.
>> >>
>> >> Solution is simple: before any rootfs access firmware loader
>> >> must wait for rootfs mount.
>> >
>> > Even simpler solution, don't do that :)
>> >
>> > If your hardware needs firmware this early, and you don't put the
>> > firmare into the initrd/initramfs, then it will just be loaded later on,
>> > when the root filesystem is needed and the driver asks for it again
>> > (your driver is failing "nicely" and allows the firmware to be loaded
>> > later, right?  If not, that should be fixed...)
>> >
>> > Or, build the firmware into the kernel, I think we still have that
>> > option for some drivers that really wanted/needed this.
>> >
>> > What has changed recently that requires this type of patch?  What is
>> > wrong with the above solutions?
>>
>>
>> I definitely agree with that "do not do that", I do agree.
>
> Great, no need for a patch, we can move on now.  :)
>
>> But, I did not find any other solution, because in my case this "to be loaded
>> later" really matters. The source of the problem is the rtlwifi, which randomly
>> delays the startup, it randomly warns me about "firmware loading failed" and
>> repeats loading later. This later costs around one second. For my tiny arm
>> setup it is important. And it happens because of this race.
>
> What "costs one second"?  I don't understand, if the firmware isn't
> there, are you timing out?  Fix the driver to do async firmware loading
> and that timeout shouldn't be an issue.

Firmware exists. Driver is built-in (this is the key, because do_initcall_level,
which inits all static stuff including built-in drivers, is started
before rootfs mount).
Driver calls request_firmware_nowait on inition, so firmware_class will attempt
to load this firmware and will randomly fail. After the possible fail
the driver will try to
load alternative firmware and also can randomly fail, because the
rootfs is still not
mounted. Then driver gives up.

>
>> So, all I want to say with this patch is:
>>
>> 1. 'mount / firmware loading' race exists, and it can cost time.
>
> Switch the driver to do async firmware loading and you should be able to
> fix this.

Under async firmware loading you mean request_firmware_nowait ?
Driver uses exactly this call.

--
Roman
--
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/