linux-kernel - Re: A workaround for request_firmware() stuck in module

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CACVXFVNFAKYxPQ0Vnp22Dx4gGUYCXCummC2-QrpGKdGe70Kp+Q@mail.gmail.com>
Date:	Thu, 6 Sep 2012 23:38:57 +0800
From:	Ming Lei <ming.lei@...onical.com>
To:	Alan Cox <alan@...rguk.ukuu.org.uk>
Cc:	Takashi Iwai <tiwai@...e.de>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	"Rafael J. Wysocki" <rjw@...k.pl>, Kay Sievers <kay@...y.org>,
	linux-kernel@...r.kernel.org,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>
Subject: Re: A workaround for request_firmware() stuck in module_init

On Thu, Sep 6, 2012 at 8:59 PM, Alan Cox <alan@...rguk.ukuu.org.uk> wrote:
>> Sorry, I don't see anyone explained clearly why request_firmware()
>> can't be called inside module_init() in module case, so maybe it is
>> a bit early to say it is a fix on 'bug', :-)
>
> Because the firmware load may trigger a need to load a driver to load the
> firmware.

Could you explain the problem in a bit detail?

I understand once the request_firmware is called, the firmware load request
is sent to userspace, so why is a driver loading involved? and what is the
problem?

>
>> > dev_discard_firmware()
>> >
>> > so you an instance can drop its firmware reference if it doesn't need it
>> > post probe.
>>
>> This kind of mechanism has been implemented already: request_firmware()
>> and release_firmware() will get and put a refcount.  And, the reference
>> count is associated with firmware name, and it should be so, IMO.
>
> Yes - so a dev_ firmware interface is very thin.
>
>> > You broke suspend/resume for lots of devices.
>>
>> The firmware cache mechanism will keep the firmware during suspend/resume
>> cycle to address the problem.
>
> Ok
>
>> For drivers, I understand request_firmware()/request_firmware_nowait()
>> and release_firmware() are enough. If many devices share one firmware,
>> there is only one firmware kept in memory for their requests if one holds
>> the firmware, and there is a refcount for it already, :-)
>>
>> So I don't see why it is difficult to use request/release_firmware() inside
>> drivers, :-)
>
> The big problem can be summed up in one word "asynchronous". Having

OK, I agree on it, but I think it just happens in built in situation. For module
case, I think the synchronous request_firmware should be OK.

> either an automated handler for it before ->probe is called or having the
> driver author cut and paste in
>
>         if (!dev_request_firmware(dev, blah))
>                 return -EPROBE_DEFER;
>
> avoids the need to deal with async completion after probe (and the
> *horrible* case of
>
>         probe
>                 request firmware
>         remove
>
>         firmware ready
> )
>
> in each driver

OK, it is one solution for asynchronous firmware loading.

IMO, another candidate solution is to implement asynchronous probe
for these drivers which will call request_firmware in its probe().

>
> Having an auto unload for it at the end is just neatness. Perhaps in fact
> it should be devm_request_firmware() and use the mechanism we have ?

IMO, the kind of devm_request_firmware() mechanism may have its
disadvantage because the firmware in memory may or should be
released just after it has been downloaded into device, otherwise it will
consume memory unnecessarily to keep it in the whole device lifetime
if it will be released inside device release handler just like devm resources.


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