Messages in this thread | | | Subject | Re: [PATCH 1/4] PM / devfreq: reuse system workqueue machanism | From | Chanwoo Choi <> | Date | Tue, 10 Dec 2019 18:42:18 +0900 |
| |
On 12/10/19 6:28 PM, Kamil Konieczny wrote: > 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.
No, devfreq_wq has not ony one work item. If one target use the multiple devfreq device with simple_ondemand governor, devfreq_wq has the the multiple work item. It depends on the number of devfreq device with simple_ondemand governor.
> >>>> 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, Chanwoo Choi Samsung Electronics
| |