linux-kernel - Re: [PATCH] firmware_class: Move request_firmware

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <4F63A1F6.7070700@codeaurora.org>
Date:	Fri, 16 Mar 2012 13:26:30 -0700
From:	Stephen Boyd <sboyd@...eaurora.org>
To:	"Rafael J. Wysocki" <rjw@...k.pl>
CC:	Christian Lamparter <chunkeey@...glemail.com>,
	linux-kernel@...r.kernel.org,
	Linux PM mailing list <linux-pm@...r.kernel.org>,
	"Srivatsa S. Bhat" <srivatsa.bhat@...ux.vnet.ibm.com>,
	alan@...rguk.ukuu.org.uk,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Saravana Kannan <skannan@...eaurora.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Kay Sievers <kay.sievers@...y.org>
Subject: Re: [PATCH] firmware_class: Move request_firmware_nowait() to workqueues

On 03/16/12 13:19, Rafael J. Wysocki wrote:
> On Friday, March 16, 2012, Stephen Boyd wrote:
>> On 03/15/12 15:31, Rafael J. Wysocki wrote:
>>> On Thursday, March 15, 2012, Stephen Boyd wrote:
>>>> On 03/15/12 13:07, Christian Lamparter wrote:
>>>>> On Thursday, March 15, 2012 08:50:15 PM Stephen Boyd wrote:
>>>>>> Oddly enough a work_struct was already part of the firmware_work
>>>>>> structure but nobody was using it. Instead of creating a new
>>>>>> kthread for each request_firmware_nowait() just schedule the work
>>>>>> on the system workqueue. This should avoid some overhead in
>>>>>> forking new threads when they're not strictly necessary if
>>>>>> workqueues are available.
>>>>>>
>>>>>> Signed-off-by: Stephen Boyd <sboyd@...eaurora.org>
>>>>>> Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
>>>>>> Cc: Kay Sievers <kay.sievers@...y.org>
>>>>>> Cc: Rafael J. Wysocki <rjw@...k.pl>
>>>>>> ---
>>>>>>
>>>>>> I saw this while looking at this problem we're having.
>>>>> Correct me if I'm wrong, but wouldn't that stall all other
>>>>> global workqueue tasks for up to 60 seconds [in worst case]?
>>>>>
>>>>> But I think we can get rid of the firmware_work work struct...
>>>>>
>>>> My understanding is that with concurrency managed workqueues when the
>>>> work item blocks another will be scheduled to run almost immediately. So
>>>> before that change by Tejun workqueues would have been a bad idea
>>>> because it could have blocked up to 60 second but now it should be fine
>>>> because that work item will just be put to sleep and another request
>>>> will run.
>>> Please read the description of system_wq in workqueue.h.
>>>
>>> You should have used either system_long_wq or system_nrt_wq (depending on
>>> what you really need).
>>>
>>>
>> Thanks. I think we can use system_nrt_wq then? Or maybe even the
>> unbounded workqueue system_unbound_wq?
> Hmm.  Can you please remind me what the exact role of that work item is?
>
> It loads the device's firmware, but I'm not sure in what situations that's
> supposed to happen.
>

request_firmware_nowait() is used by code that wants to get the firmware
asynchronously. Callers pass in a callback function which is called once
the firmware is retrieved. The work item will correspond to one call to
request_firmware_nowait(), where the work item will handle the sysfs
entry generation, uevent generation, and wait_for_completion() calls
that _request_firmware() does.

The work item also executes the callback function the caller passes in
which could do probably anything and could take an arbitrarily long
time. It looks like some drivers even chain request_firmware_nowait()
together by calling request_firmware_nowait() from the callback functions.

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

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