Commit 1fd3ff28 authored by Akshay Adiga's avatar Akshay Adiga Committed by Rafael J. Wysocki

cpufreq: powernv: Move smp_call_function_any() out of irq safe block

Fix a WARN_ON caused by smp_call_function_any() when irq is disabled,
because of changes made in the patch ('cpufreq: powernv: Ramp-down
 global pstate slower than local-pstate')
https://patchwork.ozlabs.org/patch/612058/

 WARNING: CPU: 0 PID: 4 at kernel/smp.c:291
smp_call_function_single+0x170/0x180

 Call Trace:
 [c0000007f648f9f0] [c0000007f648fa90] 0xc0000007f648fa90 (unreliable)
 [c0000007f648fa30] [c0000000001430e0] smp_call_function_any+0x170/0x1c0
 [c0000007f648fa90] [c0000000007b4b00]
powernv_cpufreq_target_index+0xe0/0x250
 [c0000007f648fb00] [c0000000007ac9dc]
__cpufreq_driver_target+0x20c/0x3d0
 [c0000007f648fbc0] [c0000000007b1b4c] od_dbs_timer+0xcc/0x260
 [c0000007f648fc10] [c0000000007b3024] dbs_work_handler+0x54/0xa0
 [c0000007f648fc50] [c0000000000c49a8] process_one_work+0x1d8/0x590
 [c0000007f648fce0] [c0000000000c4e08] worker_thread+0xa8/0x660
 [c0000007f648fd80] [c0000000000cca88] kthread+0x108/0x130
 [c0000007f648fe30] [c0000000000095e8] ret_from_kernel_thread+0x5c/0x74

- Calling smp_call_function_any() with interrupt disabled (through
 spin_lock_irqsave) could cause a deadlock, as smp_call_function_any()
 relies on the IPI to complete. This is detected in the
 smp_call_function_any() call and hence the WARN_ON.

- As the spinlock (gpstates->lock) is only used to synchronize access of
 global_pstate_info  between timer irq handler and target_index calls. And
 the timer irq handler just try_locks() hence it would not cause a
 deadlock. Hence could do without making spinlocks irq safe.

- As the smp_call_function_any() is a blocking call and does not access
 global_pstates_info, it could reduce the critcal section by moving
 smp_call_function_any() after giving up the lock.
Reported-by: default avatarAbdul Haleem <abdhalee@linux.vnet.linux.com>
Signed-off-by: default avatarAkshay Adiga <akshay.adiga@linux.vnet.ibm.com>
Acked-by: default avatarViresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: default avatarRafael J. Wysocki <rafael.j.wysocki@intel.com>
parent f96fd0c8
...@@ -581,9 +581,10 @@ void gpstate_timer_handler(unsigned long data) ...@@ -581,9 +581,10 @@ void gpstate_timer_handler(unsigned long data)
gpstates->last_gpstate = freq_data.gpstate_id; gpstates->last_gpstate = freq_data.gpstate_id;
gpstates->last_lpstate = freq_data.pstate_id; gpstates->last_lpstate = freq_data.pstate_id;
spin_unlock(&gpstates->gpstate_lock);
/* Timer may get migrated to a different cpu on cpu hot unplug */ /* Timer may get migrated to a different cpu on cpu hot unplug */
smp_call_function_any(policy->cpus, set_pstate, &freq_data, 1); smp_call_function_any(policy->cpus, set_pstate, &freq_data, 1);
spin_unlock(&gpstates->gpstate_lock);
} }
/* /*
...@@ -596,7 +597,6 @@ static int powernv_cpufreq_target_index(struct cpufreq_policy *policy, ...@@ -596,7 +597,6 @@ static int powernv_cpufreq_target_index(struct cpufreq_policy *policy,
{ {
struct powernv_smp_call_data freq_data; struct powernv_smp_call_data freq_data;
unsigned int cur_msec, gpstate_id; unsigned int cur_msec, gpstate_id;
unsigned long flags;
struct global_pstate_info *gpstates = policy->driver_data; struct global_pstate_info *gpstates = policy->driver_data;
if (unlikely(rebooting) && new_index != get_nominal_index()) if (unlikely(rebooting) && new_index != get_nominal_index())
...@@ -607,7 +607,7 @@ static int powernv_cpufreq_target_index(struct cpufreq_policy *policy, ...@@ -607,7 +607,7 @@ static int powernv_cpufreq_target_index(struct cpufreq_policy *policy,
cur_msec = jiffies_to_msecs(get_jiffies_64()); cur_msec = jiffies_to_msecs(get_jiffies_64());
spin_lock_irqsave(&gpstates->gpstate_lock, flags); spin_lock(&gpstates->gpstate_lock);
freq_data.pstate_id = powernv_freqs[new_index].driver_data; freq_data.pstate_id = powernv_freqs[new_index].driver_data;
if (!gpstates->last_sampled_time) { if (!gpstates->last_sampled_time) {
...@@ -654,13 +654,14 @@ static int powernv_cpufreq_target_index(struct cpufreq_policy *policy, ...@@ -654,13 +654,14 @@ static int powernv_cpufreq_target_index(struct cpufreq_policy *policy,
gpstates->last_gpstate = freq_data.gpstate_id; gpstates->last_gpstate = freq_data.gpstate_id;
gpstates->last_lpstate = freq_data.pstate_id; gpstates->last_lpstate = freq_data.pstate_id;
spin_unlock(&gpstates->gpstate_lock);
/* /*
* Use smp_call_function to send IPI and execute the * Use smp_call_function to send IPI and execute the
* mtspr on target CPU. We could do that without IPI * mtspr on target CPU. We could do that without IPI
* if current CPU is within policy->cpus (core) * if current CPU is within policy->cpus (core)
*/ */
smp_call_function_any(policy->cpus, set_pstate, &freq_data, 1); smp_call_function_any(policy->cpus, set_pstate, &freq_data, 1);
spin_unlock_irqrestore(&gpstates->gpstate_lock, flags);
return 0; return 0;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment