Re: [PATCH 1/2] PPC: powernv: remove redundant cpuidle_idle_call()

2014-02-14 Thread Preeti U Murthy 
Hi Nicolas,

You will have to include the below patch with yours. You
could squash the two I guess, I have added the changelog
just for clarity. And you also might want to change the subject to
cpuidle/powernv. It gives a better picture.

Thanks

Regards
Preeti U Murthy


cpuidle/powernv: Add ppc64_runlatch_off/on() to idle routines

Following moving of cpuidle_idle_call() to the generic idle loop, we need to
add the runlatch functions to the idle routines on powernv which was earlier
taken care of by the arch specific idle routine.

Signed-off-by: Preeti U Murthy pre...@linux.vnet.ibm.com
---
 drivers/cpuidle/cpuidle-powernv.c |5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/cpuidle/cpuidle-powernv.c 
b/drivers/cpuidle/cpuidle-powernv.c
index 78fd174..f48607c 100644
--- a/drivers/cpuidle/cpuidle-powernv.c
+++ b/drivers/cpuidle/cpuidle-powernv.c
@@ -14,6 +14,7 @@
 
 #include asm/machdep.h
 #include asm/firmware.h
+#include asm/runlatch.h
 
 struct cpuidle_driver powernv_idle_driver = {
.name = powernv_idle,
@@ -30,12 +31,14 @@ static int snooze_loop(struct cpuidle_device *dev,
local_irq_enable();
set_thread_flag(TIF_POLLING_NRFLAG);
 
+   ppc64_runlatch_off();
while (!need_resched()) {
HMT_low();
HMT_very_low();
}
 
HMT_medium();
+   ppc64_runlatch_on();
clear_thread_flag(TIF_POLLING_NRFLAG);
smp_mb();
return index;
@@ -45,7 +48,9 @@ static int nap_loop(struct cpuidle_device *dev,
struct cpuidle_driver *drv,
int index)
 {
+   ppc64_runlatch_off();
power7_idle();
+   ppc64_runlatch_on();
return index;
 }

 
On 02/06/2014 07:46 PM, Nicolas Pitre wrote:
 The core idle loop now takes care of it.
 
 Signed-off-by: Nicolas Pitre n...@linaro.org
 ---
  arch/powerpc/platforms/powernv/setup.c | 13 +
  1 file changed, 1 insertion(+), 12 deletions(-)
 
 diff --git a/arch/powerpc/platforms/powernv/setup.c 
 b/arch/powerpc/platforms/powernv/setup.c
 index 21166f65c9..a932feb290 100644
 --- a/arch/powerpc/platforms/powernv/setup.c
 +++ b/arch/powerpc/platforms/powernv/setup.c
 @@ -26,7 +26,6 @@
  #include linux/of_fdt.h
  #include linux/interrupt.h
  #include linux/bug.h
 -#include linux/cpuidle.h
 
  #include asm/machdep.h
  #include asm/firmware.h
 @@ -217,16 +216,6 @@ static int __init pnv_probe(void)
   return 1;
  }
 
 -void powernv_idle(void)
 -{
 - /* Hook to cpuidle framework if available, else
 -  * call on default platform idle code
 -  */
 - if (cpuidle_idle_call()) {
 - power7_idle();
 - }
 -}
 -
  define_machine(powernv) {
   .name   = PowerNV,
   .probe  = pnv_probe,
 @@ -236,7 +225,7 @@ define_machine(powernv) {
   .show_cpuinfo   = pnv_show_cpuinfo,
   .progress   = pnv_progress,
   .machine_shutdown   = pnv_shutdown,
 - .power_save = powernv_idle,
 + .power_save = power7_idle,
   .calibrate_decr = generic_calibrate_decr,
  #ifdef CONFIG_KEXEC
   .kexec_cpu_down = pnv_kexec_cpu_down,
 

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

Re: [PATCH 1/2] PPC: powernv: remove redundant cpuidle_idle_call()

2014-02-14 Thread Deepthi Dharwar 
On 02/14/2014 04:47 PM, Preeti U Murthy wrote:
 Hi Nicolas,
 
 You will have to include the below patch with yours. You
 could squash the two I guess, I have added the changelog
 just for clarity. And you also might want to change the subject to
 cpuidle/powernv. It gives a better picture.
 
 Thanks
 
 Regards
 Preeti U Murthy
 
 
 cpuidle/powernv: Add ppc64_runlatch_off/on() to idle routines
 
 Following moving of cpuidle_idle_call() to the generic idle loop, we need to
 add the runlatch functions to the idle routines on powernv which was earlier
 taken care of by the arch specific idle routine.
 
 Signed-off-by: Preeti U Murthy pre...@linux.vnet.ibm.com

Reviewed-by: Deepthi Dharwar deep...@linux.vnet.ibm.com

 ---
  drivers/cpuidle/cpuidle-powernv.c |5 +
  1 file changed, 5 insertions(+)
 
 diff --git a/drivers/cpuidle/cpuidle-powernv.c 
 b/drivers/cpuidle/cpuidle-powernv.c
 index 78fd174..f48607c 100644
 --- a/drivers/cpuidle/cpuidle-powernv.c
 +++ b/drivers/cpuidle/cpuidle-powernv.c
 @@ -14,6 +14,7 @@
 
  #include asm/machdep.h
  #include asm/firmware.h
 +#include asm/runlatch.h
 
  struct cpuidle_driver powernv_idle_driver = {
   .name = powernv_idle,
 @@ -30,12 +31,14 @@ static int snooze_loop(struct cpuidle_device *dev,
   local_irq_enable();
   set_thread_flag(TIF_POLLING_NRFLAG);
 
 + ppc64_runlatch_off();
   while (!need_resched()) {
   HMT_low();
   HMT_very_low();
   }
 
   HMT_medium();
 + ppc64_runlatch_on();
   clear_thread_flag(TIF_POLLING_NRFLAG);
   smp_mb();
   return index;
 @@ -45,7 +48,9 @@ static int nap_loop(struct cpuidle_device *dev,
   struct cpuidle_driver *drv,
   int index)
  {
 + ppc64_runlatch_off();
   power7_idle();
 + ppc64_runlatch_on();
   return index;
  }
 
 
 On 02/06/2014 07:46 PM, Nicolas Pitre wrote:
 The core idle loop now takes care of it.

 Signed-off-by: Nicolas Pitre n...@linaro.org
 ---
  arch/powerpc/platforms/powernv/setup.c | 13 +
  1 file changed, 1 insertion(+), 12 deletions(-)

 diff --git a/arch/powerpc/platforms/powernv/setup.c 
 b/arch/powerpc/platforms/powernv/setup.c
 index 21166f65c9..a932feb290 100644
 --- a/arch/powerpc/platforms/powernv/setup.c
 +++ b/arch/powerpc/platforms/powernv/setup.c
 @@ -26,7 +26,6 @@
  #include linux/of_fdt.h
  #include linux/interrupt.h
  #include linux/bug.h
 -#include linux/cpuidle.h

  #include asm/machdep.h
  #include asm/firmware.h
 @@ -217,16 +216,6 @@ static int __init pnv_probe(void)
  return 1;
  }

 -void powernv_idle(void)
 -{
 -/* Hook to cpuidle framework if available, else
 - * call on default platform idle code
 - */
 -if (cpuidle_idle_call()) {
 -power7_idle();
 -}
 -}
 -
  define_machine(powernv) {
  .name   = PowerNV,
  .probe  = pnv_probe,
 @@ -236,7 +225,7 @@ define_machine(powernv) {
  .show_cpuinfo   = pnv_show_cpuinfo,
  .progress   = pnv_progress,
  .machine_shutdown   = pnv_shutdown,
 -.power_save = powernv_idle,
 +.power_save = power7_idle,
  .calibrate_decr = generic_calibrate_decr,
  #ifdef CONFIG_KEXEC
  .kexec_cpu_down = pnv_kexec_cpu_down,

 
 ___
 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

Re: [PATCH 1/2] PPC: powernv: remove redundant cpuidle_idle_call()

2014-02-09 Thread Preeti U Murthy 
Hi Peter,

On 02/07/2014 06:11 PM, Peter Zijlstra wrote:
 On Fri, Feb 07, 2014 at 05:11:26PM +0530, Preeti U Murthy wrote:
 But observe the idle state snooze on powerpc. The power that this idle
 state saves is through the lowering of the thread priority of the CPU.
 After it lowers the thread priority, it is done. It cannot
 wait_for_interrupts. It will exit my_idle(). It is now upto the
 generic idle loop to increase the thread priority if the need_resched
 flag is set. Only an interrupt routine can increase the thread priority.
 Else we will need to do it explicitly. And in such states which have a
 polling nature, the cpu will not receive a reschedule IPI.

 That is why in the snooze_loop() we poll on need_resched. If it is set
 we up the priority of the thread using HMT_MEDIUM() and then exit the
 my_idle() loop. In case of interrupts, the priority gets automatically
 increased.
 
 You can poll without setting TS_POLLING/TIF_POLLING_NRFLAGS just fine
 and get the IPI if that is what you want.
 
 Depending on how horribly unprovisioned the thread gets at the lowest
 priority, that might actually be faster than polling and raising the
 prio whenever it does get ran.

So I am assuming you mean something like the below:

my_idle()
{
   local_irq_enable();
   /* Remove the setting of the polling flag */
   HMT_low();
   return index;
}

And then exit into the generic idle loop. But the issue I see here is
that the TS_POLLING/TIF_POLLING_NRFLAGS gets set immediately. So, if on
testing need_resched() immediately after this returns that the
TIF_NEED_RESCHED flag is set, the thread will exit at low priority right?
 We could raise the priority of the thread in arch_cpu_idle_exit() soon
after setting the polling flag but that would mean for cases where the
TIF_NEED_RESCHED flag is not set we unnecessarily raise the priority of
the thread.

Thanks

Regards
Preeti U Murthy

 

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

Re: [PATCH 1/2] PPC: powernv: remove redundant cpuidle_idle_call()

2014-02-07 Thread Deepthi Dharwar 
Hi Preeti,

Thanks for the patch.

On 02/07/2014 12:31 PM, Preeti U Murthy wrote:
 Hi Nicolas,
 
 Find below the patch that will need to be squashed with this one.
 This patch is based on the mainline.Adding Deepthi, the author of
 the patch which introduced the powernv cpuidle driver. Deepthi,
 do you think the below patch looks right? We do not need to do an
 explicit local_irq_enable() since we are in the call path of
 cpuidle driver and that explicitly enables irqs on exit from
 idle states.

Yes, We enable irqs explicitly while entering snooze loop and we always
have interrupts enabled in the snooze state.
For NAP state, we exit out of this state with interrupts enabled so we
do not need an explicit enable of irqs.

 On 02/07/2014 06:47 AM, Nicolas Pitre wrote:
 On Thu, 6 Feb 2014, Preeti U Murthy wrote:

 Hi Daniel,

 On 02/06/2014 09:55 PM, Daniel Lezcano wrote:
 Hi Nico,


 On 6 February 2014 14:16, Nicolas Pitre nicolas.pi...@linaro.org wrote:

 The core idle loop now takes care of it.

 Signed-off-by: Nicolas Pitre n...@linaro.org
 ---
  arch/powerpc/platforms/powernv/setup.c | 13 +
  1 file changed, 1 insertion(+), 12 deletions(-)

 diff --git a/arch/powerpc/platforms/powernv/setup.c
 b/arch/powerpc/platforms/powernv/setup.c
 index 21166f65c9..a932feb290 100644
 --- a/arch/powerpc/platforms/powernv/setup.c
 +++ b/arch/powerpc/platforms/powernv/setup.c
 @@ -26,7 +26,6 @@
  #include linux/of_fdt.h
  #include linux/interrupt.h
  #include linux/bug.h
 -#include linux/cpuidle.h

  #include asm/machdep.h
  #include asm/firmware.h
 @@ -217,16 +216,6 @@ static int __init pnv_probe(void)
 return 1;
  }

 -void powernv_idle(void)
 -{
 -   /* Hook to cpuidle framework if available, else
 -* call on default platform idle code
 -*/
 -   if (cpuidle_idle_call()) {
 -   power7_idle();
 -   }

 
  drivers/cpuidle/cpuidle-powernv.c |4 
  1 file changed, 4 insertions(+)
 
 diff --git a/drivers/cpuidle/cpuidle-powernv.c 
 b/drivers/cpuidle/cpuidle-powernv.c
 index 78fd174..130f081 100644
 --- a/drivers/cpuidle/cpuidle-powernv.c
 +++ b/drivers/cpuidle/cpuidle-powernv.c
 @@ -31,11 +31,13 @@ static int snooze_loop(struct cpuidle_device *dev,
   set_thread_flag(TIF_POLLING_NRFLAG);
 
   while (!need_resched()) {
 + ppc64_runlatch_off();
^^^
We could move this before the while() loop.
It would ideal to turn off latch when we enter snooze and
turn it on when we are about to exit, rather than doing
it over and over in the while loop.

   HMT_low();
   HMT_very_low();
   }
 
   HMT_medium();
 + ppc64_runlatch_on();
   clear_thread_flag(TIF_POLLING_NRFLAG);
   smp_mb();
   return index;
 @@ -45,7 +47,9 @@ static int nap_loop(struct cpuidle_device *dev,
   struct cpuidle_driver *drv,
   int index)
  {
 + ppc64_runlatch_off();
   power7_idle();
 + ppc64_runlatch_on();
   return index;
  }
 
 Thanks
 
 Regards
 Preeti U Murthy
 

Regards,
Deepthi

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

Re: [PATCH 1/2] PPC: powernv: remove redundant cpuidle_idle_call()

2014-02-07 Thread Nicolas Pitre 
On Fri, 7 Feb 2014, Preeti U Murthy wrote:

 Hi Nicolas,
 
 On 02/07/2014 06:47 AM, Nicolas Pitre wrote:
  
  What about creating arch_cpu_idle_enter() and arch_cpu_idle_exit() in 
  arch/powerpc/kernel/idle.c and calling ppc64_runlatch_off() and 
  ppc64_runlatch_on() respectively from there instead?  Would that work?  
  That would make the idle consolidation much easier afterwards.
 
 I would not suggest doing this. The ppc64_runlatch_*() routines need to
 be called when we are sure that the cpu is about to enter or has exit an
 idle state. Moving the ppc64_runlatch_on() routine to
 arch_cpu_idle_enter() for instance is not a good idea because there are
 places where the cpu can decide not to enter any idle state before the
 call to cpuidle_idle_call() itself. In that case communicating
 prematurely that we are in an idle state would not be a good idea.
 
 So its best to add the ppc64_runlatch_* calls in the powernv cpuidle
 driver IMO. We could however create idle_loop_prologue/epilogue()
 variants inside it so that in addition to the runlatch routines we could
 potentially add more such similar routines that are powernv specific.
   If there are cases where there is work to be done prior to and post an
 entry into an idle state common to both pseries and powernv, we will
 probably put them in arch_cpu_idle_enter/exit(). But the runlatch
 routines are not suitable to be moved there as far as I can see.

OK.

However, one thing we need to do as much as possible is to remove those 
loops based on need_resched() from idle backend drivers.  A somewhat 
common pattern is:

my_idle()
{
/* interrupts disabled on entry */
while (!need_resched()) {
lowpower_wait_for_interrupts();
local_irq_enable();
/* IRQ serviced from here */
local_irq_disable();
}
local_irq_enable();
/* interrupts enabled on exit */
}

To be able to keep statistics on the actual idleness of the CPU we'd 
need for all idle backends to always return to generic code on every 
interrupt similar to this:

my_idle()
{
/* interrupts disabled on entry */
lowpower_wait_for_interrupts();
local_irq_enable();
/* interrupts enabled on exit */
}

The generic code would be responsible for dealing with need_resched() 
and call back into the backend right away if necessary after updating 
some stats.

Do you see a problem with the runlatch calls happening around each 
interrrupt from such a simplified idle backend?


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

Re: [PATCH 1/2] PPC: powernv: remove redundant cpuidle_idle_call()

2014-02-07 Thread Preeti U Murthy 
Hi Deepthi,

On 02/07/2014 03:15 PM, Deepthi Dharwar wrote:
 Hi Preeti,
 
 Thanks for the patch.
 
 On 02/07/2014 12:31 PM, Preeti U Murthy wrote:
 Hi Nicolas,

 Find below the patch that will need to be squashed with this one.
 This patch is based on the mainline.Adding Deepthi, the author of
 the patch which introduced the powernv cpuidle driver. Deepthi,
 do you think the below patch looks right? We do not need to do an
 explicit local_irq_enable() since we are in the call path of
 cpuidle driver and that explicitly enables irqs on exit from
 idle states.
 
 Yes, We enable irqs explicitly while entering snooze loop and we always
 have interrupts enabled in the snooze state.
 For NAP state, we exit out of this state with interrupts enabled so we
 do not need an explicit enable of irqs.
 
 On 02/07/2014 06:47 AM, Nicolas Pitre wrote:
 On Thu, 6 Feb 2014, Preeti U Murthy wrote:

 Hi Daniel,

 On 02/06/2014 09:55 PM, Daniel Lezcano wrote:
 Hi Nico,


 On 6 February 2014 14:16, Nicolas Pitre nicolas.pi...@linaro.org wrote:

 The core idle loop now takes care of it.

 Signed-off-by: Nicolas Pitre n...@linaro.org
 ---
  arch/powerpc/platforms/powernv/setup.c | 13 +
  1 file changed, 1 insertion(+), 12 deletions(-)

 diff --git a/arch/powerpc/platforms/powernv/setup.c
 b/arch/powerpc/platforms/powernv/setup.c
 index 21166f65c9..a932feb290 100644
 --- a/arch/powerpc/platforms/powernv/setup.c
 +++ b/arch/powerpc/platforms/powernv/setup.c
 @@ -26,7 +26,6 @@
  #include linux/of_fdt.h
  #include linux/interrupt.h
  #include linux/bug.h
 -#include linux/cpuidle.h

  #include asm/machdep.h
  #include asm/firmware.h
 @@ -217,16 +216,6 @@ static int __init pnv_probe(void)
 return 1;
  }

 -void powernv_idle(void)
 -{
 -   /* Hook to cpuidle framework if available, else
 -* call on default platform idle code
 -*/
 -   if (cpuidle_idle_call()) {
 -   power7_idle();
 -   }


  drivers/cpuidle/cpuidle-powernv.c |4 
  1 file changed, 4 insertions(+)

 diff --git a/drivers/cpuidle/cpuidle-powernv.c 
 b/drivers/cpuidle/cpuidle-powernv.c
 index 78fd174..130f081 100644
 --- a/drivers/cpuidle/cpuidle-powernv.c
 +++ b/drivers/cpuidle/cpuidle-powernv.c
 @@ -31,11 +31,13 @@ static int snooze_loop(struct cpuidle_device *dev,
  set_thread_flag(TIF_POLLING_NRFLAG);

  while (!need_resched()) {
 +ppc64_runlatch_off();
 ^^^
 We could move this before the while() loop.
 It would ideal to turn off latch when we enter snooze and
 turn it on when we are about to exit, rather than doing
 it over and over in the while loop.

You are right, this can be moved out of the loop.

Thanks

Regards
Preeti U Murthy

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

Re: [PATCH 1/2] PPC: powernv: remove redundant cpuidle_idle_call()

2014-02-07 Thread Preeti U Murthy 
Hi Nicolas,

On 02/07/2014 04:18 PM, Nicolas Pitre wrote:
 On Fri, 7 Feb 2014, Preeti U Murthy wrote:
 
 Hi Nicolas,

 On 02/07/2014 06:47 AM, Nicolas Pitre wrote:

 What about creating arch_cpu_idle_enter() and arch_cpu_idle_exit() in 
 arch/powerpc/kernel/idle.c and calling ppc64_runlatch_off() and 
 ppc64_runlatch_on() respectively from there instead?  Would that work?  
 That would make the idle consolidation much easier afterwards.

 I would not suggest doing this. The ppc64_runlatch_*() routines need to
 be called when we are sure that the cpu is about to enter or has exit an
 idle state. Moving the ppc64_runlatch_on() routine to
 arch_cpu_idle_enter() for instance is not a good idea because there are
 places where the cpu can decide not to enter any idle state before the
 call to cpuidle_idle_call() itself. In that case communicating
 prematurely that we are in an idle state would not be a good idea.

 So its best to add the ppc64_runlatch_* calls in the powernv cpuidle
 driver IMO. We could however create idle_loop_prologue/epilogue()
 variants inside it so that in addition to the runlatch routines we could
 potentially add more such similar routines that are powernv specific.
   If there are cases where there is work to be done prior to and post an
 entry into an idle state common to both pseries and powernv, we will
 probably put them in arch_cpu_idle_enter/exit(). But the runlatch
 routines are not suitable to be moved there as far as I can see.
 
 OK.
 
 However, one thing we need to do as much as possible is to remove those 
 loops based on need_resched() from idle backend drivers.  A somewhat 
 common pattern is:
 
 my_idle()
 {
   /* interrupts disabled on entry */
   while (!need_resched()) {
   lowpower_wait_for_interrupts();
   local_irq_enable();
   /* IRQ serviced from here */
   local_irq_disable();
   }
   local_irq_enable();
   /* interrupts enabled on exit */
 }
 
 To be able to keep statistics on the actual idleness of the CPU we'd 
 need for all idle backends to always return to generic code on every 
 interrupt similar to this:
 
 my_idle()
 {
   /* interrupts disabled on entry */
   lowpower_wait_for_interrupts();

You can do this for the idle states which do not have the polling
nature. IOW, these idle states are capable of doing what you describe as
wait_for_interrupts. They do some kind of spinning at the hardware
level with interrupts enabled. A reschedule IPI or any other interrupt
will wake them up to enter the generic idle loop where they check for
the cause of the interrupt.

But observe the idle state snooze on powerpc. The power that this idle
state saves is through the lowering of the thread priority of the CPU.
After it lowers the thread priority, it is done. It cannot
wait_for_interrupts. It will exit my_idle(). It is now upto the
generic idle loop to increase the thread priority if the need_resched
flag is set. Only an interrupt routine can increase the thread priority.
Else we will need to do it explicitly. And in such states which have a
polling nature, the cpu will not receive a reschedule IPI.

That is why in the snooze_loop() we poll on need_resched. If it is set
we up the priority of the thread using HMT_MEDIUM() and then exit the
my_idle() loop. In case of interrupts, the priority gets automatically
increased.

This might not be required to be done for similar idle routines on other
archs but this is the consequence of applying this idea of simplified
cpuidle backend driver on powerpc.

I would say you could let the backend cpuidle drivers be in this regard,
it could complicate the generic idle loop IMO depending on how the
polling states are implemented in each architecture.


 The generic code would be responsible for dealing with need_resched() 
 and call back into the backend right away if necessary after updating 
 some stats.
 
 Do you see a problem with the runlatch calls happening around each 
 interrrupt from such a simplified idle backend?

The runlatch calls could be moved outside the loop.They do not need to
be called each time.

Thanks

Regards
Preeti U Murthy
 
 
 Nicolas
 

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

Re: [PATCH 1/2] PPC: powernv: remove redundant cpuidle_idle_call()

2014-02-07 Thread Peter Zijlstra 
On Fri, Feb 07, 2014 at 05:11:26PM +0530, Preeti U Murthy wrote:
 But observe the idle state snooze on powerpc. The power that this idle
 state saves is through the lowering of the thread priority of the CPU.
 After it lowers the thread priority, it is done. It cannot
 wait_for_interrupts. It will exit my_idle(). It is now upto the
 generic idle loop to increase the thread priority if the need_resched
 flag is set. Only an interrupt routine can increase the thread priority.
 Else we will need to do it explicitly. And in such states which have a
 polling nature, the cpu will not receive a reschedule IPI.
 
 That is why in the snooze_loop() we poll on need_resched. If it is set
 we up the priority of the thread using HMT_MEDIUM() and then exit the
 my_idle() loop. In case of interrupts, the priority gets automatically
 increased.

You can poll without setting TS_POLLING/TIF_POLLING_NRFLAGS just fine
and get the IPI if that is what you want.

Depending on how horribly unprovisioned the thread gets at the lowest
priority, that might actually be faster than polling and raising the
prio whenever it does get ran.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 1/2] PPC: powernv: remove redundant cpuidle_idle_call()

2014-02-06 Thread Thomas Gleixner 
An Thu, 6 Feb 2014, Nicolas Pitre wrote:

 The core idle loop now takes care of it.
 
 Signed-off-by: Nicolas Pitre n...@linaro.org

Acked-by: Thomas Gleixner t...@linutronix.de
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 1/2] PPC: powernv: remove redundant cpuidle_idle_call()

2014-02-06 Thread Preeti U Murthy 
On 02/06/2014 07:46 PM, Nicolas Pitre wrote:
 The core idle loop now takes care of it.
 
 Signed-off-by: Nicolas Pitre n...@linaro.org
 ---
  arch/powerpc/platforms/powernv/setup.c | 13 +
  1 file changed, 1 insertion(+), 12 deletions(-)
 
 diff --git a/arch/powerpc/platforms/powernv/setup.c 
 b/arch/powerpc/platforms/powernv/setup.c
 index 21166f65c9..a932feb290 100644
 --- a/arch/powerpc/platforms/powernv/setup.c
 +++ b/arch/powerpc/platforms/powernv/setup.c
 @@ -26,7 +26,6 @@
  #include linux/of_fdt.h
  #include linux/interrupt.h
  #include linux/bug.h
 -#include linux/cpuidle.h
 
  #include asm/machdep.h
  #include asm/firmware.h
 @@ -217,16 +216,6 @@ static int __init pnv_probe(void)
   return 1;
  }
 
 -void powernv_idle(void)
 -{
 - /* Hook to cpuidle framework if available, else
 -  * call on default platform idle code
 -  */
 - if (cpuidle_idle_call()) {
 - power7_idle();
 - }
 -}
 -
  define_machine(powernv) {
   .name   = PowerNV,
   .probe  = pnv_probe,
 @@ -236,7 +225,7 @@ define_machine(powernv) {
   .show_cpuinfo   = pnv_show_cpuinfo,
   .progress   = pnv_progress,
   .machine_shutdown   = pnv_shutdown,
 - .power_save = powernv_idle,
 + .power_save = power7_idle,
   .calibrate_decr = generic_calibrate_decr,
  #ifdef CONFIG_KEXEC
   .kexec_cpu_down = pnv_kexec_cpu_down,
 

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

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

Re: [PATCH 1/2] PPC: powernv: remove redundant cpuidle_idle_call()

2014-02-06 Thread Preeti U Murthy 
Hi Daniel,

On 02/06/2014 09:55 PM, Daniel Lezcano wrote:
 Hi Nico,
 
 
 On 6 February 2014 14:16, Nicolas Pitre nicolas.pi...@linaro.org wrote:
 
 The core idle loop now takes care of it.

 Signed-off-by: Nicolas Pitre n...@linaro.org
 ---
  arch/powerpc/platforms/powernv/setup.c | 13 +
  1 file changed, 1 insertion(+), 12 deletions(-)

 diff --git a/arch/powerpc/platforms/powernv/setup.c
 b/arch/powerpc/platforms/powernv/setup.c
 index 21166f65c9..a932feb290 100644
 --- a/arch/powerpc/platforms/powernv/setup.c
 +++ b/arch/powerpc/platforms/powernv/setup.c
 @@ -26,7 +26,6 @@
  #include linux/of_fdt.h
  #include linux/interrupt.h
  #include linux/bug.h
 -#include linux/cpuidle.h

  #include asm/machdep.h
  #include asm/firmware.h
 @@ -217,16 +216,6 @@ static int __init pnv_probe(void)
 return 1;
  }

 -void powernv_idle(void)
 -{
 -   /* Hook to cpuidle framework if available, else
 -* call on default platform idle code
 -*/
 -   if (cpuidle_idle_call()) {
 -   power7_idle();
 -   }

 
 The cpuidle_idle_call is called from arch_cpu_idle in
 arch/powerpc/kernel/idle.c between a ppc64_runlatch_off|on section.
 Shouldn't the cpuidle-powernv driver call these functions when entering
 idle ?

Yes they should, I will send out a patch that does that ontop of this.
There have been cpuidle driver cleanups for powernv and pseries in this
merge window. While no change would be required in the pseries cpuidle
driver as a result of Nicolas's cleanup, we would need to add the
ppc64_runlatch_on and off functions before and after the entry into the
powernv idle states.

Thanks

Regards
Preeti U Murthy
 
   -- Daniel
 
 
 -}
 -
  define_machine(powernv) {
 .name   = PowerNV,
 .probe  = pnv_probe,
 @@ -236,7 +225,7 @@ define_machine(powernv) {
 .show_cpuinfo   = pnv_show_cpuinfo,
 .progress   = pnv_progress,
 .machine_shutdown   = pnv_shutdown,
 -   .power_save = powernv_idle,
 +   .power_save = power7_idle,
 .calibrate_decr = generic_calibrate_decr,
  #ifdef CONFIG_KEXEC
 .kexec_cpu_down = pnv_kexec_cpu_down,
 --
 1.8.4.108.g55ea5f6


 

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

Re: [PATCH 1/2] PPC: powernv: remove redundant cpuidle_idle_call()

2014-02-06 Thread Daniel Lezcano 
Hi Nico,


On 6 February 2014 14:16, Nicolas Pitre nicolas.pi...@linaro.org wrote:

 The core idle loop now takes care of it.

 Signed-off-by: Nicolas Pitre n...@linaro.org
 ---
  arch/powerpc/platforms/powernv/setup.c | 13 +
  1 file changed, 1 insertion(+), 12 deletions(-)

 diff --git a/arch/powerpc/platforms/powernv/setup.c
 b/arch/powerpc/platforms/powernv/setup.c
 index 21166f65c9..a932feb290 100644
 --- a/arch/powerpc/platforms/powernv/setup.c
 +++ b/arch/powerpc/platforms/powernv/setup.c
 @@ -26,7 +26,6 @@
  #include linux/of_fdt.h
  #include linux/interrupt.h
  #include linux/bug.h
 -#include linux/cpuidle.h

  #include asm/machdep.h
  #include asm/firmware.h
 @@ -217,16 +216,6 @@ static int __init pnv_probe(void)
 return 1;
  }

 -void powernv_idle(void)
 -{
 -   /* Hook to cpuidle framework if available, else
 -* call on default platform idle code
 -*/
 -   if (cpuidle_idle_call()) {
 -   power7_idle();
 -   }


The cpuidle_idle_call is called from arch_cpu_idle in
arch/powerpc/kernel/idle.c between a ppc64_runlatch_off|on section.
Shouldn't the cpuidle-powernv driver call these functions when entering
idle ?

  -- Daniel


 -}
 -
  define_machine(powernv) {
 .name   = PowerNV,
 .probe  = pnv_probe,
 @@ -236,7 +225,7 @@ define_machine(powernv) {
 .show_cpuinfo   = pnv_show_cpuinfo,
 .progress   = pnv_progress,
 .machine_shutdown   = pnv_shutdown,
 -   .power_save = powernv_idle,
 +   .power_save = power7_idle,
 .calibrate_decr = generic_calibrate_decr,
  #ifdef CONFIG_KEXEC
 .kexec_cpu_down = pnv_kexec_cpu_down,
 --
 1.8.4.108.g55ea5f6


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

Re: [PATCH 1/2] PPC: powernv: remove redundant cpuidle_idle_call()

2014-02-06 Thread Nicolas Pitre 
On Thu, 6 Feb 2014, Preeti U Murthy wrote:

 Hi Daniel,
 
 On 02/06/2014 09:55 PM, Daniel Lezcano wrote:
  Hi Nico,
  
  
  On 6 February 2014 14:16, Nicolas Pitre nicolas.pi...@linaro.org wrote:
  
  The core idle loop now takes care of it.
 
  Signed-off-by: Nicolas Pitre n...@linaro.org
  ---
   arch/powerpc/platforms/powernv/setup.c | 13 +
   1 file changed, 1 insertion(+), 12 deletions(-)
 
  diff --git a/arch/powerpc/platforms/powernv/setup.c
  b/arch/powerpc/platforms/powernv/setup.c
  index 21166f65c9..a932feb290 100644
  --- a/arch/powerpc/platforms/powernv/setup.c
  +++ b/arch/powerpc/platforms/powernv/setup.c
  @@ -26,7 +26,6 @@
   #include linux/of_fdt.h
   #include linux/interrupt.h
   #include linux/bug.h
  -#include linux/cpuidle.h
 
   #include asm/machdep.h
   #include asm/firmware.h
  @@ -217,16 +216,6 @@ static int __init pnv_probe(void)
  return 1;
   }
 
  -void powernv_idle(void)
  -{
  -   /* Hook to cpuidle framework if available, else
  -* call on default platform idle code
  -*/
  -   if (cpuidle_idle_call()) {
  -   power7_idle();
  -   }
 
  
  The cpuidle_idle_call is called from arch_cpu_idle in
  arch/powerpc/kernel/idle.c between a ppc64_runlatch_off|on section.
  Shouldn't the cpuidle-powernv driver call these functions when entering
  idle ?
 
 Yes they should, I will send out a patch that does that ontop of this.
 There have been cpuidle driver cleanups for powernv and pseries in this
 merge window. While no change would be required in the pseries cpuidle
 driver as a result of Nicolas's cleanup, we would need to add the
 ppc64_runlatch_on and off functions before and after the entry into the
 powernv idle states.

What about creating arch_cpu_idle_enter() and arch_cpu_idle_exit() in 
arch/powerpc/kernel/idle.c and calling ppc64_runlatch_off() and 
ppc64_runlatch_on() respectively from there instead?  Would that work?  
That would make the idle consolidation much easier afterwards.


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

Re: [PATCH 1/2] PPC: powernv: remove redundant cpuidle_idle_call()

2014-02-06 Thread Preeti U Murthy 
Hi Nicolas,

On 02/07/2014 06:47 AM, Nicolas Pitre wrote:
 On Thu, 6 Feb 2014, Preeti U Murthy wrote:
 
 Hi Daniel,

 On 02/06/2014 09:55 PM, Daniel Lezcano wrote:
 Hi Nico,


 On 6 February 2014 14:16, Nicolas Pitre nicolas.pi...@linaro.org wrote:

 The core idle loop now takes care of it.

 Signed-off-by: Nicolas Pitre n...@linaro.org
 ---
  arch/powerpc/platforms/powernv/setup.c | 13 +
  1 file changed, 1 insertion(+), 12 deletions(-)

 diff --git a/arch/powerpc/platforms/powernv/setup.c
 b/arch/powerpc/platforms/powernv/setup.c
 index 21166f65c9..a932feb290 100644
 --- a/arch/powerpc/platforms/powernv/setup.c
 +++ b/arch/powerpc/platforms/powernv/setup.c
 @@ -26,7 +26,6 @@
  #include linux/of_fdt.h
  #include linux/interrupt.h
  #include linux/bug.h
 -#include linux/cpuidle.h

  #include asm/machdep.h
  #include asm/firmware.h
 @@ -217,16 +216,6 @@ static int __init pnv_probe(void)
 return 1;
  }

 -void powernv_idle(void)
 -{
 -   /* Hook to cpuidle framework if available, else
 -* call on default platform idle code
 -*/
 -   if (cpuidle_idle_call()) {
 -   power7_idle();
 -   }


 The cpuidle_idle_call is called from arch_cpu_idle in
 arch/powerpc/kernel/idle.c between a ppc64_runlatch_off|on section.
 Shouldn't the cpuidle-powernv driver call these functions when entering
 idle ?

 Yes they should, I will send out a patch that does that ontop of this.
 There have been cpuidle driver cleanups for powernv and pseries in this
 merge window. While no change would be required in the pseries cpuidle
 driver as a result of Nicolas's cleanup, we would need to add the
 ppc64_runlatch_on and off functions before and after the entry into the
 powernv idle states.
 
 What about creating arch_cpu_idle_enter() and arch_cpu_idle_exit() in 
 arch/powerpc/kernel/idle.c and calling ppc64_runlatch_off() and 
 ppc64_runlatch_on() respectively from there instead?  Would that work?  
 That would make the idle consolidation much easier afterwards.

I would not suggest doing this. The ppc64_runlatch_*() routines need to
be called when we are sure that the cpu is about to enter or has exit an
idle state. Moving the ppc64_runlatch_on() routine to
arch_cpu_idle_enter() for instance is not a good idea because there are
places where the cpu can decide not to enter any idle state before the
call to cpuidle_idle_call() itself. In that case communicating
prematurely that we are in an idle state would not be a good idea.

So its best to add the ppc64_runlatch_* calls in the powernv cpuidle
driver IMO. We could however create idle_loop_prologue/epilogue()
variants inside it so that in addition to the runlatch routines we could
potentially add more such similar routines that are powernv specific.
  If there are cases where there is work to be done prior to and post an
entry into an idle state common to both pseries and powernv, we will
probably put them in arch_cpu_idle_enter/exit(). But the runlatch
routines are not suitable to be moved there as far as I can see.

Thank you

Regards
Preeti U Murthy
 
 
 Nicolas
 ___
 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

Re: [PATCH 1/2] PPC: powernv: remove redundant cpuidle_idle_call()

2014-02-06 Thread Preeti U Murthy 
Hi Nicolas,

Find below the patch that will need to be squashed with this one.
This patch is based on the mainline.Adding Deepthi, the author of
the patch which introduced the powernv cpuidle driver. Deepthi,
do you think the below patch looks right? We do not need to do an
explicit local_irq_enable() since we are in the call path of
cpuidle driver and that explicitly enables irqs on exit from
idle states.

On 02/07/2014 06:47 AM, Nicolas Pitre wrote:
 On Thu, 6 Feb 2014, Preeti U Murthy wrote:
 
 Hi Daniel,

 On 02/06/2014 09:55 PM, Daniel Lezcano wrote:
 Hi Nico,


 On 6 February 2014 14:16, Nicolas Pitre nicolas.pi...@linaro.org wrote:

 The core idle loop now takes care of it.

 Signed-off-by: Nicolas Pitre n...@linaro.org
 ---
  arch/powerpc/platforms/powernv/setup.c | 13 +
  1 file changed, 1 insertion(+), 12 deletions(-)

 diff --git a/arch/powerpc/platforms/powernv/setup.c
 b/arch/powerpc/platforms/powernv/setup.c
 index 21166f65c9..a932feb290 100644
 --- a/arch/powerpc/platforms/powernv/setup.c
 +++ b/arch/powerpc/platforms/powernv/setup.c
 @@ -26,7 +26,6 @@
  #include linux/of_fdt.h
  #include linux/interrupt.h
  #include linux/bug.h
 -#include linux/cpuidle.h

  #include asm/machdep.h
  #include asm/firmware.h
 @@ -217,16 +216,6 @@ static int __init pnv_probe(void)
 return 1;
  }

 -void powernv_idle(void)
 -{
 -   /* Hook to cpuidle framework if available, else
 -* call on default platform idle code
 -*/
 -   if (cpuidle_idle_call()) {
 -   power7_idle();
 -   }


 drivers/cpuidle/cpuidle-powernv.c |4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/cpuidle/cpuidle-powernv.c 
b/drivers/cpuidle/cpuidle-powernv.c
index 78fd174..130f081 100644
--- a/drivers/cpuidle/cpuidle-powernv.c
+++ b/drivers/cpuidle/cpuidle-powernv.c
@@ -31,11 +31,13 @@ static int snooze_loop(struct cpuidle_device *dev,
set_thread_flag(TIF_POLLING_NRFLAG);
 
while (!need_resched()) {
+   ppc64_runlatch_off();
HMT_low();
HMT_very_low();
}
 
HMT_medium();
+   ppc64_runlatch_on();
clear_thread_flag(TIF_POLLING_NRFLAG);
smp_mb();
return index;
@@ -45,7 +47,9 @@ static int nap_loop(struct cpuidle_device *dev,
struct cpuidle_driver *drv,
int index)
 {
+   ppc64_runlatch_off();
power7_idle();
+   ppc64_runlatch_on();
return index;
 }
 
Thanks

Regards
Preeti U Murthy

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