Re: [PATCH v1 2/2] powernv, cpufreq: Add per-core locking to serialize frequency transitions

2014-02-11 Thread Preeti U Murthy
Hi Vaidy,

On 02/11/2014 12:32 PM, Vaidyanathan Srinivasan wrote:
 From: Srivatsa S. Bhat srivatsa.b...@linux.vnet.ibm.com
 
 On POWER systems, the CPU frequency is controlled at a core-level and
 hence we need to serialize so that only one of the threads in the core
 switches the core's frequency at a time.
 
 Using a global mutex lock would needlessly serialize _all_ frequency
 transitions in the system (across all cores). So introduce per-core
 locking to enable finer-grained synchronization and thereby enhance
 the speed and responsiveness of the cpufreq driver to varying workload
 demands.
 
 The design of per-core locking is very simple and straight-forward: we
 first define a Per-CPU lock and use the ones that belongs to the first
 thread sibling of the core.
 
 cpu_first_thread_sibling() macro is used to find the *common* lock for
 all thread siblings belonging to a core.
 
 Signed-off-by: Srivatsa S. Bhat srivatsa.b...@linux.vnet.ibm.com
 Signed-off-by: Vaidyanathan Srinivasan sva...@linux.vnet.ibm.com
 ---
  drivers/cpufreq/powernv-cpufreq.c |   21 -
  1 file changed, 16 insertions(+), 5 deletions(-)
 
 diff --git a/drivers/cpufreq/powernv-cpufreq.c 
 b/drivers/cpufreq/powernv-cpufreq.c
 index ea3b630..8240e90 100644
 --- a/drivers/cpufreq/powernv-cpufreq.c
 +++ b/drivers/cpufreq/powernv-cpufreq.c
 @@ -24,8 +24,15 @@
  #include linux/of.h
  #include asm/cputhreads.h
 
 -/* FIXME: Make this per-core */
 -static DEFINE_MUTEX(freq_switch_mutex);
 +/* Per-Core locking for frequency transitions */
 +static DEFINE_PER_CPU(struct mutex, freq_switch_lock);
 +
 +#define lock_core_freq(cpu)  \
 + mutex_lock(per_cpu(freq_switch_lock,\
 + cpu_first_thread_sibling(cpu)));
 +#define unlock_core_freq(cpu)\
 + mutex_unlock(per_cpu(freq_switch_lock,\
 + cpu_first_thread_sibling(cpu)));
 
  #define POWERNV_MAX_PSTATES  256
 
 @@ -219,7 +226,7 @@ static int powernv_cpufreq_target(struct cpufreq_policy 
 *policy,
   freqs.new = powernv_freqs[new_index].frequency;
   freqs.cpu = policy-cpu;
 
 - mutex_lock(freq_switch_mutex);
 + lock_core_freq(policy-cpu);
   cpufreq_notify_transition(policy, freqs, CPUFREQ_PRECHANGE);
 
   pr_debug(setting frequency for cpu %d to %d kHz index %d pstate %d,
 @@ -231,7 +238,7 @@ static int powernv_cpufreq_target(struct cpufreq_policy 
 *policy,
   rc = powernv_set_freq(policy-cpus, new_index);
 
   cpufreq_notify_transition(policy, freqs, CPUFREQ_POSTCHANGE);
 - mutex_unlock(freq_switch_mutex);
 + unlock_core_freq(policy-cpu);
 
   return rc;
  }
 @@ -248,7 +255,7 @@ static struct cpufreq_driver powernv_cpufreq_driver = {
 
  static int __init powernv_cpufreq_init(void)
  {
 - int rc = 0;
 + int cpu, rc = 0;
 
   /* Discover pstates from device tree and init */
 
 @@ -258,6 +265,10 @@ static int __init powernv_cpufreq_init(void)
   pr_info(powernv-cpufreq disabled\n);
   return rc;
   }
 + /* Init per-core mutex */
 + for_each_possible_cpu(cpu) {
 + mutex_init(per_cpu(freq_switch_lock, cpu));
 + }
 
   rc = cpufreq_register_driver(powernv_cpufreq_driver);
   return rc;

This looks good to me.

Reviewed-by: Preeti U Murthy pre...@linux.vnet.ibm.com

Thanks

Regards
Preeti U Murthy
 
 ___
 Linuxppc-dev mailing list
 Linuxppc-dev@lists.ozlabs.org
 https://lists.ozlabs.org/listinfo/linuxppc-dev
 

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


[PATCH v1 2/2] powernv, cpufreq: Add per-core locking to serialize frequency transitions

2014-02-10 Thread Vaidyanathan Srinivasan
From: Srivatsa S. Bhat srivatsa.b...@linux.vnet.ibm.com

On POWER systems, the CPU frequency is controlled at a core-level and
hence we need to serialize so that only one of the threads in the core
switches the core's frequency at a time.

Using a global mutex lock would needlessly serialize _all_ frequency
transitions in the system (across all cores). So introduce per-core
locking to enable finer-grained synchronization and thereby enhance
the speed and responsiveness of the cpufreq driver to varying workload
demands.

The design of per-core locking is very simple and straight-forward: we
first define a Per-CPU lock and use the ones that belongs to the first
thread sibling of the core.

cpu_first_thread_sibling() macro is used to find the *common* lock for
all thread siblings belonging to a core.

Signed-off-by: Srivatsa S. Bhat srivatsa.b...@linux.vnet.ibm.com
Signed-off-by: Vaidyanathan Srinivasan sva...@linux.vnet.ibm.com
---
 drivers/cpufreq/powernv-cpufreq.c |   21 -
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/cpufreq/powernv-cpufreq.c 
b/drivers/cpufreq/powernv-cpufreq.c
index ea3b630..8240e90 100644
--- a/drivers/cpufreq/powernv-cpufreq.c
+++ b/drivers/cpufreq/powernv-cpufreq.c
@@ -24,8 +24,15 @@
 #include linux/of.h
 #include asm/cputhreads.h
 
-/* FIXME: Make this per-core */
-static DEFINE_MUTEX(freq_switch_mutex);
+/* Per-Core locking for frequency transitions */
+static DEFINE_PER_CPU(struct mutex, freq_switch_lock);
+
+#define lock_core_freq(cpu)\
+   mutex_lock(per_cpu(freq_switch_lock,\
+   cpu_first_thread_sibling(cpu)));
+#define unlock_core_freq(cpu)  \
+   mutex_unlock(per_cpu(freq_switch_lock,\
+   cpu_first_thread_sibling(cpu)));
 
 #define POWERNV_MAX_PSTATES256
 
@@ -219,7 +226,7 @@ static int powernv_cpufreq_target(struct cpufreq_policy 
*policy,
freqs.new = powernv_freqs[new_index].frequency;
freqs.cpu = policy-cpu;
 
-   mutex_lock(freq_switch_mutex);
+   lock_core_freq(policy-cpu);
cpufreq_notify_transition(policy, freqs, CPUFREQ_PRECHANGE);
 
pr_debug(setting frequency for cpu %d to %d kHz index %d pstate %d,
@@ -231,7 +238,7 @@ static int powernv_cpufreq_target(struct cpufreq_policy 
*policy,
rc = powernv_set_freq(policy-cpus, new_index);
 
cpufreq_notify_transition(policy, freqs, CPUFREQ_POSTCHANGE);
-   mutex_unlock(freq_switch_mutex);
+   unlock_core_freq(policy-cpu);
 
return rc;
 }
@@ -248,7 +255,7 @@ static struct cpufreq_driver powernv_cpufreq_driver = {
 
 static int __init powernv_cpufreq_init(void)
 {
-   int rc = 0;
+   int cpu, rc = 0;
 
/* Discover pstates from device tree and init */
 
@@ -258,6 +265,10 @@ static int __init powernv_cpufreq_init(void)
pr_info(powernv-cpufreq disabled\n);
return rc;
}
+   /* Init per-core mutex */
+   for_each_possible_cpu(cpu) {
+   mutex_init(per_cpu(freq_switch_lock, cpu));
+   }
 
rc = cpufreq_register_driver(powernv_cpufreq_driver);
return rc;

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev