* Re: [PATCH] nohz: Fix possible missing clock reprog after tick soft restart
2017-02-07 16:44 [PATCH] nohz: Fix possible missing clock reprog after tick soft restart Frederic Weisbecker
@ 2017-02-07 20:05 ` Rik van Riel
2017-02-08 1:56 ` Wanpeng Li
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Rik van Riel @ 2017-02-07 20:05 UTC (permalink / raw)
To: Frederic Weisbecker, Thomas Gleixner
Cc: LKML, Peter Zijlstra, Wanpeng Li, stable, Ingo Molnar
On Tue, 2017-02-07 at 17:44 +0100, Frederic Weisbecker wrote:
> ts->next_tick keeps track of the next tick deadline in order to
> optimize
> clock programmation on irq exit and avoid redundant clock device
> writes.
>
> Now if ts->next_tick missed an update, we may spuriously miss a clock
> reprog later as the nohz code is fooled by an obsolete next_tick
> value.
>
> This is what happens here on a specific path: when we observe an
> expired timer from the nohz update code on irq exit, we perform a
> soft
> tick restart which simply fires the closest possible tick without
> actually exiting the nohz mode and restoring a periodic state. But we
> forget to update ts->next_tick accordingly.
>
> As a result, after the next tick resulting from such soft tick
> restart,
> the nohz code sees a stale value on ts->next_tick which doesn't match
> the clock deadline that just expired. If that obsolete ts->next_tick
> value happens to collide with the actual next tick deadline to be
> scheduled, we may spuriously bypass the clock reprogramming. In the
> worst case, the tick may never fire again.
>
> Lets fix this with a ts->next_tick reset on soft tick restart.
>
> Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Wanpeng Li <wanpeng.li@hotmail.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Rik van Riel <riel@redhat.com>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: stable@vger.kernel.org
>
Acked-by: Rik van Riel <riel@redhat.com>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] nohz: Fix possible missing clock reprog after tick soft restart
2017-02-07 16:44 [PATCH] nohz: Fix possible missing clock reprog after tick soft restart Frederic Weisbecker
2017-02-07 20:05 ` Rik van Riel
@ 2017-02-08 1:56 ` Wanpeng Li
2017-02-10 8:45 ` [tip:timers/urgent] tick/nohz: " tip-bot for Frederic Weisbecker
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Wanpeng Li @ 2017-02-08 1:56 UTC (permalink / raw)
To: Frederic Weisbecker
Cc: Thomas Gleixner, LKML, Peter Zijlstra, Wanpeng Li, stable,
Ingo Molnar, Rik van Riel
2017-02-08 0:44 GMT+08:00 Frederic Weisbecker <fweisbec@gmail.com>:
> ts->next_tick keeps track of the next tick deadline in order to optimize
> clock programmation on irq exit and avoid redundant clock device writes.
>
> Now if ts->next_tick missed an update, we may spuriously miss a clock
> reprog later as the nohz code is fooled by an obsolete next_tick value.
>
> This is what happens here on a specific path: when we observe an
> expired timer from the nohz update code on irq exit, we perform a soft
> tick restart which simply fires the closest possible tick without
> actually exiting the nohz mode and restoring a periodic state. But we
> forget to update ts->next_tick accordingly.
>
> As a result, after the next tick resulting from such soft tick restart,
> the nohz code sees a stale value on ts->next_tick which doesn't match
> the clock deadline that just expired. If that obsolete ts->next_tick
> value happens to collide with the actual next tick deadline to be
> scheduled, we may spuriously bypass the clock reprogramming. In the
> worst case, the tick may never fire again.
>
> Lets fix this with a ts->next_tick reset on soft tick restart.
>
> Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Wanpeng Li <wanpeng.li@hotmail.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Rik van Riel <riel@redhat.com>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: stable@vger.kernel.org
Reviewed-by: Wanpeng Li <wanpeng.li@hotmail.com>
> ---
> kernel/time/tick-sched.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
> index 74e0388..fc6f740 100644
> --- a/kernel/time/tick-sched.c
> +++ b/kernel/time/tick-sched.c
> @@ -725,6 +725,11 @@ static ktime_t tick_nohz_stop_sched_tick(struct tick_sched *ts,
> */
> if (delta == 0) {
> tick_nohz_restart(ts, now);
> + /*
> + * Make sure next tick stop doesn't get fooled by past
> + * clock deadline
> + */
> + ts->next_tick = 0;
> goto out;
> }
> }
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 6+ messages in thread* [tip:timers/urgent] tick/nohz: Fix possible missing clock reprog after tick soft restart
2017-02-07 16:44 [PATCH] nohz: Fix possible missing clock reprog after tick soft restart Frederic Weisbecker
2017-02-07 20:05 ` Rik van Riel
2017-02-08 1:56 ` Wanpeng Li
@ 2017-02-10 8:45 ` tip-bot for Frederic Weisbecker
2017-02-17 3:22 ` [PATCH] nohz: " kbuild test robot
2017-02-17 5:34 ` kbuild test robot
4 siblings, 0 replies; 6+ messages in thread
From: tip-bot for Frederic Weisbecker @ 2017-02-10 8:45 UTC (permalink / raw)
To: linux-tip-commits; +Cc: linux-kernel, mingo, fweisbec, hpa, tglx, riel, peterz
Commit-ID: 7bdb59f1ad474bd7161adc8f923cdef10f2638d1
Gitweb: http://git.kernel.org/tip/7bdb59f1ad474bd7161adc8f923cdef10f2638d1
Author: Frederic Weisbecker <fweisbec@gmail.com>
AuthorDate: Tue, 7 Feb 2017 17:44:54 +0100
Committer: Thomas Gleixner <tglx@linutronix.de>
CommitDate: Fri, 10 Feb 2017 09:43:48 +0100
tick/nohz: Fix possible missing clock reprog after tick soft restart
ts->next_tick keeps track of the next tick deadline in order to optimize
clock programmation on irq exit and avoid redundant clock device writes.
Now if ts->next_tick missed an update, we may spuriously miss a clock
reprog later as the nohz code is fooled by an obsolete next_tick value.
This is what happens here on a specific path: when we observe an
expired timer from the nohz update code on irq exit, we perform a soft
tick restart which simply fires the closest possible tick without
actually exiting the nohz mode and restoring a periodic state. But we
forget to update ts->next_tick accordingly.
As a result, after the next tick resulting from such soft tick restart,
the nohz code sees a stale value on ts->next_tick which doesn't match
the clock deadline that just expired. If that obsolete ts->next_tick
value happens to collide with the actual next tick deadline to be
scheduled, we may spuriously bypass the clock reprogramming. In the
worst case, the tick may never fire again.
Fix this with a ts->next_tick reset on soft tick restart.
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Reviewed: Wanpeng Li <wanpeng.li@hotmail.com>
Acked-by: Rik van Riel <riel@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: stable@vger.kernel.org
Link: http://lkml.kernel.org/r/1486485894-29173-1-git-send-email-fweisbec@gmail.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
kernel/time/tick-sched.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 74e0388..fc6f740 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -725,6 +725,11 @@ static ktime_t tick_nohz_stop_sched_tick(struct tick_sched *ts,
*/
if (delta == 0) {
tick_nohz_restart(ts, now);
+ /*
+ * Make sure next tick stop doesn't get fooled by past
+ * clock deadline
+ */
+ ts->next_tick = 0;
goto out;
}
}
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] nohz: Fix possible missing clock reprog after tick soft restart
2017-02-07 16:44 [PATCH] nohz: Fix possible missing clock reprog after tick soft restart Frederic Weisbecker
` (2 preceding siblings ...)
2017-02-10 8:45 ` [tip:timers/urgent] tick/nohz: " tip-bot for Frederic Weisbecker
@ 2017-02-17 3:22 ` kbuild test robot
2017-02-17 5:34 ` kbuild test robot
4 siblings, 0 replies; 6+ messages in thread
From: kbuild test robot @ 2017-02-17 3:22 UTC (permalink / raw)
To: Frederic Weisbecker
Cc: kbuild-all, Thomas Gleixner, LKML, Frederic Weisbecker,
Peter Zijlstra, Wanpeng Li, stable, Ingo Molnar, Rik van Riel
[-- Attachment #1: Type: text/plain, Size: 1230 bytes --]
Hi Frederic,
[auto build test ERROR on tip/timers/core]
[cannot apply to v4.10-rc8 next-20170216]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Frederic-Weisbecker/nohz-Fix-possible-missing-clock-reprog-after-tick-soft-restart/20170208-013019
config: i386-randconfig-c0-02171046 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
kernel/time/tick-sched.c: In function 'tick_nohz_stop_sched_tick':
>> kernel/time/tick-sched.c:732:6: error: 'struct tick_sched' has no member named 'next_tick'
ts->next_tick = 0;
^
vim +732 kernel/time/tick-sched.c
726 if (delta == 0) {
727 tick_nohz_restart(ts, now);
728 /*
729 * Make sure next tick stop doesn't get fooled by past
730 * clock deadline
731 */
> 732 ts->next_tick = 0;
733 goto out;
734 }
735 }
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 20133 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] nohz: Fix possible missing clock reprog after tick soft restart
2017-02-07 16:44 [PATCH] nohz: Fix possible missing clock reprog after tick soft restart Frederic Weisbecker
` (3 preceding siblings ...)
2017-02-17 3:22 ` [PATCH] nohz: " kbuild test robot
@ 2017-02-17 5:34 ` kbuild test robot
4 siblings, 0 replies; 6+ messages in thread
From: kbuild test robot @ 2017-02-17 5:34 UTC (permalink / raw)
To: Frederic Weisbecker
Cc: kbuild-all, Thomas Gleixner, LKML, Frederic Weisbecker,
Peter Zijlstra, Wanpeng Li, stable, Ingo Molnar, Rik van Riel
[-- Attachment #1: Type: text/plain, Size: 1252 bytes --]
Hi Frederic,
[auto build test ERROR on tip/timers/core]
[cannot apply to v4.10-rc8 next-20170216]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Frederic-Weisbecker/nohz-Fix-possible-missing-clock-reprog-after-tick-soft-restart/20170208-013019
config: x86_64-kexec (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
kernel/time/tick-sched.c: In function 'tick_nohz_stop_sched_tick':
>> kernel/time/tick-sched.c:732:6: error: 'struct tick_sched' has no member named 'next_tick'; did you mean 'last_tick'?
ts->next_tick = 0;
^~
vim +732 kernel/time/tick-sched.c
726 if (delta == 0) {
727 tick_nohz_restart(ts, now);
728 /*
729 * Make sure next tick stop doesn't get fooled by past
730 * clock deadline
731 */
> 732 ts->next_tick = 0;
733 goto out;
734 }
735 }
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 24438 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread