Re: [PATCH] firmware_class: Move request_firmware_nowait() to workqueues

[Stephen Boyd <sboyd@codeaurora.org>]

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@codeaurora.org>
>>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>>> Cc: Kay Sievers <kay.sievers@vrfy.org>
>>>> Cc: Rafael J. Wysocki <rjw@sisk.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?

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