Messages in this thread | | | Subject | Re: [PATCH 1/4] PM / devfreq: reuse system workqueue machanism | From | Kamil Konieczny <> | Date | Tue, 10 Dec 2019 10:28:28 +0100 |
| |
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
| |