Ingo,
Please pull the timers/urgent-for-tip branch that can be found at:
git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git
timers/urgent-for-tip
Only the 6th patch is new so I'm posting it along, the others have been
posted a few days ago.
Thanks,
Frederic
---
Frederic Weisbecker (4):
vtime: Use consistent clocks among nohz accounting
watchdog: Boot-disable by default on full dynticks
kvm: Move guest entry/exit APIs to context_tracking
nohz: Prevent broadcast source from stealing full dynticks timekeeping duty
Li Zhong (1):
nohz: Fix notifier return val that enforce timekeeping
Steven Rostedt (1):
nohz: Warn if the machine can not perform nohz_full
include/linux/context_tracking.h | 35 +++++++++++++++++++++++++++++++++++
include/linux/kvm_host.h | 37 +------------------------------------
include/linux/vtime.h | 4 ++--
kernel/context_tracking.c | 1 -
kernel/sched/core.c | 2 +-
kernel/sched/cputime.c | 6 +++---
kernel/time/tick-broadcast.c | 11 ++++++++---
kernel/time/tick-sched.c | 7 ++++++-
kernel/watchdog.c | 6 ++++++
9 files changed, 62 insertions(+), 47 deletions(-)
The timekeeping duty is currently assigned to the CPU that
handles the tick broadcast clock device by the time it is set in
one shot mode.
The reason for this is not entirely clear as outlined by Jiri
Bohac: https://patchwork.kernel.org/patch/2302951/
One could speculate though that it makes sure only one CPU
is woken up to fixup the timekeeping max deferment. But the
timekeeper can change anytime after the broadcast CPU becomes
idle. So probably we can remove this as in Jiri's patch, but
not late in the -rc's.
The issue we need to deal with now is that the timekeeping duty
must stay handled by the boot CPU in full dynticks mode for now.
Otherwise it prevents secondary CPUs from offlining and this breaks
suspend/shutdown/reboot/...
Long term plan is to make the timekeeper dynamic in full dynticks
as well but for now lets prevent that to happen to avoid breakages.
Reported-by: Steven Rostedt <[email protected]>
Signed-off-by: Frederic Weisbecker <[email protected]>
Cc: Jiri Bohac <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Paul E. McKenney <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Borislav Petkov <[email protected]>
---
kernel/time/tick-broadcast.c | 11 ++++++++---
1 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c
index 24938d5..ed3a253 100644
--- a/kernel/time/tick-broadcast.c
+++ b/kernel/time/tick-broadcast.c
@@ -692,9 +692,14 @@ void tick_broadcast_setup_oneshot(struct clock_event_device *bc)
bc->event_handler = tick_handle_oneshot_broadcast;
- /* Take the do_timer update */
- if (!tick_nohz_full_cpu(cpu))
- tick_do_timer_cpu = cpu;
+ /*
+ * Take the timekeeping duty unless we run in full
+ * dynticks mode that require the boot CPU to stay
+ * the timekeeper for now.
+ */
+#ifndef CONFIG_NO_HZ_FULL
+ tick_do_timer_cpu = cpu;
+#endif
/*
* We must be careful here. There might be other CPUs
--
1.7.5.4
On Wed, 29 May 2013, Frederic Weisbecker wrote:
> The timekeeping duty is currently assigned to the CPU that
> handles the tick broadcast clock device by the time it is set in
> one shot mode.
>
> The reason for this is not entirely clear as outlined by Jiri
> Bohac: https://patchwork.kernel.org/patch/2302951/
>
> One could speculate though that it makes sure only one CPU
> is woken up to fixup the timekeeping max deferment. But the
> timekeeper can change anytime after the broadcast CPU becomes
> idle. So probably we can remove this as in Jiri's patch, but
> not late in the -rc's.
Looking at commit 7300711e (clockevents: broadcast fixup possible
waiters) which introduced that takeover, I really can't see a reason
why we must do that. It's safe to remove it completely even now.
Thanks,
tglx
On Thu, May 30, 2013 at 03:57:17PM +0200, Thomas Gleixner wrote:
> On Wed, 29 May 2013, Frederic Weisbecker wrote:
>
> > The timekeeping duty is currently assigned to the CPU that
> > handles the tick broadcast clock device by the time it is set in
> > one shot mode.
> >
> > The reason for this is not entirely clear as outlined by Jiri
> > Bohac: https://patchwork.kernel.org/patch/2302951/
> >
> > One could speculate though that it makes sure only one CPU
> > is woken up to fixup the timekeeping max deferment. But the
> > timekeeper can change anytime after the broadcast CPU becomes
> > idle. So probably we can remove this as in Jiri's patch, but
> > not late in the -rc's.
>
> Looking at commit 7300711e (clockevents: broadcast fixup possible
> waiters) which introduced that takeover, I really can't see a reason
> why we must do that. It's safe to remove it completely even now.
Yeah it seems so, if you're ok I can commit https://patchwork.kernel.org/patch/2302951/
with your ack and send another pull request.
Thanks.
On Thu, 30 May 2013, Frederic Weisbecker wrote:
> On Thu, May 30, 2013 at 03:57:17PM +0200, Thomas Gleixner wrote:
> > On Wed, 29 May 2013, Frederic Weisbecker wrote:
> >
> > > The timekeeping duty is currently assigned to the CPU that
> > > handles the tick broadcast clock device by the time it is set in
> > > one shot mode.
> > >
> > > The reason for this is not entirely clear as outlined by Jiri
> > > Bohac: https://patchwork.kernel.org/patch/2302951/
> > >
> > > One could speculate though that it makes sure only one CPU
> > > is woken up to fixup the timekeeping max deferment. But the
> > > timekeeper can change anytime after the broadcast CPU becomes
> > > idle. So probably we can remove this as in Jiri's patch, but
> > > not late in the -rc's.
> >
> > Looking at commit 7300711e (clockevents: broadcast fixup possible
> > waiters) which introduced that takeover, I really can't see a reason
> > why we must do that. It's safe to remove it completely even now.
>
> Yeah it seems so, if you're ok I can commit https://patchwork.kernel.org/patch/2302951/
> with your ack and send another pull request.
Yup.
From: Jiri Bohac <[email protected]>
Since 7300711e ("clockevents: broadcast fixup possible waiters"),
the timekeeping duty is assigned to the CPU that handles the tick
broadcast clock device by the time it is set in one shot mode.
This is an issue in full dynticks mode where the timekeeping duty
must stay handled by the boot CPU for now. Otherwise it prevents
secondary CPUs from offlining and this breaks
suspend/shutdown/reboot/...
As it appears there is no reason for this timekeeping duty to be
moved to the broadcast CPU, besides nothing prevent it from being
later re-assigned to another target, let's simply remove it.
Signed-off-by: Jiri Bohac <[email protected]>
Reported-by: Steven Rostedt <[email protected]>
Acked-by: Thomas Gleixner <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Paul E. McKenney <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Borislav Petkov <[email protected]>
Signed-off-by: Frederic Weisbecker <[email protected]>
---
kernel/time/tick-broadcast.c | 4 ----
1 files changed, 0 insertions(+), 4 deletions(-)
diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c
index 24938d5..ee316b9 100644
--- a/kernel/time/tick-broadcast.c
+++ b/kernel/time/tick-broadcast.c
@@ -692,10 +692,6 @@ void tick_broadcast_setup_oneshot(struct clock_event_device *bc)
bc->event_handler = tick_handle_oneshot_broadcast;
- /* Take the do_timer update */
- if (!tick_nohz_full_cpu(cpu))
- tick_do_timer_cpu = cpu;
-
/*
* We must be careful here. There might be other CPUs
* waiting for periodic broadcast. We need to set the
--
1.7.5.4
On Wed, May 29, 2013 at 06:39:39PM +0200, Frederic Weisbecker wrote:
> Ingo,
>
> Please pull the timers/urgent-for-tip branch that can be found at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git
> timers/urgent-for-tip
Please rather pull timers/urgent-for-tip-v2, it removes
"nohz: Prevent broadcast source from stealing full dynticks timekeeping duty"
and includes "tick: Remove useless timekeeping duty attribution to broadcast source"
instead, acked by Thomas.
Thanks.
>
> Only the 6th patch is new so I'm posting it along, the others have been
> posted a few days ago.
>
> Thanks,
> Frederic
> ---
>
> Frederic Weisbecker (4):
> vtime: Use consistent clocks among nohz accounting
> watchdog: Boot-disable by default on full dynticks
> kvm: Move guest entry/exit APIs to context_tracking
> nohz: Prevent broadcast source from stealing full dynticks timekeeping duty
>
> Li Zhong (1):
> nohz: Fix notifier return val that enforce timekeeping
>
> Steven Rostedt (1):
> nohz: Warn if the machine can not perform nohz_full
>
>
> include/linux/context_tracking.h | 35 +++++++++++++++++++++++++++++++++++
> include/linux/kvm_host.h | 37 +------------------------------------
> include/linux/vtime.h | 4 ++--
> kernel/context_tracking.c | 1 -
> kernel/sched/core.c | 2 +-
> kernel/sched/cputime.c | 6 +++---
> kernel/time/tick-broadcast.c | 11 ++++++++---
> kernel/time/tick-sched.c | 7 ++++++-
> kernel/watchdog.c | 6 ++++++
> 9 files changed, 62 insertions(+), 47 deletions(-)
* Frederic Weisbecker <[email protected]> wrote:
> On Wed, May 29, 2013 at 06:39:39PM +0200, Frederic Weisbecker wrote:
> > Ingo,
> >
> > Please pull the timers/urgent-for-tip branch that can be found at:
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git
> > timers/urgent-for-tip
>
> Please rather pull timers/urgent-for-tip-v2, it removes "nohz: Prevent
> broadcast source from stealing full dynticks timekeeping duty" and
> includes "tick: Remove useless timekeeping duty attribution to broadcast
> source" instead, acked by Thomas.
Applied, thanks a lot Frederic!
Ingo