Quoting Daniel Lezcano (2015-05-04 01:05:04)
On 05/01/2015 01:49 AM, Michael Turquette wrote:
[ ... ]
/* init per-policy kthread */
gd->task = kthread_run(gov_cfs_thread, policy, "kgov_cfs_task");
if (IS_ERR_OR_NULL(gd->task))
pr_err("%s: failed to create kgov_cfs_task thread\n", __func__);
init_irq_work(&gd->irq_work, gov_cfs_irq_work);
It does not make sense to have a workqueue and a kthread, this is duplicating what the workqueue already does.
There is no traditional wq here. Just irq_work handler + kthread.
I saw also the irqwork + kthread mail you sent and I believe it is the way to go. Did you think about creating a workqueue per clock line instead of using the irqworkq ?
I am confused by how you phrased the above text. This patch uses irqwork
- kthread, which you say is the way to go. But I don't use a traditional
workqueue instead of irqwork.
Sorry for the confusion. I was refering to the email:
"[Eas-dev] [PATCH] cap_gov: irq_work + workqueue".
I did implement a method that uses irqwork + wq, and it had some bugs,
Why are you using the irqwork ?
as well as the fact that it clearly increased cfs load tracking stats.
Ah, interesting. Could you elaborate ?
Oops. Forgot to answer this in my previous mail.
The kthread is currently set to use SCHED_FIFO. It is an RT task. The main reason to do this is that it receives higher priority in the runqueue and will be run BEFORE the cfs tasks.
There is also the nice side effect that the cfs governor only looks at cfs load right now. Thus the added load of doing a DVFS transition as an rt task doesn't affect the cfs load statistics and we kind of get this behavior "for free". In other words we get to avoid Heisenberg's observability principle ;-)
Of course some day if we want to start basing a dvfs decision on rt tasks stats then we will lose this behavior.
To answer your question, using a workqueue puts SCHED_OTHER tasks onto the cfs runqueues. Thus we DO see an impact to cfs load stats by doing a dvfs transition in this way.
Regards, Mike
Thanks
-- Daniel
policy->governor_data = gd;
gd->policy = policy;
+}
+static void gov_cfs_stop(struct cpufreq_policy *policy) +{
struct gov_data *gd;
gd = policy->governor_data;
kthread_stop(gd->task);
policy->governor_data = NULL;
/* FIXME replace with devm counterparts? */
kfree(gd);
+}
+static int gov_cfs_setup(struct cpufreq_policy *policy, unsigned int event) +{
switch (event) {
case CPUFREQ_GOV_START:
/* Start managing the frequency */
gov_cfs_start(policy);
return 0;
case CPUFREQ_GOV_STOP:
gov_cfs_stop(policy);
return 0;
case CPUFREQ_GOV_LIMITS: /* unused */
case CPUFREQ_GOV_POLICY_INIT: /* unused */
case CPUFREQ_GOV_POLICY_EXIT: /* unused */
break;
}
return 0;
+}
+#ifndef CONFIG_CPU_FREQ_DEFAULT_GOV_SCHED_CFS +static +#endif +struct cpufreq_governor cpufreq_gov_cfs = {
.name = "gov_cfs",
.governor = gov_cfs_setup,
.owner = THIS_MODULE,
+};
+static int __init gov_cfs_init(void) +{
return cpufreq_register_governor(&cpufreq_gov_cfs);
+}
+static void __exit gov_cfs_exit(void) +{
cpufreq_unregister_governor(&cpufreq_gov_cfs);
+}
+/* Try to make this the default governor */ +fs_initcall(gov_cfs_init);
+MODULE_LICENSE("GPL"); diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index 393fc36..a7b97f9 100644 --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -4257,6 +4257,10 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags) update_rq_runnable_avg(rq, rq->nr_running); add_nr_running(rq, 1); }
if(sched_energy_freq())
gov_cfs_update_cpu(cpu_of(rq));
}hrtick_update(rq);
@@ -4318,6 +4322,10 @@ static void dequeue_task_fair(struct rq *rq, struct task_struct *p, int flags) sub_nr_running(rq, 1); update_rq_runnable_avg(rq, 1); }
if(sched_energy_freq())
gov_cfs_update_cpu(cpu_of(rq));
}hrtick_update(rq);
@@ -7821,6 +7829,9 @@ static void task_tick_fair(struct rq *rq, struct task_struct *curr, int queued) task_tick_numa(rq, curr);
update_rq_runnable_avg(rq, 1);
if(sched_energy_freq())
gov_cfs_update_cpu(cpu_of(rq));
}
/*
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h index 63a8be9..ec23523 100644 --- a/kernel/sched/sched.h +++ b/kernel/sched/sched.h @@ -1399,6 +1399,12 @@ unsigned long arch_scale_freq_capacity(struct sched_domain *sd, int cpu) int get_cpu_usage(int cpu); unsigned long capacity_orig_of(int cpu);
+#ifdef CONFIG_CPU_FREQ_GOV_SCHED_CFS +void gov_cfs_update_cpu(int cpu); +#else +static inline void gov_cfs_update_cpu(int cpu) {} +#endif
- static inline void sched_rt_avg_update(struct rq *rq, u64 rt_delta) { rq->rt_avg += rt_delta * arch_scale_freq_capacity(NULL, cpu_of(rq));
-- http://www.linaro.org/ Linaro.org │ Open source software for ARM SoCs
Follow Linaro: http://www.facebook.com/pages/Linaro Facebook | http://twitter.com/#!/linaroorg Twitter | http://www.linaro.org/linaro-blog/ Blog
-- http://www.linaro.org/ Linaro.org │ Open source software for ARM SoCs
Follow Linaro: http://www.facebook.com/pages/Linaro Facebook | http://twitter.com/#!/linaroorg Twitter | http://www.linaro.org/linaro-blog/ Blog