drivers/cpufreq/cpufreq.c | 23 +++++++++++++++++------ include/linux/cpufreq.h | 4 +++- 2 files changed, 20 insertions(+), 7 deletions(-)
We should ensure the low schedule latency of cpu frequency limits work
to meet performance and power demands. so queue the policy->update work
to a dedicated thread.
Remove the rt setting of the thread in patch v1 at Tim and
Rafael J's request. However, it's will not meet everyone's request
when we add a dedicated highpri workqueue to do the policy update work.
Therefore, we keep the thread and will add a vendor hook in andorid aosp
branch lately so we can customize the thread conveniently.
Changes in v2:
- Remove the rt setting of the thread and rephrase commit msg.
- Link to v1: https://lore.kernel.org/lkml/20240717063321.629-1-pugaowei@oppo.com/
Signed-off-by: Gaowei Pu <pugaowei@oppo.com>
---
drivers/cpufreq/cpufreq.c | 23 +++++++++++++++++------
include/linux/cpufreq.h | 4 +++-
2 files changed, 20 insertions(+), 7 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index d7426e1d8bdd..3980f6789a17 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1196,7 +1196,7 @@ void refresh_frequency_limits(struct cpufreq_policy *policy)
}
EXPORT_SYMBOL(refresh_frequency_limits);
-static void handle_update(struct work_struct *work)
+static void handle_update(struct kthread_work *work)
{
struct cpufreq_policy *policy =
container_of(work, struct cpufreq_policy, update);
@@ -1213,7 +1213,7 @@ static int cpufreq_notifier_min(struct notifier_block *nb, unsigned long freq,
{
struct cpufreq_policy *policy = container_of(nb, struct cpufreq_policy, nb_min);
- schedule_work(&policy->update);
+ kthread_queue_work(policy->worker, &policy->update);
return 0;
}
@@ -1222,7 +1222,7 @@ static int cpufreq_notifier_max(struct notifier_block *nb, unsigned long freq,
{
struct cpufreq_policy *policy = container_of(nb, struct cpufreq_policy, nb_max);
- schedule_work(&policy->update);
+ kthread_queue_work(policy->worker, &policy->update);
return 0;
}
@@ -1305,14 +1305,23 @@ static struct cpufreq_policy *cpufreq_policy_alloc(unsigned int cpu)
goto err_min_qos_notifier;
}
+ policy->worker = kthread_create_worker_on_cpu(cpu, 0, "policy_worker%d", cpu);
+ if (IS_ERR(policy->worker)) {
+ dev_err(dev, "Failed to create policy_worker%d\n", cpu);
+ goto err_max_qos_notifier;
+ }
+
INIT_LIST_HEAD(&policy->policy_list);
init_rwsem(&policy->rwsem);
spin_lock_init(&policy->transition_lock);
init_waitqueue_head(&policy->transition_wait);
- INIT_WORK(&policy->update, handle_update);
+ kthread_init_work(&policy->update, handle_update);
return policy;
+err_max_qos_notifier:
+ freq_qos_remove_notifier(&policy->constraints, FREQ_QOS_MAX,
+ &policy->nb_max);
err_min_qos_notifier:
freq_qos_remove_notifier(&policy->constraints, FREQ_QOS_MIN,
&policy->nb_min);
@@ -1356,7 +1365,9 @@ static void cpufreq_policy_free(struct cpufreq_policy *policy)
&policy->nb_min);
/* Cancel any pending policy->update work before freeing the policy. */
- cancel_work_sync(&policy->update);
+ kthread_cancel_work_sync(&policy->update);
+ if (policy->worker)
+ kthread_destroy_worker(policy->worker);
if (policy->max_freq_req) {
/*
@@ -1827,7 +1838,7 @@ static unsigned int cpufreq_verify_current_freq(struct cpufreq_policy *policy, b
cpufreq_out_of_sync(policy, new_freq);
if (update)
- schedule_work(&policy->update);
+ kthread_queue_work(policy->worker, &policy->update);
}
return new_freq;
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 95f3807c8c55..728fa00a9647 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -20,6 +20,7 @@
#include <linux/spinlock.h>
#include <linux/sysfs.h>
#include <linux/minmax.h>
+#include <linux/kthread.h>
/*********************************************************************
* CPUFREQ INTERFACE *
@@ -77,8 +78,9 @@ struct cpufreq_policy {
void *governor_data;
char last_governor[CPUFREQ_NAME_LEN]; /* last governor used */
- struct work_struct update; /* if update_policy() needs to be
+ struct kthread_work update; /* if update_policy() needs to be
* called, but you're in IRQ context */
+ struct kthread_worker *worker;
struct freq_constraints constraints;
struct freq_qos_request *min_freq_req;
--
2.17.1Hi Gaowei,
kernel test robot noticed the following build errors:
[auto build test ERROR on v6.16-rc6]
[also build test ERROR on linus/master next-20250717]
[cannot apply to rafael-pm/linux-next rafael-pm/bleeding-edge]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Gaowei-Pu/cpufreq-queue-policy-update-work-to-a-dedicated-thread/20250717-165319
base: v6.16-rc6
patch link: https://lore.kernel.org/r/20250717085110.1468-1-pugaowei%40oppo.com
patch subject: [PATCH v2] cpufreq: queue policy->update work to a dedicated thread
config: i386-buildonly-randconfig-003-20250718 (https://download.01.org/0day-ci/archive/20250718/202507180955.YXkJg7bR-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14+deb12u1) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250718/202507180955.YXkJg7bR-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202507180955.YXkJg7bR-lkp@intel.com/
All errors (new ones prefixed by >>):
drivers/cpufreq/cpufreq.c: In function 'cpufreq_policy_alloc':
>> drivers/cpufreq/cpufreq.c:1308:26: error: too many arguments to function 'kthread_create_worker_on_cpu'
1308 | policy->worker = kthread_create_worker_on_cpu(cpu, 0, "policy_worker%d", cpu);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from include/linux/cpufreq.h:23,
from drivers/cpufreq/cpufreq.c:18:
include/linux/kthread.h:216:1: note: declared here
216 | kthread_create_worker_on_cpu(int cpu, unsigned int flags,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
vim +/kthread_create_worker_on_cpu +1308 drivers/cpufreq/cpufreq.c
1250
1251 static struct cpufreq_policy *cpufreq_policy_alloc(unsigned int cpu)
1252 {
1253 struct cpufreq_policy *policy;
1254 struct device *dev = get_cpu_device(cpu);
1255 int ret;
1256
1257 if (!dev)
1258 return NULL;
1259
1260 policy = kzalloc(sizeof(*policy), GFP_KERNEL);
1261 if (!policy)
1262 return NULL;
1263
1264 if (!alloc_cpumask_var(&policy->cpus, GFP_KERNEL))
1265 goto err_free_policy;
1266
1267 if (!zalloc_cpumask_var(&policy->related_cpus, GFP_KERNEL))
1268 goto err_free_cpumask;
1269
1270 if (!zalloc_cpumask_var(&policy->real_cpus, GFP_KERNEL))
1271 goto err_free_rcpumask;
1272
1273 init_completion(&policy->kobj_unregister);
1274 ret = kobject_init_and_add(&policy->kobj, &ktype_cpufreq,
1275 cpufreq_global_kobject, "policy%u", cpu);
1276 if (ret) {
1277 dev_err(dev, "%s: failed to init policy->kobj: %d\n", __func__, ret);
1278 /*
1279 * The entire policy object will be freed below, but the extra
1280 * memory allocated for the kobject name needs to be freed by
1281 * releasing the kobject.
1282 */
1283 kobject_put(&policy->kobj);
1284 goto err_free_real_cpus;
1285 }
1286
1287 freq_constraints_init(&policy->constraints);
1288
1289 policy->nb_min.notifier_call = cpufreq_notifier_min;
1290 policy->nb_max.notifier_call = cpufreq_notifier_max;
1291
1292 ret = freq_qos_add_notifier(&policy->constraints, FREQ_QOS_MIN,
1293 &policy->nb_min);
1294 if (ret) {
1295 dev_err(dev, "Failed to register MIN QoS notifier: %d (CPU%u)\n",
1296 ret, cpu);
1297 goto err_kobj_remove;
1298 }
1299
1300 ret = freq_qos_add_notifier(&policy->constraints, FREQ_QOS_MAX,
1301 &policy->nb_max);
1302 if (ret) {
1303 dev_err(dev, "Failed to register MAX QoS notifier: %d (CPU%u)\n",
1304 ret, cpu);
1305 goto err_min_qos_notifier;
1306 }
1307
> 1308 policy->worker = kthread_create_worker_on_cpu(cpu, 0, "policy_worker%d", cpu);
1309 if (IS_ERR(policy->worker)) {
1310 dev_err(dev, "Failed to create policy_worker%d\n", cpu);
1311 goto err_max_qos_notifier;
1312 }
1313
1314 INIT_LIST_HEAD(&policy->policy_list);
1315 init_rwsem(&policy->rwsem);
1316 spin_lock_init(&policy->transition_lock);
1317 init_waitqueue_head(&policy->transition_wait);
1318 kthread_init_work(&policy->update, handle_update);
1319
1320 return policy;
1321
1322 err_max_qos_notifier:
1323 freq_qos_remove_notifier(&policy->constraints, FREQ_QOS_MAX,
1324 &policy->nb_max);
1325 err_min_qos_notifier:
1326 freq_qos_remove_notifier(&policy->constraints, FREQ_QOS_MIN,
1327 &policy->nb_min);
1328 err_kobj_remove:
1329 cpufreq_policy_put_kobj(policy);
1330 err_free_real_cpus:
1331 free_cpumask_var(policy->real_cpus);
1332 err_free_rcpumask:
1333 free_cpumask_var(policy->related_cpus);
1334 err_free_cpumask:
1335 free_cpumask_var(policy->cpus);
1336 err_free_policy:
1337 kfree(policy);
1338
1339 return NULL;
1340 }
1341
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On Thu, Jul 17, 2025 at 10:51 AM Gaowei Pu <pugaowei@oppo.com> wrote: > > We should ensure the low schedule latency of cpu frequency limits work > to meet performance and power demands. Why is the current arrangement insufficient? > so queue the policy->update work to a dedicated thread. > > Remove the rt setting of the thread in patch v1 at Tim and > Rafael J's request. However, it's will not meet everyone's request > when we add a dedicated highpri workqueue to do the policy update work. Why is it insufficient? > Therefore, we keep the thread and will add a vendor hook in andorid aosp > branch lately so we can customize the thread conveniently. If you want to do something in the mainline kernel just for the convenience of Android AOSP, with all due respect thereof, don't do it. This is not going to be considered for 6.17, so you may as well come back with it when 6.17-rc1 is out. Thanks!
Hi Rafael J, On 2025/7/18 2:13, Rafael J. Wysocki wrote: > On Thu, Jul 17, 2025 at 10:51 AM Gaowei Pu <pugaowei@oppo.com> wrote: >> >> We should ensure the low schedule latency of cpu frequency limits work >> to meet performance and power demands. > > Why is the current arrangement insufficient? > >> so queue the policy->update work to a dedicated thread. >> >> Remove the rt setting of the thread in patch v1 at Tim and >> Rafael J's request. However, it's will not meet everyone's request >> when we add a dedicated highpri workqueue to do the policy update work. > > Why is it insufficient? Sorry, mabye i didn't make it clear. There are lots of critical tasks with higher schedule priority than normal cfs tasks because they are related to drawing in android systems(e.g., mvp tasks on Qualcomm, vip tasks on Mediatek). The problem we met is that these critical tasks preempt the kworker doing the 'cpufreq policy->update' work and delay the cpufreq update work for about 10ms. We can't boost the kworker beacause it's a common worker thread which also doing other works. Therefore, queue the 'cpufreq policy->update' work to a dedicated thread and customize the thread is a proper way we can do, not just for convenient. > >> Therefore, we keep the thread and will add a vendor hook in andorid aosp >> branch lately so we can customize the thread conveniently. > > If you want to do something in the mainline kernel just for the > convenience of Android AOSP, with all due respect thereof, don't do > it. Okay, Should i try to pick this patch to Android AOSP branch? thanks. > > This is not going to be considered for 6.17, so you may as well come > back with it when 6.17-rc1 is out. > > Thanks!
> when we add a dedicated highpri workqueue to do the policy update work.
high-priority?
> Therefore, we keep the thread and will add a vendor hook in andorid aosp
Android OSP?
> branch lately so we can customize the thread conveniently.
>
> Changes in v2:
…
Please move your patch version descriptions behind a marker line.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.16-rc6#n784
Regards,
Markus
© 2016 - 2025 Red Hat, Inc.