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 ?
Good question. The bulk of the Real Work is done in the kthread. We need to wake up the kthread somehow from inside enqueue_task_fair, dequeue_task_fair and task_tick_fair. These functions hold runqueue locks and disable interrupts. We cannot call any function that might sleep or call schedule().
In order to wake the kthread we use wake_up_process. The good news is that this function does not sleep. The bad news is the calling it will re-enter the scheduler, which is fatal.
Thus we need a way to call wake_up_process AFTER we exit the critical section in the scheduler where irqs are disabled. One of the ways we handled this in previous patch sets was to hack in a callback in run_rebalance_domains, but this has two problems:
1) it is an ugly hack 2) waking up the kthread here causes an undesirable periodic behavior
Juri proposed a solution to register an irq_work callback that simply calls wake_up_process (which is safe since wake_up_process will not sleep). From within the scheduler we raise an IPI. After we exit the critical section and re-enable interrupts then we handle the IPI which wakes up the kthread.
The ideal solution would be to wake up the kthread from within the scheduler's critical section via some special case which does not cause reentry. This can be done but it is a bit over my head and might not be accepted upstream.
Regards, Mike
as well as the fact that it clearly increased cfs load tracking stats.
Ah, interesting. Could you elaborate ?
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