From 19c763031acb831a5ab9c1a701b7fedda073eb3f Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Sat, 31 Aug 2013 17:48:23 +0530 Subject: cpufreq: serialize calls to __cpufreq_governor() We can't take a big lock around __cpufreq_governor() as this causes recursive locking for some cases. But calls to this routine must be serialized for every policy. Otherwise we can see some unpredictable events. For example, consider following scenario: __cpufreq_remove_dev() __cpufreq_governor(policy, CPUFREQ_GOV_STOP); policy->governor->governor(policy, CPUFREQ_GOV_STOP); cpufreq_governor_dbs() case CPUFREQ_GOV_STOP: mutex_destroy(&cpu_cdbs->timer_mutex) cpu_cdbs->cur_policy = NULL; store() __cpufreq_set_policy() __cpufreq_governor(policy, CPUFREQ_GOV_LIMITS); policy->governor->governor(policy, CPUFREQ_GOV_LIMITS); case CPUFREQ_GOV_LIMITS: mutex_lock(&cpu_cdbs->timer_mutex); <-- Warning (destroyed mutex) if (policy->max < cpu_cdbs->cur_policy->cur) <- cur_policy == NULL And so store() will eventually result in a crash if cur_policy is NULL at this point. Introduce an additional variable which would guarantee serialization here. Reported-by: Stephen Boyd Signed-off-by: Viresh Kumar Signed-off-by: Rafael J. Wysocki --- include/linux/cpufreq.h | 1 + 1 file changed, 1 insertion(+) (limited to 'include') diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h index d568f3975ee..cca885dac1d 100644 --- a/include/linux/cpufreq.h +++ b/include/linux/cpufreq.h @@ -76,6 +76,7 @@ struct cpufreq_policy { struct cpufreq_governor *governor; /* see below */ void *governor_data; bool governor_enabled; /* governor start/stop flag */ + bool governor_busy; struct work_struct update; /* if update_policy() needs to be * called, but you're in IRQ context */ -- cgit v1.2.3-18-g5258 From 56d07db274b7b15ca38b60ea4a762d40de093000 Mon Sep 17 00:00:00 2001 From: "Srivatsa S. Bhat" Date: Sat, 7 Sep 2013 01:23:55 +0530 Subject: cpufreq: Remove temporary fix for race between CPU hotplug and sysfs-writes Commit "cpufreq: serialize calls to __cpufreq_governor()" had been a temporary and partial solution to the race condition between writing to a cpufreq sysfs file and taking a CPU offline. Now that we have a proper and complete solution to that problem, remove the temporary fix. Signed-off-by: Srivatsa S. Bhat Signed-off-by: Rafael J. Wysocki --- include/linux/cpufreq.h | 1 - 1 file changed, 1 deletion(-) (limited to 'include') diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h index cca885dac1d..d568f3975ee 100644 --- a/include/linux/cpufreq.h +++ b/include/linux/cpufreq.h @@ -76,7 +76,6 @@ struct cpufreq_policy { struct cpufreq_governor *governor; /* see below */ void *governor_data; bool governor_enabled; /* governor start/stop flag */ - bool governor_busy; struct work_struct update; /* if update_policy() needs to be * called, but you're in IRQ context */ -- cgit v1.2.3-18-g5258 From 798282a8718347b04a2f0a4bae7d775c48c6bcb9 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 10 Sep 2013 02:54:50 +0200 Subject: Revert "cpufreq: make sure frequency transitions are serialized" Commit 7c30ed5 (cpufreq: make sure frequency transitions are serialized) attempted to serialize frequency transitions by adding checks to the CPUFREQ_PRECHANGE and CPUFREQ_POSTCHANGE notifications. However, it assumed that the notifications will always originate from the driver's .target() callback, but they also can be triggered by cpufreq_out_of_sync() and that leads to warnings like this on some systems: WARNING: CPU: 0 PID: 14543 at drivers/cpufreq/cpufreq.c:317 __cpufreq_notify_transition+0x238/0x260() In middle of another frequency transition accompanied by a call trace similar to this one: [] dump_stack+0x46/0x58 [] warn_slowpath_common+0x8c/0xc0 [] ? acpi_cpufreq_target+0x320/0x320 [] warn_slowpath_fmt+0x46/0x50 [] __cpufreq_notify_transition+0x238/0x260 [] cpufreq_notify_transition+0x3e/0x70 [] cpufreq_out_of_sync+0x6d/0xb0 [] cpufreq_update_policy+0x10c/0x160 [] ? cpufreq_update_policy+0x160/0x160 [] cpufreq_set_cur_state+0x8c/0xb5 [] processor_set_cur_state+0xa3/0xcf [] thermal_cdev_update+0x9c/0xb0 [] step_wise_throttle+0x5a/0x90 [] handle_thermal_trip+0x4f/0x140 [] thermal_zone_device_update+0x57/0xa0 [] acpi_thermal_check+0x2e/0x30 [] acpi_thermal_notify+0x40/0xdc [] acpi_device_notify+0x19/0x1b [] acpi_ev_notify_dispatch+0x41/0x5c [] acpi_os_execute_deferred+0x25/0x32 [] process_one_work+0x170/0x4a0 [] worker_thread+0x121/0x390 [] ? manage_workers.isra.20+0x170/0x170 [] kthread+0xc0/0xd0 [] ? flush_kthread_worker+0xb0/0xb0 [] ret_from_fork+0x7c/0xb0 [] ? flush_kthread_worker+0xb0/0xb0 For this reason, revert commit 7c30ed5 along with the fix 266c13d (cpufreq: Fix serialization of frequency transitions) on top of it and we will revisit the serialization problem later. Reported-by: Alessandro Bono Signed-off-by: Rafael J. Wysocki --- include/linux/cpufreq.h | 1 - 1 file changed, 1 deletion(-) (limited to 'include') diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h index d568f3975ee..fcabc42d66a 100644 --- a/include/linux/cpufreq.h +++ b/include/linux/cpufreq.h @@ -85,7 +85,6 @@ struct cpufreq_policy { struct list_head policy_list; struct kobject kobj; struct completion kobj_unregister; - int transition_ongoing; /* Tracks transition status */ }; /* Only for ACPI */ -- cgit v1.2.3-18-g5258