On 09/27/13 03:52, Daniel Lezcano wrote:
> The sleep_length is computed in the tick_nohz_stop_sched_tick function but it
> is used later in the code with in between the local irq enabled.
>
> cpu_idle_loop
> tick_nohz_idle_enter [ exits with local irq enabled ]
> __tick_nohz_idle_enter
> tick_nohz_stop_sched_tick
> ...
>
> arch_cpu_idle
> menu_select [ uses here 'sleep_length' ]
> ...
>
> Between the computation of the sleep length and its usage, some interrupts
> can occur, making the sleep length shorter than actually it is.
>
> This patch fixes that by moving the sleep_length computation in the
> tick_nohz_get_sleep_length function and store the next_event for the device
> instead of the sleep_length.
>
> Signed-off-by: Daniel Lezcano <daniel.lezc...@linaro.org>
> ---
> include/linux/tick.h | 2 +-
> kernel/time/tick-sched.c | 5 +++--
> 2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/tick.h b/include/linux/tick.h
> index 5128d33..4932004 100644
> --- a/include/linux/tick.h
> +++ b/include/linux/tick.h
> @@ -67,7 +67,7 @@ struct tick_sched {
> ktime_t idle_exittime;
> ktime_t idle_sleeptime;
> ktime_t iowait_sleeptime;
> - ktime_t sleep_length;
> + ktime_t next_event;
> unsigned long last_jiffies;
> unsigned long next_jiffies;
> ktime_t idle_expires;
Documentation update?
> diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
> index 3612fc7..2007a7f 100644
> --- a/kernel/time/tick-sched.c
> +++ b/kernel/time/tick-sched.c
> @@ -673,7 +673,7 @@ static ktime_t tick_nohz_stop_sched_tick(struct
> tick_sched *ts,
> out:
> ts->next_jiffies = next_jiffies;
> ts->last_jiffies = last_jiffies;
> - ts->sleep_length = ktime_sub(dev->next_event, now);
> + ts->next_event = dev->next_event;
>
> return ret;
> }
> @@ -837,8 +837,9 @@ void tick_nohz_irq_exit(void)
> ktime_t tick_nohz_get_sleep_length(void)
> {
> struct tick_sched *ts = &__get_cpu_var(tick_cpu_sched);
> + ktime_t now = ktime_get();
>
> - return ts->sleep_length;
> + return ktime_sub(ts->next_event, now);
> }
>
> static void tick_nohz_restart(struct tick_sched *ts, ktime_t now)
What happens if the idling CPU's next_event is updated via that
interrupt? Say if the interrupt handler schedules a timer to fire before
the next timer on the CPU? It looks like we won't notice that.
Perhaps it's better to do this instead?
ktime_t tick_nohz_get_sleep_length(void)
{
struct tick_sched *ts = &__get_cpu_var(tick_cpu_sched);
+ ktime_t now = ktime_get();
+ struct clock_event_device *dev = __get_cpu_var(tick_cpu_device).evtdev;
- return ts->sleep_length;
+ return ktime_sub(dev->next_event, now);
}
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
--
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/
