Re: [PATCH] cpufreq, store_scaling_governor requires policy->rwsem to be held for duration of changing governors [v2]

[Prarit Bhargava <prarit@redhat.com>]

On 08/01/2014 01:18 PM, Stephen Boyd wrote:
> On 08/01/14 03:27, Prarit Bhargava wrote:
>>
>> Can you send me the test and the trace of the deadlock?  I'm not creating it with:
>>
> 
> This was with conservative as the default, and switching to ondemand
> 
> # cd /sys/devices/system/cpu/cpu2/cpufreq
> # ls
> affected_cpus                  scaling_available_governors
> conservative                   scaling_cur_freq
> cpuinfo_cur_freq               scaling_driver
> cpuinfo_max_freq               scaling_governor
> cpuinfo_min_freq               scaling_max_freq
> cpuinfo_transition_latency     scaling_min_freq
> related_cpus                   scaling_setspeed
> scaling_available_frequencies  stats
> # cat conservative/down_threshold
> 20
> # echo ondemand > scaling_governor

Thanks Stephen,

There's obviously a difference in our .configs.  I have a global conservative
directory, ie) /sys/devices/system/cpu/cpufreq/conservative instead of a per-cpu
governor file.

ie) what are your .config options for CPUFREQ?

Mine are:

#
# CPU Frequency scaling
#
CONFIG_CPU_FREQ=y
CONFIG_CPU_FREQ_GOV_COMMON=y
CONFIG_CPU_FREQ_STAT=m
CONFIG_CPU_FREQ_STAT_DETAILS=y
# CONFIG_CPU_FREQ_DEFAULT_GOV_PERFORMANCE is not set
# CONFIG_CPU_FREQ_DEFAULT_GOV_USERSPACE is not set
# CONFIG_CPU_FREQ_DEFAULT_GOV_ONDEMAND is not set
CONFIG_CPU_FREQ_DEFAULT_GOV_CONSERVATIVE=y
CONFIG_CPU_FREQ_GOV_PERFORMANCE=y
CONFIG_CPU_FREQ_GOV_POWERSAVE=y
CONFIG_CPU_FREQ_GOV_USERSPACE=y
CONFIG_CPU_FREQ_GOV_ONDEMAND=y
CONFIG_CPU_FREQ_GOV_CONSERVATIVE=y

Is there some other config option I have to set?

P.

> 
>  ======================================================
>  [ INFO: possible circular locking dependency detected ]
>  3.16.0-rc3-00039-ge1e38f124d87 #47 Not tainted
>  -------------------------------------------------------
>  sh/75 is trying to acquire lock:
>   (s_active#9){++++..}, at: [<c0358a94>] kernfs_remove_by_name_ns+0x3c/0x84
> 
>  but task is already holding lock:
>   (&policy->rwsem){+++++.}, at: [<c05ab1f0>] store+0x68/0xb8
> 
>  which lock already depends on the new lock.
> 
> 
>  the existing dependency chain (in reverse order) is:
> 
> -> #1 (&policy->rwsem){+++++.}:
>         [<c0359234>] kernfs_fop_open+0x138/0x298
>         [<c02fa3f4>] do_dentry_open.isra.12+0x1b0/0x2f0
>         [<c02fa604>] finish_open+0x20/0x38
>         [<c0308d34>] do_last.isra.37+0x5ac/0xb68
>         [<c03093a4>] path_openat+0xb4/0x5d8
>         [<c0309bcc>] do_filp_open+0x2c/0x80
>         [<c02fb558>] do_sys_open+0x10c/0x1c8
>         [<c020f0a0>] ret_fast_syscall+0x0/0x48
> 
> -> #0 (s_active#9){++++..}:
>         [<c0357d18>] __kernfs_remove+0x250/0x300
>         [<c0358a94>] kernfs_remove_by_name_ns+0x3c/0x84
>         [<c035aa78>] remove_files+0x34/0x78
>         [<c035aee0>] sysfs_remove_group+0x40/0x98
>         [<c05b0560>] cpufreq_governor_dbs+0x4c0/0x6ec
>         [<c05abebc>] __cpufreq_governor+0x118/0x200
>         [<c05ac0fc>] cpufreq_set_policy+0x158/0x2ac
>         [<c05ad5e4>] store_scaling_governor+0x6c/0x94
>         [<c05ab210>] store+0x88/0xb8
>         [<c035a00c>] sysfs_kf_write+0x4c/0x50
>         [<c03594d4>] kernfs_fop_write+0xc0/0x180
>         [<c02fc5c8>] vfs_write+0xa0/0x1a8
>         [<c02fc9d4>] SyS_write+0x40/0x8c
>         [<c020f0a0>] ret_fast_syscall+0x0/0x48
> 
>  other info that might help us debug this:
> 
>   Possible unsafe locking scenario:
> 
>         CPU0                    CPU1
>         ----                    ----
>    lock(&policy->rwsem);
>                                 lock(s_active#9);
>                                 lock(&policy->rwsem);
>    lock(s_active#9);
> 
>   *** DEADLOCK ***
> 
>  6 locks held by sh/75:
>   #0:  (sb_writers#4){.+.+..}, at: [<c02fc6a8>] vfs_write+0x180/0x1a8
>   #1:  (&of->mutex){+.+...}, at: [<c0359498>] kernfs_fop_write+0x84/0x180
>   #2:  (s_active#10){.+.+..}, at: [<c03594a0>] kernfs_fop_write+0x8c/0x180
>   #3:  (cpu_hotplug.lock){++++++}, at: [<c0221ef8>] get_online_cpus+0x38/0x9c
>   #4:  (cpufreq_rwsem){.+.+.+}, at: [<c05ab1d8>] store+0x50/0xb8
>   #5:  (&policy->rwsem){+++++.}, at: [<c05ab1f0>] store+0x68/0xb8
> 
>  stack backtrace:
>  CPU: 0 PID: 75 Comm: sh Not tainted 3.16.0-rc3-00039-ge1e38f124d87 #47
>  [<c0214de8>] (unwind_backtrace) from [<c02123f8>] (show_stack+0x10/0x14)
>  [<c02123f8>] (show_stack) from [<c0709e5c>] (dump_stack+0x70/0xbc)
>  [<c0709e5c>] (dump_stack) from [<c070722c>] (print_circular_bug+0x280/0x2d4)
>  [<c070722c>] (print_circular_bug) from [<c02629cc>] (__lock_acquire+0x18d0/0x1abc)
>  [<c02629cc>] (__lock_acquire) from [<c026310c>] (lock_acquire+0x9c/0x138)
>  [<c026310c>] (lock_acquire) from [<c0357d18>] (__kernfs_remove+0x250/0x300)
>  [<c0357d18>] (__kernfs_remove) from [<c0358a94>] (kernfs_remove_by_name_ns+0x3c/0x84)
>  [<c0358a94>] (kernfs_remove_by_name_ns) from [<c035aa78>] (remove_files+0x34/0x78)
>  [<c035aa78>] (remove_files) from [<c035aee0>] (sysfs_remove_group+0x40/0x98)
>  [<c035aee0>] (sysfs_remove_group) from [<c05b0560>] (cpufreq_governor_dbs+0x4c0/0x6ec)
>  [<c05b0560>] (cpufreq_governor_dbs) from [<c05abebc>] (__cpufreq_governor+0x118/0x200)
>  [<c05abebc>] (__cpufreq_governor) from [<c05ac0fc>] (cpufreq_set_policy+0x158/0x2ac)
>  [<c05ac0fc>] (cpufreq_set_policy) from [<c05ad5e4>] (store_scaling_governor+0x6c/0x94)
>  [<c05ad5e4>] (store_scaling_governor) from [<c05ab210>] (store+0x88/0xb8)
>  [<c05ab210>] (store) from [<c035a00c>] (sysfs_kf_write+0x4c/0x50)
>  [<c035a00c>] (sysfs_kf_write) from [<c03594d4>] (kernfs_fop_write+0xc0/0x180)
>  [<c03594d4>] (kernfs_fop_write) from [<c02fc5c8>] (vfs_write+0xa0/0x1a8)
>  [<c02fc5c8>] (vfs_write) from [<c02fc9d4>] (SyS_write+0x40/0x8c)
>  [<c02fc9d4>] (SyS_write) from [<c020f0a0>] (ret_fast_syscall+0x0/0x48)
> 
>