From 955ef4833574636819cd269cfbae12f79cbde63a Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Thu, 16 May 2013 05:09:58 +0000 Subject: [PATCH] cpufreq: Drop rwsem lock around CPUFREQ_GOV_POLICY_EXIT With the rwsem lock around __cpufreq_governor(policy, CPUFREQ_GOV_POLICY_EXIT), we get circular dependency when we call sysfs_remove_group(). ====================================================== [ INFO: possible circular locking dependency detected ] 3.9.0-rc7+ #15 Not tainted ------------------------------------------------------- cat/2387 is trying to acquire lock: (&per_cpu(cpu_policy_rwsem, cpu)){+++++.}, at: [] lock_policy_rwsem_read+0x25/0x34 but task is already holding lock: (s_active#41){++++.+}, at: [] sysfs_read_file+0x4f/0xcc which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #1 (s_active#41){++++.+}: [] lock_acquire+0x61/0xbc [] sysfs_addrm_finish+0xc1/0x128 [] sysfs_hash_and_remove+0x35/0x64 [] remove_files.isra.0+0x1b/0x24 [] sysfs_remove_group+0x2d/0xa8 [] cpufreq_governor_interactive+0x13b/0x35c [] __cpufreq_governor+0x2b/0x8c [] __cpufreq_set_policy+0xa9/0xf8 [] store_scaling_governor+0x61/0x100 [] store+0x39/0x60 [] sysfs_write_file+0xed/0x114 [] vfs_write+0x65/0xd8 [] sys_write+0x2f/0x50 [] ret_fast_syscall+0x1/0x52 -> #0 (&per_cpu(cpu_policy_rwsem, cpu)){+++++.}: [] __lock_acquire+0xef3/0x13dc [] lock_acquire+0x61/0xbc [] down_read+0x25/0x30 [] lock_policy_rwsem_read+0x25/0x34 [] show+0x21/0x58 [] sysfs_read_file+0x67/0xcc [] vfs_read+0x63/0xd8 [] sys_read+0x2f/0x50 [] ret_fast_syscall+0x1/0x52 other info that might help us debug this: Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(s_active#41); lock(&per_cpu(cpu_policy_rwsem, cpu)); lock(s_active#41); lock(&per_cpu(cpu_policy_rwsem, cpu)); *** DEADLOCK *** 2 locks held by cat/2387: #0: (&buffer->mutex){+.+.+.}, at: [] sysfs_read_file+0x25/0xcc #1: (s_active#41){++++.+}, at: [] sysfs_read_file+0x4f/0xcc stack backtrace: [] (unwind_backtrace+0x1/0x9c) from [] (print_circular_bug+0x19d/0x1e8) [] (print_circular_bug+0x19d/0x1e8) from [] (__lock_acquire+0xef3/0x13dc) [] (__lock_acquire+0xef3/0x13dc) from [] (lock_acquire+0x61/0xbc) [] (lock_acquire+0x61/0xbc) from [] (down_read+0x25/0x30) [] (down_read+0x25/0x30) from [] (lock_policy_rwsem_read+0x25/0x34) [] (lock_policy_rwsem_read+0x25/0x34) from [] (show+0x21/0x58) [] (show+0x21/0x58) from [] (sysfs_read_file+0x67/0xcc) [] (sysfs_read_file+0x67/0xcc) from [] (vfs_read+0x63/0xd8) [] (vfs_read+0x63/0xd8) from [] (sys_read+0x2f/0x50) [] (sys_read+0x2f/0x50) from [] (ret_fast_syscall+0x1/0x52) This lock isn't required while calling __cpufreq_governor(policy, CPUFREQ_GOV_POLICY_EXIT). Remove it. Signed-off-by: Viresh Kumar Signed-off-by: Rafael J. Wysocki --- drivers/cpufreq/cpufreq.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 4b8c7f2..2d53f47 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1729,18 +1729,23 @@ static int __cpufreq_set_policy(struct cpufreq_policy *data, /* end old governor */ if (data->governor) { __cpufreq_governor(data, CPUFREQ_GOV_STOP); + unlock_policy_rwsem_write(policy->cpu); __cpufreq_governor(data, CPUFREQ_GOV_POLICY_EXIT); + lock_policy_rwsem_write(policy->cpu); } /* start new governor */ data->governor = policy->governor; if (!__cpufreq_governor(data, CPUFREQ_GOV_POLICY_INIT)) { - if (!__cpufreq_governor(data, CPUFREQ_GOV_START)) + if (!__cpufreq_governor(data, CPUFREQ_GOV_START)) { failed = 0; - else + } else { + unlock_policy_rwsem_write(policy->cpu); __cpufreq_governor(data, CPUFREQ_GOV_POLICY_EXIT); + lock_policy_rwsem_write(policy->cpu); + } } if (failed) { -- 2.7.4