diff mbox

[V2,14/19] tick-sched: add comment about 'idle_active' in tick_nohz_idle_exit()

Message ID 387f4713d52595f055620f39dc7a0edad7d4d8bf.1398072824.git.viresh.kumar@linaro.org
State New
Headers show

Commit Message

Viresh Kumar April 21, 2014, 9:55 a.m. UTC
The sequence of calls for dynticks CPUs is a bit confusing. Add a comment in
tick_nohz_idle_exit() to mention it clearly. All information required is in
commit and this conversation with Frederic.

https://lkml.org/lkml/2014/4/10/355

Suggested-by: Frederic Weisbecker <fweisbec@gmail.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 kernel/time/tick-sched.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

Comments

Frederic Weisbecker April 21, 2014, 11:20 p.m. UTC | #1
On Mon, Apr 21, 2014 at 03:25:10PM +0530, Viresh Kumar wrote:
> The sequence of calls for dynticks CPUs is a bit confusing. Add a comment in
> tick_nohz_idle_exit() to mention it clearly. All information required is in
> commit and this conversation with Frederic.
> 
> https://lkml.org/lkml/2014/4/10/355
> 
> Suggested-by: Frederic Weisbecker <fweisbec@gmail.com>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  kernel/time/tick-sched.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
> index 71f64ee..c3aed50 100644
> --- a/kernel/time/tick-sched.c
> +++ b/kernel/time/tick-sched.c
> @@ -922,6 +922,22 @@ void tick_nohz_idle_exit(void)
>  
>  	ts->inidle = 0;
>  
> +	/*
> +	 * Can idle_active be false here?
> +	 * Ideally this would be the sequence of calls:
> +	 * - tick_nohz_idle_enter(), i.e. idle_active = true;
> +	 * - local_irq_disable()
> +	 * - IDLE
> +	 * - wake up due to IPI or other interrupt
> +	 * - local_irq_enable()
> +	 * - tick_nohz_irq_enter(), i.e. idle_active = false;
> +	 * - tick_nohz_irq_exit(), i.e. idle_active = true; This is not called
> +	 *   in case of IPI's as need_resched() will prevent that in
> +	 *   tick_irq_exit(), as we don't need to account any more for idle time
> +	 *   or try to enter dyntics mode (We are going to exit idle state).
> +	 *
> +	 * - tick_nohz_idle_exit()
> +	 */
>  	if (ts->idle_active || ts->tick_stopped)
>  		now = ktime_get();

It's still over-detailed. Much of the above is easily deduced after common review. OTOH
I proposed to summarize there: https://lkml.org/lkml/2014/4/11/334
The below disambiguates it a bit further.

Now it's eventually getting as big as your comment ;-)


          /*
           * ts->idle_active drives the idle time which typically elapses in the idle loop
           * but breaks on IRQs interrupting idle loop.
           *
           * Hence ts->idle_active can be 1 here if we exit the idle loop without the help of
           * an IRQ. OTOH it can be 0 on idle exit if a wake up IPI pulled the CPU out of
           * the idle loop. Since we know that we'll be exiting the idle task after the wake
           * up IPI, all the pending idle sleep time is flushed on irq entry and no more is
           * accounted further thanks to the need_resched() check on irq_exit().
           */

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

Patch

diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 71f64ee..c3aed50 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -922,6 +922,22 @@  void tick_nohz_idle_exit(void)
 
 	ts->inidle = 0;
 
+	/*
+	 * Can idle_active be false here?
+	 * Ideally this would be the sequence of calls:
+	 * - tick_nohz_idle_enter(), i.e. idle_active = true;
+	 * - local_irq_disable()
+	 * - IDLE
+	 * - wake up due to IPI or other interrupt
+	 * - local_irq_enable()
+	 * - tick_nohz_irq_enter(), i.e. idle_active = false;
+	 * - tick_nohz_irq_exit(), i.e. idle_active = true; This is not called
+	 *   in case of IPI's as need_resched() will prevent that in
+	 *   tick_irq_exit(), as we don't need to account any more for idle time
+	 *   or try to enter dyntics mode (We are going to exit idle state).
+	 *
+	 * - tick_nohz_idle_exit()
+	 */
 	if (ts->idle_active || ts->tick_stopped)
 		now = ktime_get();