Re: [PATCH 19/21] cpuidle: create list of registered drivers

2013-10-03 Thread Daniel Lezcano 

On 10/03/2013 06:38 AM, Viresh Kumar wrote:

On 1 October 2013 00:07, Daniel Lezcano  wrote:

Interesting, thanks for the pointer.


So, should I keep this patch with SRCU?


IMHO, we should, for now, keep the code as it is and then focus on the 
lock/refcount for drivers in a separate series.



--
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 19/21] cpuidle: create list of registered drivers

2013-10-03 Thread Daniel Lezcano 

On 10/03/2013 06:38 AM, Viresh Kumar wrote:

On 1 October 2013 00:07, Daniel Lezcano daniel.lezc...@linaro.org wrote:

Interesting, thanks for the pointer.


So, should I keep this patch with SRCU?


IMHO, we should, for now, keep the code as it is and then focus on the 
lock/refcount for drivers in a separate series.



--
 http://www.linaro.org/ Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  http://www.facebook.com/pages/Linaro Facebook |
http://twitter.com/#!/linaroorg Twitter |
http://www.linaro.org/linaro-blog/ Blog

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 19/21] cpuidle: create list of registered drivers

2013-10-02 Thread Viresh Kumar 
On 1 October 2013 00:07, Daniel Lezcano  wrote:
> Interesting, thanks for the pointer.

So, should I keep this patch with SRCU?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 19/21] cpuidle: create list of registered drivers

2013-10-02 Thread Viresh Kumar 
On 1 October 2013 00:07, Daniel Lezcano daniel.lezc...@linaro.org wrote:
 Interesting, thanks for the pointer.

So, should I keep this patch with SRCU?
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 19/21] cpuidle: create list of registered drivers

2013-09-30 Thread Daniel Lezcano 

On 09/28/2013 11:33 PM, Paul E. McKenney wrote:

On Thu, Sep 26, 2013 at 10:19:14AM +0200, Daniel Lezcano wrote:

On 09/26/2013 08:17 AM, Viresh Kumar wrote:

On 26 September 2013 04:00, Daniel Lezcano  wrote:

If you introduce a list, you will have to introduce a lock to protect
it.


I missed it, should have added that :)


This lock will be in the fast path cpuidle_idle_call with the
get_driver function and conforming to the comment: "NOTE: no locks or
semaphores should be used here".

A lock has been introduced in this function already and the system hangs
with 1024 cpus.


Hmm... I see.. I didn't knew about this expectation.. What about a rcu
read/write lock? As far as I know its too lightweight... Can we have that
in fast path?


Nope, we can't use rcu in the idle path :)

http://lists.infradead.org/pipermail/linux-arm-kernel/2012-February/083054.html


But you should be able to use SRCU in the idle path, if that helps.


Interesting, thanks for the pointer.

  -- Daniel


--
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 19/21] cpuidle: create list of registered drivers

2013-09-30 Thread Daniel Lezcano 

On 09/28/2013 11:33 PM, Paul E. McKenney wrote:

On Thu, Sep 26, 2013 at 10:19:14AM +0200, Daniel Lezcano wrote:

On 09/26/2013 08:17 AM, Viresh Kumar wrote:

On 26 September 2013 04:00, Daniel Lezcano daniel.lezc...@linaro.org wrote:

If you introduce a list, you will have to introduce a lock to protect
it.


I missed it, should have added that :)


This lock will be in the fast path cpuidle_idle_call with the
get_driver function and conforming to the comment: NOTE: no locks or
semaphores should be used here.

A lock has been introduced in this function already and the system hangs
with 1024 cpus.


Hmm... I see.. I didn't knew about this expectation.. What about a rcu
read/write lock? As far as I know its too lightweight... Can we have that
in fast path?


Nope, we can't use rcu in the idle path :)

http://lists.infradead.org/pipermail/linux-arm-kernel/2012-February/083054.html


But you should be able to use SRCU in the idle path, if that helps.


Interesting, thanks for the pointer.

  -- Daniel


--
 http://www.linaro.org/ Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  http://www.facebook.com/pages/Linaro Facebook |
http://twitter.com/#!/linaroorg Twitter |
http://www.linaro.org/linaro-blog/ Blog

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 19/21] cpuidle: create list of registered drivers

2013-09-28 Thread Paul E. McKenney 
On Thu, Sep 26, 2013 at 10:19:14AM +0200, Daniel Lezcano wrote:
> On 09/26/2013 08:17 AM, Viresh Kumar wrote:
> >On 26 September 2013 04:00, Daniel Lezcano  wrote:
> >>If you introduce a list, you will have to introduce a lock to protect
> >>it.
> >
> >I missed it, should have added that :)
> >
> >>This lock will be in the fast path cpuidle_idle_call with the
> >>get_driver function and conforming to the comment: "NOTE: no locks or
> >>semaphores should be used here".
> >>
> >>A lock has been introduced in this function already and the system hangs
> >>with 1024 cpus.
> >
> >Hmm... I see.. I didn't knew about this expectation.. What about a rcu
> >read/write lock? As far as I know its too lightweight... Can we have that
> >in fast path?
> 
> Nope, we can't use rcu in the idle path :)
> 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2012-February/083054.html

But you should be able to use SRCU in the idle path, if that helps.

Thanx, Paul

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 19/21] cpuidle: create list of registered drivers

2013-09-28 Thread Paul E. McKenney 
On Thu, Sep 26, 2013 at 10:19:14AM +0200, Daniel Lezcano wrote:
 On 09/26/2013 08:17 AM, Viresh Kumar wrote:
 On 26 September 2013 04:00, Daniel Lezcano daniel.lezc...@linaro.org wrote:
 If you introduce a list, you will have to introduce a lock to protect
 it.
 
 I missed it, should have added that :)
 
 This lock will be in the fast path cpuidle_idle_call with the
 get_driver function and conforming to the comment: NOTE: no locks or
 semaphores should be used here.
 
 A lock has been introduced in this function already and the system hangs
 with 1024 cpus.
 
 Hmm... I see.. I didn't knew about this expectation.. What about a rcu
 read/write lock? As far as I know its too lightweight... Can we have that
 in fast path?
 
 Nope, we can't use rcu in the idle path :)
 
 http://lists.infradead.org/pipermail/linux-arm-kernel/2012-February/083054.html

But you should be able to use SRCU in the idle path, if that helps.

Thanx, Paul

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 19/21] cpuidle: create list of registered drivers

2013-09-26 Thread Daniel Lezcano 

On 09/26/2013 08:17 AM, Viresh Kumar wrote:

On 26 September 2013 04:00, Daniel Lezcano  wrote:

If you introduce a list, you will have to introduce a lock to protect
it.


I missed it, should have added that :)


This lock will be in the fast path cpuidle_idle_call with the
get_driver function and conforming to the comment: "NOTE: no locks or
semaphores should be used here".

A lock has been introduced in this function already and the system hangs
with 1024 cpus.


Hmm... I see.. I didn't knew about this expectation.. What about a rcu
read/write lock? As far as I know its too lightweight... Can we have that
in fast path?


Nope, we can't use rcu in the idle path :)

http://lists.infradead.org/pipermail/linux-arm-kernel/2012-February/083054.html


--
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 19/21] cpuidle: create list of registered drivers

2013-09-26 Thread Viresh Kumar 
On 26 September 2013 04:00, Daniel Lezcano  wrote:
> If you introduce a list, you will have to introduce a lock to protect
> it.

I missed it, should have added that :)

> This lock will be in the fast path cpuidle_idle_call with the
> get_driver function and conforming to the comment: "NOTE: no locks or
> semaphores should be used here".
>
> A lock has been introduced in this function already and the system hangs
> with 1024 cpus.

Hmm... I see.. I didn't knew about this expectation.. What about a rcu
read/write lock? As far as I know its too lightweight... Can we have that
in fast path?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 19/21] cpuidle: create list of registered drivers

2013-09-26 Thread Viresh Kumar 
On 26 September 2013 04:00, Daniel Lezcano daniel.lezc...@linaro.org wrote:
 If you introduce a list, you will have to introduce a lock to protect
 it.

I missed it, should have added that :)

 This lock will be in the fast path cpuidle_idle_call with the
 get_driver function and conforming to the comment: NOTE: no locks or
 semaphores should be used here.

 A lock has been introduced in this function already and the system hangs
 with 1024 cpus.

Hmm... I see.. I didn't knew about this expectation.. What about a rcu
read/write lock? As far as I know its too lightweight... Can we have that
in fast path?
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 19/21] cpuidle: create list of registered drivers

2013-09-26 Thread Daniel Lezcano 

On 09/26/2013 08:17 AM, Viresh Kumar wrote:

On 26 September 2013 04:00, Daniel Lezcano daniel.lezc...@linaro.org wrote:

If you introduce a list, you will have to introduce a lock to protect
it.


I missed it, should have added that :)


This lock will be in the fast path cpuidle_idle_call with the
get_driver function and conforming to the comment: NOTE: no locks or
semaphores should be used here.

A lock has been introduced in this function already and the system hangs
with 1024 cpus.


Hmm... I see.. I didn't knew about this expectation.. What about a rcu
read/write lock? As far as I know its too lightweight... Can we have that
in fast path?


Nope, we can't use rcu in the idle path :)

http://lists.infradead.org/pipermail/linux-arm-kernel/2012-February/083054.html


--
 http://www.linaro.org/ Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  http://www.facebook.com/pages/Linaro Facebook |
http://twitter.com/#!/linaroorg Twitter |
http://www.linaro.org/linaro-blog/ Blog

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 19/21] cpuidle: create list of registered drivers

2013-09-25 Thread Daniel Lezcano 
On 09/22/2013 03:21 AM, Viresh Kumar wrote:
> Currently we have multiple definitions of few routines based on following 
> config
> option: CONFIG_CPU_IDLE_MULTIPLE_DRIVERS.
> 
> These are present to save space by not creating per-cpu variable for platforms
> which need only one cpuidle driver to be registered for all CPUs.
> 
> But this setup has a problem. For ARM multi-platform kernel use case this 
> option
> will get enabled and so we will have per-cpu variables even for platforms that
> don't need it.
> 
> The bigger problem is two separate code paths for such platforms for single &
> multi platform kernels. Which doesn't sound good.
> 
> A better way of solving this problem would be to create cpuidle driver's list
> that can be used to manage all information we need. Then we don't really have 
> to
> write any special code for handling platforms with
> CONFIG_CPU_IDLE_MULTIPLE_DRIVERS option set.
> 
> This patch does it.

If you introduce a list, you will have to introduce a lock to protect
it. This lock will be in the fast path cpuidle_idle_call with the
get_driver function and conforming to the comment: "NOTE: no locks or
semaphores should be used here".

A lock has been introduced in this function already and the system hangs
with 1024 cpus.

> Signed-off-by: Viresh Kumar 
> ---
>  drivers/cpuidle/driver.c | 106 
> ---
>  include/linux/cpuidle.h  |   1 +
>  2 files changed, 27 insertions(+), 80 deletions(-)
> 
> diff --git a/drivers/cpuidle/driver.c b/drivers/cpuidle/driver.c
> index a4a93b4..320b4ec 100644
> --- a/drivers/cpuidle/driver.c
> +++ b/drivers/cpuidle/driver.c
> @@ -18,10 +18,19 @@
>  #include "cpuidle.h"
>  
>  DEFINE_SPINLOCK(cpuidle_driver_lock);
> +static LIST_HEAD(cpuidle_detected_drivers);
>  
> -#ifdef CONFIG_CPU_IDLE_MULTIPLE_DRIVERS
> +static inline struct cpuidle_driver *
> +__cpuidle_get_driver(const struct cpumask *cpumask)
> +{
> + struct cpuidle_driver *drv;
> +
> + list_for_each_entry(drv, _detected_drivers, driver_list)
> + if (cpumask_intersects(drv->cpumask, cpumask))
> + return drv;
>  
> -static DEFINE_PER_CPU(struct cpuidle_driver *, cpuidle_drivers);
> + return NULL;
> +}
>  
>  /**
>   * __cpuidle_get_cpu_driver - return the cpuidle driver tied to a CPU.
> @@ -32,103 +41,39 @@ static DEFINE_PER_CPU(struct cpuidle_driver *, 
> cpuidle_drivers);
>   */
>  static inline struct cpuidle_driver *__cpuidle_get_cpu_driver(int cpu)
>  {
> - return per_cpu(cpuidle_drivers, cpu);
> + return __cpuidle_get_driver(cpumask_of(cpu));
>  }
>  
>  /**
> - * __cpuidle_unset_driver - unset per CPU driver variables.
> + * __cpuidle_add_driver - adds a cpuidle driver to list.
>   * @drv: a valid pointer to a struct cpuidle_driver
>   *
> - * For each CPU in the driver's CPU mask, unset the registered driver per CPU
> - * variable. If @drv is different from the registered driver, the 
> corresponding
> - * variable is not cleared.
> - */
> -static inline void __cpuidle_unset_driver(struct cpuidle_driver *drv)
> -{
> - int cpu;
> -
> - for_each_cpu(cpu, drv->cpumask) {
> -
> - if (drv != __cpuidle_get_cpu_driver(cpu))
> - continue;
> -
> - per_cpu(cpuidle_drivers, cpu) = NULL;
> - }
> -}
> -
> -/**
> - * __cpuidle_set_driver - set per CPU driver variables for the given driver.
> - * @drv: a valid pointer to a struct cpuidle_driver
> - *
> - * For each CPU in the driver's cpumask, unset the registered driver per CPU
> - * to @drv.
> + * Adds cpuidle driver to cpuidle_detected_drivers list if no driver is 
> already
> + * registered for any CPUs present in drv->cpumask.
>   *
>   * Returns 0 on success, -EBUSY if the CPUs have driver(s) already.
>   */
> -static inline int __cpuidle_set_driver(struct cpuidle_driver *drv)
> -{
> - int cpu;
> -
> - for_each_cpu(cpu, drv->cpumask) {
> -
> - if (__cpuidle_get_cpu_driver(cpu)) {
> - __cpuidle_unset_driver(drv);
> - return -EBUSY;
> - }
> -
> - per_cpu(cpuidle_drivers, cpu) = drv;
> - }
> -
> - return 0;
> -}
> -
> -#else
> -
> -static struct cpuidle_driver *cpuidle_curr_driver;
> -
> -/**
> - * __cpuidle_get_cpu_driver - return the global cpuidle driver pointer.
> - * @cpu: ignored without the multiple driver support
> - *
> - * Return a pointer to a struct cpuidle_driver object or NULL if no driver 
> was
> - * previously registered.
> - */
> -static inline struct cpuidle_driver *__cpuidle_get_cpu_driver(int cpu)
> -{
> - return cpuidle_curr_driver;
> -}
> -
> -/**
> - * __cpuidle_set_driver - assign the global cpuidle driver variable.
> - * @drv: pointer to a struct cpuidle_driver object
> - *
> - * Returns 0 on success, -EBUSY if the driver is already registered.
> - */
> -static inline int __cpuidle_set_driver(struct cpuidle_driver *drv)
> +static inline int __cpuidle_add_driver(struct

Re: [PATCH 19/21] cpuidle: create list of registered drivers

2013-09-25 Thread Daniel Lezcano 
On 09/22/2013 03:21 AM, Viresh Kumar wrote:
 Currently we have multiple definitions of few routines based on following 
 config
 option: CONFIG_CPU_IDLE_MULTIPLE_DRIVERS.
 
 These are present to save space by not creating per-cpu variable for platforms
 which need only one cpuidle driver to be registered for all CPUs.
 
 But this setup has a problem. For ARM multi-platform kernel use case this 
 option
 will get enabled and so we will have per-cpu variables even for platforms that
 don't need it.
 
 The bigger problem is two separate code paths for such platforms for single 
 multi platform kernels. Which doesn't sound good.
 
 A better way of solving this problem would be to create cpuidle driver's list
 that can be used to manage all information we need. Then we don't really have 
 to
 write any special code for handling platforms with
 CONFIG_CPU_IDLE_MULTIPLE_DRIVERS option set.
 
 This patch does it.

If you introduce a list, you will have to introduce a lock to protect
it. This lock will be in the fast path cpuidle_idle_call with the
get_driver function and conforming to the comment: NOTE: no locks or
semaphores should be used here.

A lock has been introduced in this function already and the system hangs
with 1024 cpus.

 Signed-off-by: Viresh Kumar viresh.ku...@linaro.org
 ---
  drivers/cpuidle/driver.c | 106 
 ---
  include/linux/cpuidle.h  |   1 +
  2 files changed, 27 insertions(+), 80 deletions(-)
 
 diff --git a/drivers/cpuidle/driver.c b/drivers/cpuidle/driver.c
 index a4a93b4..320b4ec 100644
 --- a/drivers/cpuidle/driver.c
 +++ b/drivers/cpuidle/driver.c
 @@ -18,10 +18,19 @@
  #include cpuidle.h
  
  DEFINE_SPINLOCK(cpuidle_driver_lock);
 +static LIST_HEAD(cpuidle_detected_drivers);
  
 -#ifdef CONFIG_CPU_IDLE_MULTIPLE_DRIVERS
 +static inline struct cpuidle_driver *
 +__cpuidle_get_driver(const struct cpumask *cpumask)
 +{
 + struct cpuidle_driver *drv;
 +
 + list_for_each_entry(drv, cpuidle_detected_drivers, driver_list)
 + if (cpumask_intersects(drv-cpumask, cpumask))
 + return drv;
  
 -static DEFINE_PER_CPU(struct cpuidle_driver *, cpuidle_drivers);
 + return NULL;
 +}
  
  /**
   * __cpuidle_get_cpu_driver - return the cpuidle driver tied to a CPU.
 @@ -32,103 +41,39 @@ static DEFINE_PER_CPU(struct cpuidle_driver *, 
 cpuidle_drivers);
   */
  static inline struct cpuidle_driver *__cpuidle_get_cpu_driver(int cpu)
  {
 - return per_cpu(cpuidle_drivers, cpu);
 + return __cpuidle_get_driver(cpumask_of(cpu));
  }
  
  /**
 - * __cpuidle_unset_driver - unset per CPU driver variables.
 + * __cpuidle_add_driver - adds a cpuidle driver to list.
   * @drv: a valid pointer to a struct cpuidle_driver
   *
 - * For each CPU in the driver's CPU mask, unset the registered driver per CPU
 - * variable. If @drv is different from the registered driver, the 
 corresponding
 - * variable is not cleared.
 - */
 -static inline void __cpuidle_unset_driver(struct cpuidle_driver *drv)
 -{
 - int cpu;
 -
 - for_each_cpu(cpu, drv-cpumask) {
 -
 - if (drv != __cpuidle_get_cpu_driver(cpu))
 - continue;
 -
 - per_cpu(cpuidle_drivers, cpu) = NULL;
 - }
 -}
 -
 -/**
 - * __cpuidle_set_driver - set per CPU driver variables for the given driver.
 - * @drv: a valid pointer to a struct cpuidle_driver
 - *
 - * For each CPU in the driver's cpumask, unset the registered driver per CPU
 - * to @drv.
 + * Adds cpuidle driver to cpuidle_detected_drivers list if no driver is 
 already
 + * registered for any CPUs present in drv-cpumask.
   *
   * Returns 0 on success, -EBUSY if the CPUs have driver(s) already.
   */
 -static inline int __cpuidle_set_driver(struct cpuidle_driver *drv)
 -{
 - int cpu;
 -
 - for_each_cpu(cpu, drv-cpumask) {
 -
 - if (__cpuidle_get_cpu_driver(cpu)) {
 - __cpuidle_unset_driver(drv);
 - return -EBUSY;
 - }
 -
 - per_cpu(cpuidle_drivers, cpu) = drv;
 - }
 -
 - return 0;
 -}
 -
 -#else
 -
 -static struct cpuidle_driver *cpuidle_curr_driver;
 -
 -/**
 - * __cpuidle_get_cpu_driver - return the global cpuidle driver pointer.
 - * @cpu: ignored without the multiple driver support
 - *
 - * Return a pointer to a struct cpuidle_driver object or NULL if no driver 
 was
 - * previously registered.
 - */
 -static inline struct cpuidle_driver *__cpuidle_get_cpu_driver(int cpu)
 -{
 - return cpuidle_curr_driver;
 -}
 -
 -/**
 - * __cpuidle_set_driver - assign the global cpuidle driver variable.
 - * @drv: pointer to a struct cpuidle_driver object
 - *
 - * Returns 0 on success, -EBUSY if the driver is already registered.
 - */
 -static inline int __cpuidle_set_driver(struct cpuidle_driver *drv)
 +static inline int __cpuidle_add_driver(struct cpuidle_driver *drv)
  {
 - if (cpuidle_curr_driver)
 + if (__cpuidle_get_driver(drv-cpumask))
   return

[PATCH 19/21] cpuidle: create list of registered drivers

2013-09-21 Thread Viresh Kumar 
Currently we have multiple definitions of few routines based on following config
option: CONFIG_CPU_IDLE_MULTIPLE_DRIVERS.

These are present to save space by not creating per-cpu variable for platforms
which need only one cpuidle driver to be registered for all CPUs.

But this setup has a problem. For ARM multi-platform kernel use case this option
will get enabled and so we will have per-cpu variables even for platforms that
don't need it.

The bigger problem is two separate code paths for such platforms for single &
multi platform kernels. Which doesn't sound good.

A better way of solving this problem would be to create cpuidle driver's list
that can be used to manage all information we need. Then we don't really have to
write any special code for handling platforms with
CONFIG_CPU_IDLE_MULTIPLE_DRIVERS option set.

This patch does it.

Signed-off-by: Viresh Kumar 
---
 drivers/cpuidle/driver.c | 106 ---
 include/linux/cpuidle.h  |   1 +
 2 files changed, 27 insertions(+), 80 deletions(-)

diff --git a/drivers/cpuidle/driver.c b/drivers/cpuidle/driver.c
index a4a93b4..320b4ec 100644
--- a/drivers/cpuidle/driver.c
+++ b/drivers/cpuidle/driver.c
@@ -18,10 +18,19 @@
 #include "cpuidle.h"
 
 DEFINE_SPINLOCK(cpuidle_driver_lock);
+static LIST_HEAD(cpuidle_detected_drivers);
 
-#ifdef CONFIG_CPU_IDLE_MULTIPLE_DRIVERS
+static inline struct cpuidle_driver *
+__cpuidle_get_driver(const struct cpumask *cpumask)
+{
+   struct cpuidle_driver *drv;
+
+   list_for_each_entry(drv, _detected_drivers, driver_list)
+   if (cpumask_intersects(drv->cpumask, cpumask))
+   return drv;
 
-static DEFINE_PER_CPU(struct cpuidle_driver *, cpuidle_drivers);
+   return NULL;
+}
 
 /**
  * __cpuidle_get_cpu_driver - return the cpuidle driver tied to a CPU.
@@ -32,103 +41,39 @@ static DEFINE_PER_CPU(struct cpuidle_driver *, 
cpuidle_drivers);
  */
 static inline struct cpuidle_driver *__cpuidle_get_cpu_driver(int cpu)
 {
-   return per_cpu(cpuidle_drivers, cpu);
+   return __cpuidle_get_driver(cpumask_of(cpu));
 }
 
 /**
- * __cpuidle_unset_driver - unset per CPU driver variables.
+ * __cpuidle_add_driver - adds a cpuidle driver to list.
  * @drv: a valid pointer to a struct cpuidle_driver
  *
- * For each CPU in the driver's CPU mask, unset the registered driver per CPU
- * variable. If @drv is different from the registered driver, the corresponding
- * variable is not cleared.
- */
-static inline void __cpuidle_unset_driver(struct cpuidle_driver *drv)
-{
-   int cpu;
-
-   for_each_cpu(cpu, drv->cpumask) {
-
-   if (drv != __cpuidle_get_cpu_driver(cpu))
-   continue;
-
-   per_cpu(cpuidle_drivers, cpu) = NULL;
-   }
-}
-
-/**
- * __cpuidle_set_driver - set per CPU driver variables for the given driver.
- * @drv: a valid pointer to a struct cpuidle_driver
- *
- * For each CPU in the driver's cpumask, unset the registered driver per CPU
- * to @drv.
+ * Adds cpuidle driver to cpuidle_detected_drivers list if no driver is already
+ * registered for any CPUs present in drv->cpumask.
  *
  * Returns 0 on success, -EBUSY if the CPUs have driver(s) already.
  */
-static inline int __cpuidle_set_driver(struct cpuidle_driver *drv)
-{
-   int cpu;
-
-   for_each_cpu(cpu, drv->cpumask) {
-
-   if (__cpuidle_get_cpu_driver(cpu)) {
-   __cpuidle_unset_driver(drv);
-   return -EBUSY;
-   }
-
-   per_cpu(cpuidle_drivers, cpu) = drv;
-   }
-
-   return 0;
-}
-
-#else
-
-static struct cpuidle_driver *cpuidle_curr_driver;
-
-/**
- * __cpuidle_get_cpu_driver - return the global cpuidle driver pointer.
- * @cpu: ignored without the multiple driver support
- *
- * Return a pointer to a struct cpuidle_driver object or NULL if no driver was
- * previously registered.
- */
-static inline struct cpuidle_driver *__cpuidle_get_cpu_driver(int cpu)
-{
-   return cpuidle_curr_driver;
-}
-
-/**
- * __cpuidle_set_driver - assign the global cpuidle driver variable.
- * @drv: pointer to a struct cpuidle_driver object
- *
- * Returns 0 on success, -EBUSY if the driver is already registered.
- */
-static inline int __cpuidle_set_driver(struct cpuidle_driver *drv)
+static inline int __cpuidle_add_driver(struct cpuidle_driver *drv)
 {
-   if (cpuidle_curr_driver)
+   if (__cpuidle_get_driver(drv->cpumask))
return -EBUSY;
 
-   cpuidle_curr_driver = drv;
+   list_add(>driver_list, _detected_drivers);
 
return 0;
 }
 
 /**
- * __cpuidle_unset_driver - unset the global cpuidle driver variable.
- * @drv: a pointer to a struct cpuidle_driver
+ * __cpuidle_remove_driver - remove cpuidle driver from list.
+ * @drv: a valid pointer to a struct cpuidle_driver
  *
- * Reset the global cpuidle variable to NULL.  If @drv does not match the
- * registered driver, do nothing.
+ * Removes cpuidle

[PATCH 19/21] cpuidle: create list of registered drivers

2013-09-21 Thread Viresh Kumar 
Currently we have multiple definitions of few routines based on following config
option: CONFIG_CPU_IDLE_MULTIPLE_DRIVERS.

These are present to save space by not creating per-cpu variable for platforms
which need only one cpuidle driver to be registered for all CPUs.

But this setup has a problem. For ARM multi-platform kernel use case this option
will get enabled and so we will have per-cpu variables even for platforms that
don't need it.

The bigger problem is two separate code paths for such platforms for single 
multi platform kernels. Which doesn't sound good.

A better way of solving this problem would be to create cpuidle driver's list
that can be used to manage all information we need. Then we don't really have to
write any special code for handling platforms with
CONFIG_CPU_IDLE_MULTIPLE_DRIVERS option set.

This patch does it.

Signed-off-by: Viresh Kumar viresh.ku...@linaro.org
---
 drivers/cpuidle/driver.c | 106 ---
 include/linux/cpuidle.h  |   1 +
 2 files changed, 27 insertions(+), 80 deletions(-)

diff --git a/drivers/cpuidle/driver.c b/drivers/cpuidle/driver.c
index a4a93b4..320b4ec 100644
--- a/drivers/cpuidle/driver.c
+++ b/drivers/cpuidle/driver.c
@@ -18,10 +18,19 @@
 #include cpuidle.h
 
 DEFINE_SPINLOCK(cpuidle_driver_lock);
+static LIST_HEAD(cpuidle_detected_drivers);
 
-#ifdef CONFIG_CPU_IDLE_MULTIPLE_DRIVERS
+static inline struct cpuidle_driver *
+__cpuidle_get_driver(const struct cpumask *cpumask)
+{
+   struct cpuidle_driver *drv;
+
+   list_for_each_entry(drv, cpuidle_detected_drivers, driver_list)
+   if (cpumask_intersects(drv-cpumask, cpumask))
+   return drv;
 
-static DEFINE_PER_CPU(struct cpuidle_driver *, cpuidle_drivers);
+   return NULL;
+}
 
 /**
  * __cpuidle_get_cpu_driver - return the cpuidle driver tied to a CPU.
@@ -32,103 +41,39 @@ static DEFINE_PER_CPU(struct cpuidle_driver *, 
cpuidle_drivers);
  */
 static inline struct cpuidle_driver *__cpuidle_get_cpu_driver(int cpu)
 {
-   return per_cpu(cpuidle_drivers, cpu);
+   return __cpuidle_get_driver(cpumask_of(cpu));
 }
 
 /**
- * __cpuidle_unset_driver - unset per CPU driver variables.
+ * __cpuidle_add_driver - adds a cpuidle driver to list.
  * @drv: a valid pointer to a struct cpuidle_driver
  *
- * For each CPU in the driver's CPU mask, unset the registered driver per CPU
- * variable. If @drv is different from the registered driver, the corresponding
- * variable is not cleared.
- */
-static inline void __cpuidle_unset_driver(struct cpuidle_driver *drv)
-{
-   int cpu;
-
-   for_each_cpu(cpu, drv-cpumask) {
-
-   if (drv != __cpuidle_get_cpu_driver(cpu))
-   continue;
-
-   per_cpu(cpuidle_drivers, cpu) = NULL;
-   }
-}
-
-/**
- * __cpuidle_set_driver - set per CPU driver variables for the given driver.
- * @drv: a valid pointer to a struct cpuidle_driver
- *
- * For each CPU in the driver's cpumask, unset the registered driver per CPU
- * to @drv.
+ * Adds cpuidle driver to cpuidle_detected_drivers list if no driver is already
+ * registered for any CPUs present in drv-cpumask.
  *
  * Returns 0 on success, -EBUSY if the CPUs have driver(s) already.
  */
-static inline int __cpuidle_set_driver(struct cpuidle_driver *drv)
-{
-   int cpu;
-
-   for_each_cpu(cpu, drv-cpumask) {
-
-   if (__cpuidle_get_cpu_driver(cpu)) {
-   __cpuidle_unset_driver(drv);
-   return -EBUSY;
-   }
-
-   per_cpu(cpuidle_drivers, cpu) = drv;
-   }
-
-   return 0;
-}
-
-#else
-
-static struct cpuidle_driver *cpuidle_curr_driver;
-
-/**
- * __cpuidle_get_cpu_driver - return the global cpuidle driver pointer.
- * @cpu: ignored without the multiple driver support
- *
- * Return a pointer to a struct cpuidle_driver object or NULL if no driver was
- * previously registered.
- */
-static inline struct cpuidle_driver *__cpuidle_get_cpu_driver(int cpu)
-{
-   return cpuidle_curr_driver;
-}
-
-/**
- * __cpuidle_set_driver - assign the global cpuidle driver variable.
- * @drv: pointer to a struct cpuidle_driver object
- *
- * Returns 0 on success, -EBUSY if the driver is already registered.
- */
-static inline int __cpuidle_set_driver(struct cpuidle_driver *drv)
+static inline int __cpuidle_add_driver(struct cpuidle_driver *drv)
 {
-   if (cpuidle_curr_driver)
+   if (__cpuidle_get_driver(drv-cpumask))
return -EBUSY;
 
-   cpuidle_curr_driver = drv;
+   list_add(drv-driver_list, cpuidle_detected_drivers);
 
return 0;
 }
 
 /**
- * __cpuidle_unset_driver - unset the global cpuidle driver variable.
- * @drv: a pointer to a struct cpuidle_driver
+ * __cpuidle_remove_driver - remove cpuidle driver from list.
+ * @drv: a valid pointer to a struct cpuidle_driver
  *
- * Reset the global cpuidle variable to NULL.  If @drv does not match the
- * registered driver, do