Hi Juri,
On 09/03/15 14:06, Juri Lelli wrote:
We already have capacity information in the EAS energy model tables. Let's use it to both avoid duplication and achieve freq/uarch invariance.
[...]
- cpufreq_for_each_entry(pos, policy->freq_table) {
/* FIXME capacity below is not scaled for uarch */capacity = pos->frequency * SCHED_CAPACITY_SCALE / policy->max;
With CONFIG_PROVE_RCU I get:
[ 7.908644] kernel/sched/energy_model.c:271 suspicious rcu_dereference_check() usage! [ 8.024003] [ 8.024003] other info that might help us debug this: [ 8.024003] [ 8.184381] [ 8.184381] rcu_scheduler_active = 1, debug_locks = 1 [ 8.291042] 5 locks held by swapper/0/1: [ 8.345822] #0: (&dev->mutex){......}, at: [<c031837b>] __driver_attach+0x27/0x58 [ 8.455530] #1: (&dev->mutex){......}, at: [<c0318387>] __driver_attach+0x33/0x58 [ 8.566741] #2: (subsys mutex#7){+.+.+.}, at: [<c0317213>] subsys_interface_register+0x2b/0x80 [ 8.635648] scsi 0:0:0:0: Direct-Access Kingston DT 100 G2 PMAP PQ: 0 ANSI: 0 CCS [ 8.803312] #3: (cpufreq_rwsem){.+.+.+}, at: [<c038c915>] __cpufreq_add_dev.isra.27+0x6d/0x6e8 [ 8.926736] #4: (&policy->rwsem){+.+.+.}, at: [<c038cb79>] __cpufreq_add_dev.isra.27+0x2d1/0x6e8 [ 9.053076] [ 9.053076] stack backtrace: [ 9.160686] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 3.19.0-rc7+ #10 [ 9.229420] Hardware name: ARM-Versatile Express [ 9.293019] [<c0012ced>] (unwind_backtrace) from [<c000fbc9>] (show_stack+0x11/0x14) [ 9.414683] [<c000fbc9>] (show_stack) from [<c04bd991>] (dump_stack+0x65/0x70) [ 9.540294] [<c04bd991>] (dump_stack) from [<c00528c1>] (energy_model_setup+0x101/0x1b4) [ 9.675339] [<c00528c1>] (energy_model_setup) from [<c038b60b>] (__cpufreq_governor+0x63/0x180) [ 9.818187] [<c038b60b>] (__cpufreq_governor) from [<c038c639>] (cpufreq_set_policy+0x119/0x144) [ 9.962893] [<c038c639>] (cpufreq_set_policy) from [<c038c895>] (cpufreq_init_policy+0x5d/0x70) [ 10.108145] [<c038c895>] (cpufreq_init_policy) from [<c038cf15>] (__cpufreq_add_dev.isra.27+0x66d/0x6e8) [ 10.256520] [<c038cf15>] (__cpufreq_add_dev.isra.27) from [<c0317243>] (subsys_interface_register+0x5b/0x80) [ 10.407772] [<c0317243>] (subsys_interface_register) from [<c038bd93>] (cpufreq_register_driver+0xa7/0x1b0)
IMHO, some rcu locking is needed to prevent this when dealing with sd pointers:
diff --git a/kernel/sched/energy_model.c b/kernel/sched/energy_model.c index 78902d9f75e0..e17173ccfaa5 100644 --- a/kernel/sched/energy_model.c +++ b/kernel/sched/energy_model.c @@ -268,6 +268,7 @@ static void em_start(struct cpufreq_policy *policy) em->up_threshold = kcalloc(count, sizeof(unsigned int), GFP_KERNEL); em->down_threshold = kcalloc(count, sizeof(unsigned int), GFP_KERNEL);
+ rcu_read_lock(); for_each_domain(cpumask_first(policy->cpus), sd) if (!sd->child) { #ifdef CONFIG_SCHED_DEBUG @@ -285,6 +286,7 @@ static void em_start(struct cpufreq_policy *policy) kfree(em->up_threshold); kfree(em->down_threshold); kfree(em); + rcu_read_unlock(); return; }
@@ -299,6 +301,8 @@ static void em_start(struct cpufreq_policy *policy) em->down_threshold[index]); }
+ rcu_read_unlock(); + /* init per-policy kthread */ em->task = kthread_create(energy_model_thread, policy, "kenergy_model_task"); if (IS_ERR_OR_NULL(em->task))
-- Dietmar
- for_each_domain(cpumask_first(policy->cpus), sd)
if (!sd->child) {+#ifdef CONFIG_SCHED_DEBUG
[...]