[PATCH 41/70] cpufreq: interactive: fix race on governor start/stop

From: BÃlint Czobor
Date: Tue Oct 27 2015 - 13:33:09 EST


From: Lianwei Wang <a22439@xxxxxxxxxxxx>

There is race condition when both two cpu do CPUFREQ_GOV_STOP and one cpu
do CPUFREQ_GOV_START soon. The sysfs_remove_group is not done yet on one
cpu, but sysfs_create_group is called on another cpu, which cause governor
start failed and then kernel panic in timer callback because the policy and
cpu mask are all kfree in cpufreq driver.

Replace atomic with mutex to lock the whole START/STOP sequence.

Change-Id: I3762b3d44315ae021b8275aca84f5ea9147cc540
Signed-off-by: Lianwei Wang <a22439@xxxxxxxxxxxx>
Signed-off-by: BÃlint Czobor <czoborbalint@xxxxxxxxx>
---
drivers/cpufreq/cpufreq_interactive.c | 21 +++++++++++++++++----
1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/cpufreq/cpufreq_interactive.c b/drivers/cpufreq/cpufreq_interactive.c
index e7f26aa..3447e58 100644
--- a/drivers/cpufreq/cpufreq_interactive.c
+++ b/drivers/cpufreq/cpufreq_interactive.c
@@ -35,7 +35,7 @@
#define CREATE_TRACE_POINTS
#include <trace/events/cpufreq_interactive.h>

-static atomic_t active_count = ATOMIC_INIT(0);
+static int active_count;

struct cpufreq_interactive_cpuinfo {
struct timer_list cpu_timer;
@@ -61,6 +61,7 @@ static DEFINE_PER_CPU(struct cpufreq_interactive_cpuinfo, cpuinfo);
static struct task_struct *speedchange_task;
static cpumask_t speedchange_cpumask;
static spinlock_t speedchange_cpumask_lock;
+static struct mutex gov_lock;

/* Hi speed to bump to from lo speed when load burst (default max) */
static unsigned int hispeed_freq;
@@ -914,6 +915,8 @@ static int cpufreq_governor_interactive(struct cpufreq_policy *policy,
if (!cpu_online(policy->cpu))
return -EINVAL;

+ mutex_lock(&gov_lock);
+
freq_table =
cpufreq_frequency_get_table(policy->cpu);
if (!hispeed_freq)
@@ -948,20 +951,26 @@ static int cpufreq_governor_interactive(struct cpufreq_policy *policy,
* Do not register the idle hook and create sysfs
* entries if we have already done so.
*/
- if (atomic_inc_return(&active_count) > 1)
+ if (++active_count > 1) {
+ mutex_unlock(&gov_lock);
return 0;
+ }

rc = sysfs_create_group(cpufreq_global_kobject,
&interactive_attr_group);
- if (rc)
+ if (rc) {
+ mutex_unlock(&gov_lock);
return rc;
+ }

idle_notifier_register(&cpufreq_interactive_idle_nb);
cpufreq_register_notifier(
&cpufreq_notifier_block, CPUFREQ_TRANSITION_NOTIFIER);
+ mutex_unlock(&gov_lock);
break;

case CPUFREQ_GOV_STOP:
+ mutex_lock(&gov_lock);
for_each_cpu(j, policy->cpus) {
pcpu = &per_cpu(cpuinfo, j);
down_write(&pcpu->enable_sem);
@@ -971,14 +980,17 @@ static int cpufreq_governor_interactive(struct cpufreq_policy *policy,
up_write(&pcpu->enable_sem);
}

- if (atomic_dec_return(&active_count) > 0)
+ if (--active_count > 0) {
+ mutex_unlock(&gov_lock);
return 0;
+ }

cpufreq_unregister_notifier(
&cpufreq_notifier_block, CPUFREQ_TRANSITION_NOTIFIER);
idle_notifier_unregister(&cpufreq_interactive_idle_nb);
sysfs_remove_group(cpufreq_global_kobject,
&interactive_attr_group);
+ mutex_unlock(&gov_lock);

break;

@@ -1018,6 +1030,7 @@ static int __init cpufreq_interactive_init(void)

spin_lock_init(&target_loads_lock);
spin_lock_init(&speedchange_cpumask_lock);
+ mutex_init(&gov_lock);
speedchange_task =
kthread_create(cpufreq_interactive_speedchange_task, NULL,
"cfinteractive");
--
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/