lkml.org 
[lkml]   [2019]   [Dec]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 1/4] PM / devfreq: reuse system workqueue machanism
From
Date
Hi Chanwoo,

On 10.12.2019 08:53, Chanwoo Choi wrote:
> On 12/10/19 4:28 PM, Kamil Konieczny wrote:
>> On 10.12.2019 02:41, Chanwoo Choi wrote:
>>> On 12/9/19 11:44 PM, Kamil Konieczny wrote:
>>>> There is no need for creating another workqueue, it is enough
>>>> to reuse system_freezable_power_efficient one.
>>>>
>>>> Signed-off-by: Kamil Konieczny <k.konieczny@samsung.com>
>>>> ---
>>>> drivers/devfreq/devfreq.c | 6 +++---
>>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
>>>> index 46a7ff7c2994..955949c6fc1f 100644
>>>> --- a/drivers/devfreq/devfreq.c
>>>> +++ b/drivers/devfreq/devfreq.c
>>>> @@ -1532,11 +1532,11 @@ static int __init devfreq_init(void)
>>>> return PTR_ERR(devfreq_class);
>>>> }
>>>>
>>>> - devfreq_wq = create_freezable_workqueue("devfreq_wq");
>>>> + devfreq_wq = system_freezable_power_efficient_wq;
>>>
>>> It affect the behaviors of whole device drivers using devfreq subsystem.
>>> It is not good to change the workqueue type without any reasonable
>>> data like experiment result, power-consumption result and performance
>>> result for almost device drivers using devfreq subsystem.
>>>
>>> Are there any problem or any benefit to change workqueue type?
>>
>> The workqueue is freezable with additional capability of 'power_efficient',
>> it is already developed by linux community so why not reuse it ?
>
> As you agreed below, why don't you suggest the any reasonable test result
> with this patch? As I commented, it affects the all device drivers.
> It is necessary to suggest the test result on multiple scenarios
> in order to prevent the any power-consumption and performance regression.
> It is not easy to change them without any data.
>
> Frankly, if you test almost scenarios and suggest the reasonable result
> that anyone can understand, like there are never difference
> between "create_freezable_workqueue("devfreq_wq");" and system_freezable_power_efficient_wq.
> But you don't suggest any data.

I agree about tests data needed for deciding about change. As I already wrote in other
letter, I do not have such tests procedures, so if you have them and you may share
them with me or Marek, I can run them and gather tests results.

> - The original devfreq_wq include the only work related to devfreq.
> - system_freezable_power_efficient_wq include the all works registered
> from both other subsystem and device drivers in linux kernel.

I do not know that good system wq, devfreq_wq have only one work item so
imho it is not beneficial to use separate wq. Seperate wq can be good
during debugging problems with wq.

>>> Actually, it is not simple to change the like just one device driver
>>> because devfreq subsytem is very important for both performance
>>> and power-consumption.
>>
>> I agree. The name of this wq promises what you want, both freezable
>> and power efficiency.
>>
>>> If you hope to change the feature related to both performance
>>> and power-consumption, please suggest the reasonable data
>>> with fundamental reason.
>>>
>>> So, I can't agree it.
>>>
>>>
>>>> if (!devfreq_wq) {
>>>> class_destroy(devfreq_class);
>>>> - pr_err("%s: couldn't create workqueue\n", __FILE__);
>>>> - return -ENOMEM;
>>>> + pr_err("%s: system_freezable_power_efficient_wq isn't initialized\n", __FILE__);
>>>> + return -EINVAL;
>>>> }
>>>> devfreq_class->dev_groups = devfreq_groups;

--
Best regards,
Kamil Konieczny
Samsung R&D Institute Poland

\
 
 \ /
  Last update: 2019-12-10 10:29    [W:0.063 / U:0.096 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site