* [PATCH -next 0/2] cpufreq: powernv: Fixes for Global pstate management
@ 2016-05-03 9:40 Akshay Adiga
2016-05-03 9:40 ` [PATCH -next 1/2] cpufreq: powernv: Move smp_call_function_any() out of irq safe block Akshay Adiga
2016-05-03 9:40 ` [PATCH -next 2/2] cpufreq: powernv: del_timer_sync when global and local pstate are equal Akshay Adiga
0 siblings, 2 replies; 5+ messages in thread
From: Akshay Adiga @ 2016-05-03 9:40 UTC (permalink / raw)
To: rjw, viresh.kumar, linux-pm, linux-kernel, linuxppc-dev; +Cc: Akshay Adiga
Fixes are based on patch https://patchwork.ozlabs.org/patch/612058/ which is
in Rafael's linux-next.
- Patch [1] fixes WARN_ON in powernv_target_index()
- Patch [2] Deleting any pending timer to saves an unnecessary irq call
in powernv_target_index()
Akshay Adiga (2):
cpufreq: powernv: Move smp_call_function_any() out of irq safe block
cpufreq: powernv: del_timer_sync when global and local pstate are
equal
drivers/cpufreq/powernv-cpufreq.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
--
2.5.5
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH -next 1/2] cpufreq: powernv: Move smp_call_function_any() out of irq safe block
2016-05-03 9:40 [PATCH -next 0/2] cpufreq: powernv: Fixes for Global pstate management Akshay Adiga
@ 2016-05-03 9:40 ` Akshay Adiga
2016-05-03 11:49 ` Viresh Kumar
2016-05-03 9:40 ` [PATCH -next 2/2] cpufreq: powernv: del_timer_sync when global and local pstate are equal Akshay Adiga
1 sibling, 1 reply; 5+ messages in thread
From: Akshay Adiga @ 2016-05-03 9:40 UTC (permalink / raw)
To: rjw, viresh.kumar, linux-pm, linux-kernel, linuxppc-dev; +Cc: Akshay Adiga
Fixing 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
Moving smp_call_function_any() out of the critical section and changing
irq safe spinlocks to normal spinlocks.
Reported-by: Abdul Haleem <abdhalee@linux.vnet.linux.com>
Signed-off-by: Akshay Adiga <akshay.adiga@linux.vnet.ibm.com>
---
Patch is based on Rafael's linux-next
drivers/cpufreq/powernv-cpufreq.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/cpufreq/powernv-cpufreq.c b/drivers/cpufreq/powernv-cpufreq.c
index 144c732..1f0e20c 100644
--- a/drivers/cpufreq/powernv-cpufreq.c
+++ b/drivers/cpufreq/powernv-cpufreq.c
@@ -581,9 +581,10 @@ void gpstate_timer_handler(unsigned long data)
gpstates->last_gpstate = freq_data.gpstate_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 */
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,
{
struct powernv_smp_call_data freq_data;
unsigned int cur_msec, gpstate_id;
- unsigned long flags;
struct global_pstate_info *gpstates = policy->driver_data;
if (unlikely(rebooting) && new_index != get_nominal_index())
@@ -607,7 +607,7 @@ static int powernv_cpufreq_target_index(struct cpufreq_policy *policy,
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;
if (!gpstates->last_sampled_time) {
@@ -654,13 +654,14 @@ gpstates_done:
gpstates->last_gpstate = freq_data.gpstate_id;
gpstates->last_lpstate = freq_data.pstate_id;
+ spin_unlock(&gpstates->gpstate_lock);
+
/*
* Use smp_call_function to send IPI and execute the
* mtspr on target CPU. We could do that without IPI
* if current CPU is within policy->cpus (core)
*/
smp_call_function_any(policy->cpus, set_pstate, &freq_data, 1);
- spin_unlock_irqrestore(&gpstates->gpstate_lock, flags);
return 0;
}
--
2.5.5
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH -next 2/2] cpufreq: powernv: del_timer_sync when global and local pstate are equal
2016-05-03 9:40 [PATCH -next 0/2] cpufreq: powernv: Fixes for Global pstate management Akshay Adiga
2016-05-03 9:40 ` [PATCH -next 1/2] cpufreq: powernv: Move smp_call_function_any() out of irq safe block Akshay Adiga
@ 2016-05-03 9:40 ` Akshay Adiga
1 sibling, 0 replies; 5+ messages in thread
From: Akshay Adiga @ 2016-05-03 9:40 UTC (permalink / raw)
To: rjw, viresh.kumar, linux-pm, linux-kernel, linuxppc-dev; +Cc: Akshay Adiga
Deleting pending gpstates->timer for the policy when global and local pstate
are equal while executing target_index(). This saves an unnecessary
irq call.
Signed-off-by: Akshay Adiga <akshay.adiga@linux.vnet.ibm.com>
---
Patch is based on Rafael's linux-next
drivers/cpufreq/powernv-cpufreq.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/cpufreq/powernv-cpufreq.c b/drivers/cpufreq/powernv-cpufreq.c
index 1f0e20c..54c4536 100644
--- a/drivers/cpufreq/powernv-cpufreq.c
+++ b/drivers/cpufreq/powernv-cpufreq.c
@@ -647,6 +647,8 @@ static int powernv_cpufreq_target_index(struct cpufreq_policy *policy,
*/
if (gpstate_id != freq_data.pstate_id)
queue_gpstate_timer(gpstates);
+ else
+ del_timer_sync(&gpstates->timer);
gpstates_done:
freq_data.gpstate_id = gpstate_id;
--
2.5.5
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH -next 1/2] cpufreq: powernv: Move smp_call_function_any() out of irq safe block
2016-05-03 9:40 ` [PATCH -next 1/2] cpufreq: powernv: Move smp_call_function_any() out of irq safe block Akshay Adiga
@ 2016-05-03 11:49 ` Viresh Kumar
2016-05-03 13:18 ` Akshay Adiga
0 siblings, 1 reply; 5+ messages in thread
From: Viresh Kumar @ 2016-05-03 11:49 UTC (permalink / raw)
To: Akshay Adiga; +Cc: rjw, linux-pm, linux-kernel, linuxppc-dev
On 03-05-16, 15:10, Akshay Adiga wrote:
> Fixing 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
>
> Moving smp_call_function_any() out of the critical section and changing
> irq safe spinlocks to normal spinlocks.
>
> Reported-by: Abdul Haleem <abdhalee@linux.vnet.linux.com>
> Signed-off-by: Akshay Adiga <akshay.adiga@linux.vnet.ibm.com>
> ---
> Patch is based on Rafael's linux-next
> drivers/cpufreq/powernv-cpufreq.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/cpufreq/powernv-cpufreq.c b/drivers/cpufreq/powernv-cpufreq.c
> index 144c732..1f0e20c 100644
> --- a/drivers/cpufreq/powernv-cpufreq.c
> +++ b/drivers/cpufreq/powernv-cpufreq.c
> @@ -581,9 +581,10 @@ void gpstate_timer_handler(unsigned long data)
> gpstates->last_gpstate = freq_data.gpstate_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 */
> 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,
> {
> struct powernv_smp_call_data freq_data;
> unsigned int cur_msec, gpstate_id;
> - unsigned long flags;
> struct global_pstate_info *gpstates = policy->driver_data;
>
> if (unlikely(rebooting) && new_index != get_nominal_index())
> @@ -607,7 +607,7 @@ static int powernv_cpufreq_target_index(struct cpufreq_policy *policy,
>
> cur_msec = jiffies_to_msecs(get_jiffies_64());
>
> - spin_lock_irqsave(&gpstates->gpstate_lock, flags);
> + spin_lock(&gpstates->gpstate_lock);
You don't necessarily have to write 'what you are doing' in the commit log, but
tell us why you are doing that.
Please explain, why is this changed and why will things continue to work
without this.
--
viresh
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH -next 1/2] cpufreq: powernv: Move smp_call_function_any() out of irq safe block
2016-05-03 11:49 ` Viresh Kumar
@ 2016-05-03 13:18 ` Akshay Adiga
0 siblings, 0 replies; 5+ messages in thread
From: Akshay Adiga @ 2016-05-03 13:18 UTC (permalink / raw)
To: Viresh Kumar; +Cc: rjw, linux-pm, linux-kernel, linuxppc-dev
Hi Viresh,
On 05/03/2016 05:19 PM, Viresh Kumar wrote:
> On 03-05-16, 15:10, Akshay Adiga wrote:
>> Fixing 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
>>
>> Moving smp_call_function_any() out of the critical section and changing
>> irq safe spinlocks to normal spinlocks.
>>
>> Reported-by: Abdul Haleem <abdhalee@linux.vnet.linux.com>
>> Signed-off-by: Akshay Adiga <akshay.adiga@linux.vnet.ibm.com>
>> ---
>> Patch is based on Rafael's linux-next
>> drivers/cpufreq/powernv-cpufreq.c | 9 +++++----
>> 1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/cpufreq/powernv-cpufreq.c b/drivers/cpufreq/powernv-cpufreq.c
>> index 144c732..1f0e20c 100644
>> --- a/drivers/cpufreq/powernv-cpufreq.c
>> +++ b/drivers/cpufreq/powernv-cpufreq.c
>> @@ -581,9 +581,10 @@ void gpstate_timer_handler(unsigned long data)
>> gpstates->last_gpstate = freq_data.gpstate_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 */
>> 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,
>> {
>> struct powernv_smp_call_data freq_data;
>> unsigned int cur_msec, gpstate_id;
>> - unsigned long flags;
>> struct global_pstate_info *gpstates = policy->driver_data;
>>
>> if (unlikely(rebooting) && new_index != get_nominal_index())
>> @@ -607,7 +607,7 @@ static int powernv_cpufreq_target_index(struct cpufreq_policy *policy,
>>
>> cur_msec = jiffies_to_msecs(get_jiffies_64());
>>
>> - spin_lock_irqsave(&gpstates->gpstate_lock, flags);
>> + spin_lock(&gpstates->gpstate_lock);
> You don't necessarily have to write 'what you are doing' in the commit log, but
> tell us why you are doing that.
>
> Please explain, why is this changed and why will things continue to work
> without this.
>
Thanks for reviewing. I have tried to convey that in the first line of commit message,
"WARN_ON caused by smp_call_function_any() when irq is disabled,
because of changes made in the patch"
I see, i have not explained why i am changing irq safe spinlock to normal spinlock.
will add some explanation.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-05-03 13:19 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-03 9:40 [PATCH -next 0/2] cpufreq: powernv: Fixes for Global pstate management Akshay Adiga
2016-05-03 9:40 ` [PATCH -next 1/2] cpufreq: powernv: Move smp_call_function_any() out of irq safe block Akshay Adiga
2016-05-03 11:49 ` Viresh Kumar
2016-05-03 13:18 ` Akshay Adiga
2016-05-03 9:40 ` [PATCH -next 2/2] cpufreq: powernv: del_timer_sync when global and local pstate are equal Akshay Adiga
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).