make arch code use xtime_update() instead of do_timer()

make arch code use xtime_update() instead of do_timer()

[Torben Hohn]

move do_timer() into kernel/time/timekeeping.c
and provide a version which properly takes the xtime_lock()

finally makes do_timer() and xtime_lock into a header file
private to kernel/time

xtime_lock is still used by kernel/hrtimer.c and kernel/time.c
i am not sure, whether the small header file is the right
thing though.

[PATCH 01/18] move do_timer() from kernel/timer.c into kernel/time/timekeeping.c

[Torben Hohn]

Signed-off-by: Torben Hohn <torbenh@gmx.de>
---
 kernel/time/timekeeping.c |   12 ++++++++++++
 kernel/timer.c            |   13 -------------
 2 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index d27c756..546d82f 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -946,3 +946,15 @@ struct timespec get_monotonic_coarse(void)
 				now.tv_nsec + mono.tv_nsec);
 	return now;
 }
+
+/*
+ * The 64-bit jiffies value is not atomic - you MUST NOT read it
+ * without sampling the sequence number in xtime_lock.
+ * jiffies is defined in the linker script...
+ */
+void do_timer(unsigned long ticks)
+{
+	jiffies_64 += ticks;
+	update_wall_time();
+	calc_global_load(ticks);
+}
diff --git a/kernel/timer.c b/kernel/timer.c
index 43ca993..87f656c 100644
--- a/kernel/timer.c
+++ b/kernel/timer.c
@@ -1293,19 +1293,6 @@ void run_local_timers(void)
 	raise_softirq(TIMER_SOFTIRQ);
 }
 
-/*
- * The 64-bit jiffies value is not atomic - you MUST NOT read it
- * without sampling the sequence number in xtime_lock.
- * jiffies is defined in the linker script...
- */
-
-void do_timer(unsigned long ticks)
-{
-	jiffies_64 += ticks;
-	update_wall_time();
-	calc_global_load(ticks);
-}
-
 #ifdef __ARCH_WANT_SYS_ALARM
 
 /*
-- 
1.7.2.3

Re: [PATCH 01/18] move do_timer() from kernel/timer.c into kernel/time/timekeeping.c

[Thomas Gleixner]

On Sat, 22 Jan 2011, Torben Hohn wrote:

Please add a sensible change log, why you move that code.

> Signed-off-by: Torben Hohn <torbenh@gmx.de>
> ---
>  kernel/time/timekeeping.c |   12 ++++++++++++
>  kernel/timer.c            |   13 -------------
>  2 files changed, 12 insertions(+), 13 deletions(-)
> 
> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> index d27c756..546d82f 100644
> --- a/kernel/time/timekeeping.c
> +++ b/kernel/time/timekeeping.c
> @@ -946,3 +946,15 @@ struct timespec get_monotonic_coarse(void)
>  				now.tv_nsec + mono.tv_nsec);
>  	return now;
>  }
> +
> +/*
> + * The 64-bit jiffies value is not atomic - you MUST NOT read it
> + * without sampling the sequence number in xtime_lock.
> + * jiffies is defined in the linker script...
> + */
> +void do_timer(unsigned long ticks)
> +{
> +	jiffies_64 += ticks;
> +	update_wall_time();
> +	calc_global_load(ticks);
> +}

That also should make update_wall_time() static.

Thanks,

	tglx

Re: [PATCH 01/18] move do_timer() from kernel/timer.c into kernel/time/timekeeping.c

[john stultz]

On Sat, 2011-01-22 at 00:06 +0100, Torben Hohn wrote:
> Signed-off-by: Torben Hohn <torbenh@gmx.de>
> ---
>  kernel/time/timekeeping.c |   12 ++++++++++++
>  kernel/timer.c            |   13 -------------
>  2 files changed, 12 insertions(+), 13 deletions(-)
> 
> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> index d27c756..546d82f 100644
> --- a/kernel/time/timekeeping.c
> +++ b/kernel/time/timekeeping.c
> @@ -946,3 +946,15 @@ struct timespec get_monotonic_coarse(void)
>  				now.tv_nsec + mono.tv_nsec);
>  	return now;
>  }
> +
> +/*
> + * The 64-bit jiffies value is not atomic - you MUST NOT read it
> + * without sampling the sequence number in xtime_lock.
> + * jiffies is defined in the linker script...
> + */
> +void do_timer(unsigned long ticks)
> +{
> +	jiffies_64 += ticks;
> +	update_wall_time();
> +	calc_global_load(ticks);
> +}


I know Thomas suggested this move, but I'm not sure I agree (yet).
Jiffies updates, and load calculations really have much more to do with
the timer irq then with timekeeping, so I'd be prone to leave them in
place. Or maybe move them to the tick scheduling code?

I'm guessing Thomas is thinking to move these bits into timekeeping.c so
xtime_lock can be made static there, it just strikes me oddly.
Especially since jiffies access is still going to need the xtime_lock,
so we'd have to move all the jiffies code into timekeeping.c to do so.

Splitting the xtime_lock int a static timekeeper.lock and a static
jiffies_lock might be the clean way to divide things, but that really
just adds extra locking overhead. But maybe that's not much of an issue.

Thomas: I suspect I'm just not seeing where you're going with this.
Could you clarify a bit? :)

thanks
-john

Re: [PATCH 01/18] move do_timer() from kernel/timer.c into kernel/time/timekeeping.c

[Thomas Gleixner]

B1;2401;0cOn Mon, 24 Jan 2011, john stultz wrote:
> I'm guessing Thomas is thinking to move these bits into timekeeping.c so
> xtime_lock can be made static there, it just strikes me oddly.
> Especially since jiffies access is still going to need the xtime_lock,
> so we'd have to move all the jiffies code into timekeeping.c to do so.

That should go into jiffies.c. We don't need to move everything to
timekeeping.c.
 
> Splitting the xtime_lock int a static timekeeper.lock and a static
> jiffies_lock might be the clean way to divide things, but that really
> just adds extra locking overhead. But maybe that's not much of an issue.
> 
> Thomas: I suspect I'm just not seeing where you're going with this.
> Could you clarify a bit? :)

We really want to restrict xtime_lock to the core timekeeping
code. xtime_update() is really meant to take care of the timekeeping
updates. do_timer() is a horrible misnomer today.

The call to calc_global_load() is there for hysterical raisins and we
really should get rid of it sooner than later. I'm quite sure that it
could be run from a timer callback as well. Peter ?

Does that answer your questions ?

Thanks,

	tglx

Re: [PATCH 01/18] move do_timer() from kernel/timer.c into kernel/time/timekeeping.c

[Peter Zijlstra]

On Mon, 2011-01-24 at 23:10 +0100, Thomas Gleixner wrote:
> The call to calc_global_load() is there for hysterical raisins and we
> really should get rid of it sooner than later. I'm quite sure that it
> could be run from a timer callback as well. Peter ?

calc_global_load() wants to be called on just one cpu, do we have a
better place for that?

Re: [PATCH 01/18] move do_timer() from kernel/timer.c into kernel/time/timekeeping.c

[Thomas Gleixner]

On Mon, 24 Jan 2011, Peter Zijlstra wrote:

> On Mon, 2011-01-24 at 23:10 +0100, Thomas Gleixner wrote:
> > The call to calc_global_load() is there for hysterical raisins and we
> > really should get rid of it sooner than later. I'm quite sure that it
> > could be run from a timer callback as well. Peter ?
> 
> calc_global_load() wants to be called on just one cpu, do we have a
> better place for that?

Well, we can call it from a timer from a single CPU.

Thanks,

	tglx
 

Re: [PATCH 01/18] move do_timer() from kernel/timer.c into kernel/time/timekeeping.c

[Peter Zijlstra]

On Mon, 2011-01-24 at 23:44 +0100, Thomas Gleixner wrote:
> On Mon, 24 Jan 2011, Peter Zijlstra wrote:
> 
> > On Mon, 2011-01-24 at 23:10 +0100, Thomas Gleixner wrote:
> > > The call to calc_global_load() is there for hysterical raisins and we
> > > really should get rid of it sooner than later. I'm quite sure that it
> > > could be run from a timer callback as well. Peter ?
> > 
> > calc_global_load() wants to be called on just one cpu, do we have a
> > better place for that?
> 
> Well, we can call it from a timer from a single CPU.

Right that would work I guess.

Re: [PATCH 01/18] move do_timer() from kernel/timer.c into kernel/time/timekeeping.c

[Peter Zijlstra]

On Tue, 2011-01-25 at 10:08 +0100, Peter Zijlstra wrote:
> On Mon, 2011-01-24 at 23:44 +0100, Thomas Gleixner wrote:
> > On Mon, 24 Jan 2011, Peter Zijlstra wrote:
> > 
> > > On Mon, 2011-01-24 at 23:10 +0100, Thomas Gleixner wrote:
> > > > The call to calc_global_load() is there for hysterical raisins and we
> > > > really should get rid of it sooner than later. I'm quite sure that it
> > > > could be run from a timer callback as well. Peter ?
> > > 
> > > calc_global_load() wants to be called on just one cpu, do we have a
> > > better place for that?
> > 
> > Well, we can call it from a timer from a single CPU.
> 
> Right that would work I guess.

Something like the (completely untested) below would do I guess:

---
Subject: sched: Move the calc_global_load() call into the scheduler
From: Peter Zijlstra <a.p.zijlstra@chello.nl>
Date: Tue Jan 25 11:30:35 CET 2011

Remove the calc_global_load() call from the timekeeping code and make
it local to the scheduler.

Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
LKML-Reference: <new-submission>
---
 include/linux/sched.h |    2 --
 kernel/sched.c        |   22 ++++++++++++++++------
 kernel/timer.c        |    1 -
 3 files changed, 16 insertions(+), 9 deletions(-)

Index: linux-2.6/include/linux/sched.h
===================================================================
--- linux-2.6.orig/include/linux/sched.h
+++ linux-2.6/include/linux/sched.h
@@ -142,8 +142,6 @@ extern unsigned long nr_iowait_cpu(int c
 extern unsigned long this_cpu_load(void);
 
 
-extern void calc_global_load(unsigned long ticks);
-
 extern unsigned long get_parent_ip(unsigned long addr);
 
 struct seq_file;
Index: linux-2.6/kernel/sched.c
===================================================================
--- linux-2.6.orig/kernel/sched.c
+++ linux-2.6/kernel/sched.c
@@ -3192,7 +3192,7 @@ calc_load_n(unsigned long load, unsigned
  * Once we've updated the global active value, we need to apply the exponential
  * weights adjusted to the number of cycles missed.
  */
-static void calc_global_nohz(unsigned long ticks)
+static void calc_global_nohz(void)
 {
 	long delta, active, n;
 
@@ -3209,11 +3209,13 @@ static void calc_global_nohz(unsigned lo
 	if (delta)
 		atomic_long_add(delta, &calc_load_tasks);
 
+
 	/*
 	 * If we were idle for multiple load cycles, apply them.
 	 */
-	if (ticks >= LOAD_FREQ) {
-		n = ticks / LOAD_FREQ;
+	delta = jiffies - calc_load_update - 10;
+	if (delta >= LOAD_FREQ) {
+		n = delta / LOAD_FREQ;
 
 		active = atomic_long_read(&calc_load_tasks);
 		active = active > 0 ? active * FIXED_1 : 0;
@@ -3246,7 +3248,7 @@ static inline long calc_load_fold_idle(v
 	return 0;
 }
 
-static void calc_global_nohz(unsigned long ticks)
+static void calc_global_nohz(void)
 {
 }
 #endif
@@ -3266,15 +3268,20 @@ void get_avenrun(unsigned long *loads, u
 	loads[2] = (avenrun[2] + offset) << shift;
 }
 
+static void calc_global_load(unsigned long __data);
+
+static struct timer_list global_load_timer =
+	TIMER_DEFERRED_INITIALIZER(calc_global_load, 0, 0);
+
 /*
  * calc_load - update the avenrun load estimates 10 ticks after the
  * CPUs have updated calc_load_tasks.
  */
-void calc_global_load(unsigned long ticks)
+static void calc_global_load(unsigned long __data)
 {
 	long active;
 
-	calc_global_nohz(ticks);
+	calc_global_nohz();
 
 	if (time_before(jiffies, calc_load_update + 10))
 		return;
@@ -3287,6 +3294,7 @@ void calc_global_load(unsigned long tick
 	avenrun[2] = calc_load(avenrun[2], EXP_15, active);
 
 	calc_load_update += LOAD_FREQ;
+	mod_timer(&global_load_timer, calc_load_update + 10);
 }
 
 /*
@@ -8172,6 +8180,8 @@ void __init sched_init(void)
 	init_idle(current, smp_processor_id());
 
 	calc_load_update = jiffies + LOAD_FREQ;
+	global_load_timer.slack = 0;
+	mod_timer(&global_load_timer, calc_load_update + 10);
 
 	/*
 	 * During early bootup we pretend to be a normal task:
Index: linux-2.6/kernel/timer.c
===================================================================
--- linux-2.6.orig/kernel/timer.c
+++ linux-2.6/kernel/timer.c
@@ -1303,7 +1303,6 @@ void do_timer(unsigned long ticks)
 {
 	jiffies_64 += ticks;
 	update_wall_time();
-	calc_global_load(ticks);
 }
 
 #ifdef __ARCH_WANT_SYS_ALARM

Re: [PATCH 01/18] move do_timer() from kernel/timer.c into kernel/time/timekeeping.c

[Peter Zijlstra]

On Tue, 2011-01-25 at 11:34 +0100, Peter Zijlstra wrote:

> Something like the (completely untested) below would do I guess:

> @@ -8172,6 +8180,8 @@ void __init sched_init(void)
>  	init_idle(current, smp_processor_id());
>  
>  	calc_load_update = jiffies + LOAD_FREQ;
> +	global_load_timer.slack = 0;
> +	mod_timer(&global_load_timer, calc_load_update + 10);
>  
>  	/*
>  	 * During early bootup we pretend to be a normal task:

OK, so calling mod_timer() before init_timers() is _not_ a good idea ;-)

Re: [PATCH 01/18] move do_timer() from kernel/timer.c into kernel/time/timekeeping.c

[Yong Zhang]

On Tue, Jan 25, 2011 at 05:51:59PM +0100, Peter Zijlstra wrote:
> On Tue, 2011-01-25 at 11:34 +0100, Peter Zijlstra wrote:
> 
> > Something like the (completely untested) below would do I guess:
> 
> > @@ -8172,6 +8180,8 @@ void __init sched_init(void)
> >  	init_idle(current, smp_processor_id());
> >  
> >  	calc_load_update = jiffies + LOAD_FREQ;
> > +	global_load_timer.slack = 0;
> > +	mod_timer(&global_load_timer, calc_load_update + 10);
> >  
> >  	/*
> >  	 * During early bootup we pretend to be a normal task:
> 
> OK, so calling mod_timer() before init_timers() is _not_ a good idea ;-)

So I change your patch a little, and it works well on my PC ;)

0) move calc_global_times things to one function start_calc_global_timer();
1) call start_calc_global_timer() in sched_init_smp() instead of
   sched_init(); thus we change the boot-time load calculation(later
   than before), but I don't think it's matter;
2) remove timer_before() check in calc_global_load(), if that check
   returns true, we will lose global-load-timer forever.

Thanks,
Yong

---
From: Peter Zijlstra <a.p.zijlstra@chello.nl>
Subject: [PATCH] sched: Move the calc_global_load() call into the scheduler

Remove the calc_global_load() call from the timekeeping code and make
it local to the scheduler.

Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
---
 include/linux/sched.h |    2 --
 kernel/sched.c        |   34 +++++++++++++++++++++++-----------
 kernel/timer.c        |    1 -
 3 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index d747f94..f224dcc 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -142,8 +142,6 @@ extern unsigned long nr_iowait_cpu(int cpu);
 extern unsigned long this_cpu_load(void);
 
 
-extern void calc_global_load(unsigned long ticks);
-
 extern unsigned long get_parent_ip(unsigned long addr);
 
 struct seq_file;
diff --git a/kernel/sched.c b/kernel/sched.c
index 18d38e4..89f6725 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -3159,7 +3159,7 @@ calc_load_n(unsigned long load, unsigned long exp,
  * Once we've updated the global active value, we need to apply the exponential
  * weights adjusted to the number of cycles missed.
  */
-static void calc_global_nohz(unsigned long ticks)
+static void calc_global_nohz(void)
 {
 	long delta, active, n;
 
@@ -3179,8 +3179,9 @@ static void calc_global_nohz(unsigned long ticks)
 	/*
 	 * If we were idle for multiple load cycles, apply them.
 	 */
-	if (ticks >= LOAD_FREQ) {
-		n = ticks / LOAD_FREQ;
+	delta = jiffies - calc_load_update - 10;
+	if (delta >= LOAD_FREQ) {
+		n = delta / LOAD_FREQ;
 
 		active = atomic_long_read(&calc_load_tasks);
 		active = active > 0 ? active * FIXED_1 : 0;
@@ -3213,7 +3214,7 @@ static inline long calc_load_fold_idle(void)
 	return 0;
 }
 
-static void calc_global_nohz(unsigned long ticks)
+static void calc_global_nohz(void)
 {
 }
 #endif
@@ -3233,18 +3234,27 @@ void get_avenrun(unsigned long *loads, unsigned long offset, int shift)
 	loads[2] = (avenrun[2] + offset) << shift;
 }
 
+static void calc_global_load(unsigned long __data);
+
+static struct timer_list global_load_timer =
+	TIMER_DEFERRED_INITIALIZER(calc_global_load, 0, 0);
+
+static __init void start_calc_global_timer()
+{
+	calc_load_update = jiffies + LOAD_FREQ;
+	set_timer_slack(&global_load_timer, 0);
+	mod_timer(&global_load_timer, calc_load_update + 10);
+}
+
 /*
  * calc_load - update the avenrun load estimates 10 ticks after the
  * CPUs have updated calc_load_tasks.
  */
-void calc_global_load(unsigned long ticks)
+static void calc_global_load(unsigned long __data)
 {
 	long active;
 
-	calc_global_nohz(ticks);
-
-	if (time_before(jiffies, calc_load_update + 10))
-		return;
+	calc_global_nohz();
 
 	active = atomic_long_read(&calc_load_tasks);
 	active = active > 0 ? active * FIXED_1 : 0;
@@ -3254,6 +3264,7 @@ void calc_global_load(unsigned long ticks)
 	avenrun[2] = calc_load(avenrun[2], EXP_15, active);
 
 	calc_load_update += LOAD_FREQ;
+	mod_timer(&global_load_timer, calc_load_update + 10);
 }
 
 /*
@@ -7741,6 +7752,8 @@ void __init sched_init_smp(void)
 {
 	cpumask_var_t non_isolated_cpus;
 
+	start_calc_global_timer();
+
 	alloc_cpumask_var(&non_isolated_cpus, GFP_KERNEL);
 	alloc_cpumask_var(&fallback_doms, GFP_KERNEL);
 
@@ -7777,6 +7790,7 @@ void __init sched_init_smp(void)
 #else
 void __init sched_init_smp(void)
 {
+	start_calc_global_timer();
 	sched_init_granularity();
 }
 #endif /* CONFIG_SMP */
@@ -8044,8 +8058,6 @@ void __init sched_init(void)
 	 */
 	init_idle(current, smp_processor_id());
 
-	calc_load_update = jiffies + LOAD_FREQ;
-
 	/*
 	 * During early bootup we pretend to be a normal task:
 	 */
diff --git a/kernel/timer.c b/kernel/timer.c
index 43ca993..afdc13b 100644
--- a/kernel/timer.c
+++ b/kernel/timer.c
@@ -1303,7 +1303,6 @@ void do_timer(unsigned long ticks)
 {
 	jiffies_64 += ticks;
 	update_wall_time();
-	calc_global_load(ticks);
 }
 
 #ifdef __ARCH_WANT_SYS_ALARM
-- 
1.7.0.4

Re: [PATCH 01/18] move do_timer() from kernel/timer.c into kernel/time/timekeeping.c

[Yong Zhang]

On Wed, Jan 26, 2011 at 1:56 PM, Yong Zhang <yong.zhang0@gmail.com> wrote:
> From: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Subject: [PATCH] sched: Move the calc_global_load() call into the scheduler
>
> Remove the calc_global_load() call from the timekeeping code and make
> it local to the scheduler.
>
> Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
> ---
>  include/linux/sched.h |    2 --
>  kernel/sched.c        |   34 +++++++++++++++++++++++-----------
>  kernel/timer.c        |    1 -
>  3 files changed, 23 insertions(+), 14 deletions(-)
> +
> +static __init void start_calc_global_timer()

should be
static __init void start_calc_global_timer(void)

Re: [PATCH 01/18] move do_timer() from kernel/timer.c into kernel/time/timekeeping.c

[Peter Zijlstra]

On Wed, 2011-01-26 at 13:56 +0800, Yong Zhang wrote:


> +static __init void start_calc_global_timer()
> +{
> +       calc_load_update = jiffies + LOAD_FREQ;

That should really be done where it was done in sched_init(), otherwise
rq->calc_load_update and this get out of sync.

> +       set_timer_slack(&global_load_timer, 0);

Ah, there an actual function for that ;-)

> +       mod_timer(&global_load_timer, calc_load_update + 10);
> +}



> @@ -7741,6 +7752,8 @@ void __init sched_init_smp(void)
>  {
>         cpumask_var_t non_isolated_cpus;
>  
> +       start_calc_global_timer();
> +
>         alloc_cpumask_var(&non_isolated_cpus, GFP_KERNEL);
>         alloc_cpumask_var(&fallback_doms, GFP_KERNEL);
>  
> @@ -7777,6 +7790,7 @@ void __init sched_init_smp(void)
>  #else
>  void __init sched_init_smp(void)
>  {
> +       start_calc_global_timer();
>         sched_init_granularity();
>  }
>  #endif /* CONFIG_SMP */ 

Right, I done the same thing, except didn't do that wrapper function.

Re: [PATCH 01/18] move do_timer() from kernel/timer.c into kernel/time/timekeeping.c

[Yong Zhang]

On Wed, Jan 26, 2011 at 11:03:56AM +0100, Peter Zijlstra wrote:
> On Wed, 2011-01-26 at 13:56 +0800, Yong Zhang wrote:
> 
> 
> > +static __init void start_calc_global_timer()
> > +{
> > +       calc_load_update = jiffies + LOAD_FREQ;
> 
> That should really be done where it was done in sched_init(), otherwise
> rq->calc_load_update and this get out of sync.

Ah, yes. Will update.

---
From: Peter Zijlstra <a.p.zijlstra@chello.nl>
Subject: [PATCH V2] sched: Move the calc_global_load() call into the scheduler

Remove the calc_global_load() call from the timekeeping code and make
it local to the scheduler.

Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
---
 include/linux/sched.h |    2 --
 kernel/sched.c        |   31 ++++++++++++++++++++++---------
 kernel/timer.c        |    1 -
 3 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index d747f94..f224dcc 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -142,8 +142,6 @@ extern unsigned long nr_iowait_cpu(int cpu);
 extern unsigned long this_cpu_load(void);
 
 
-extern void calc_global_load(unsigned long ticks);
-
 extern unsigned long get_parent_ip(unsigned long addr);
 
 struct seq_file;
diff --git a/kernel/sched.c b/kernel/sched.c
index 18d38e4..7bbc743 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -3159,7 +3159,7 @@ calc_load_n(unsigned long load, unsigned long exp,
  * Once we've updated the global active value, we need to apply the exponential
  * weights adjusted to the number of cycles missed.
  */
-static void calc_global_nohz(unsigned long ticks)
+static void calc_global_nohz(void)
 {
 	long delta, active, n;
 
@@ -3179,8 +3179,9 @@ static void calc_global_nohz(unsigned long ticks)
 	/*
 	 * If we were idle for multiple load cycles, apply them.
 	 */
-	if (ticks >= LOAD_FREQ) {
-		n = ticks / LOAD_FREQ;
+	delta = jiffies - calc_load_update - 10;
+	if (delta >= LOAD_FREQ) {
+		n = delta / LOAD_FREQ;
 
 		active = atomic_long_read(&calc_load_tasks);
 		active = active > 0 ? active * FIXED_1 : 0;
@@ -3213,7 +3214,7 @@ static inline long calc_load_fold_idle(void)
 	return 0;
 }
 
-static void calc_global_nohz(unsigned long ticks)
+static void calc_global_nohz(void)
 {
 }
 #endif
@@ -3233,18 +3234,26 @@ void get_avenrun(unsigned long *loads, unsigned long offset, int shift)
 	loads[2] = (avenrun[2] + offset) << shift;
 }
 
+static void calc_global_load(unsigned long __data);
+
+static struct timer_list global_load_timer =
+	TIMER_DEFERRED_INITIALIZER(calc_global_load, 0, 0);
+
+static __init void start_calc_global_timer(void)
+{
+	set_timer_slack(&global_load_timer, 0);
+	mod_timer(&global_load_timer, calc_load_update + 10);
+}
+
 /*
  * calc_load - update the avenrun load estimates 10 ticks after the
  * CPUs have updated calc_load_tasks.
  */
-void calc_global_load(unsigned long ticks)
+static void calc_global_load(unsigned long __data)
 {
 	long active;
 
-	calc_global_nohz(ticks);
-
-	if (time_before(jiffies, calc_load_update + 10))
-		return;
+	calc_global_nohz();
 
 	active = atomic_long_read(&calc_load_tasks);
 	active = active > 0 ? active * FIXED_1 : 0;
@@ -3254,6 +3263,7 @@ void calc_global_load(unsigned long ticks)
 	avenrun[2] = calc_load(avenrun[2], EXP_15, active);
 
 	calc_load_update += LOAD_FREQ;
+	mod_timer(&global_load_timer, calc_load_update + 10);
 }
 
 /*
@@ -7741,6 +7751,8 @@ void __init sched_init_smp(void)
 {
 	cpumask_var_t non_isolated_cpus;
 
+	start_calc_global_timer();
+
 	alloc_cpumask_var(&non_isolated_cpus, GFP_KERNEL);
 	alloc_cpumask_var(&fallback_doms, GFP_KERNEL);
 
@@ -7777,6 +7789,7 @@ void __init sched_init_smp(void)
 #else
 void __init sched_init_smp(void)
 {
+	start_calc_global_timer();
 	sched_init_granularity();
 }
 #endif /* CONFIG_SMP */
diff --git a/kernel/timer.c b/kernel/timer.c
index 43ca993..afdc13b 100644
--- a/kernel/timer.c
+++ b/kernel/timer.c
@@ -1303,7 +1303,6 @@ void do_timer(unsigned long ticks)
 {
 	jiffies_64 += ticks;
 	update_wall_time();
-	calc_global_load(ticks);
 }
 
 #ifdef __ARCH_WANT_SYS_ALARM
-- 
1.7.1

[PATCH 02/18] provide xtime_update() which does not require holding xtime_lock like do_timer()

[Torben Hohn]

some arch code failed to lock the xtime_lock.
and some code looks like its using the xtime_lock to protect
other stuff.

this prepares the cleanup of this code.

Signed-off-by: Torben Hohn <torbenh@gmx.de>
---
 include/linux/sched.h     |    1 +
 kernel/time/timekeeping.c |    9 +++++++++
 2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index d747f94..9d9a078 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -2050,6 +2050,7 @@ extern void release_uids(struct user_namespace *ns);
 #include <asm/current.h>
 
 extern void do_timer(unsigned long ticks);
+extern void xtime_update(unsigned long ticks);
 
 extern int wake_up_state(struct task_struct *tsk, unsigned int state);
 extern int wake_up_process(struct task_struct *tsk);
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 546d82f..2deef94 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -958,3 +958,12 @@ void do_timer(unsigned long ticks)
 	update_wall_time();
 	calc_global_load(ticks);
 }
+
+/* xtime_update - updates the timer infrastructure.
+ */
+void xtime_update(unsigned long ticks)
+{
+	write_seqlock(&xtime_lock);
+	do_timer(ticks);
+	write_sequnlock(&xtime_lock);
+}
-- 
1.7.2.3

Re: [PATCH 02/18] provide xtime_update() which does not require holding xtime_lock like do_timer()

[Thomas Gleixner]

On Sat, 22 Jan 2011, Torben Hohn wrote:

> some arch code failed to lock the xtime_lock.
> and some code looks like its using the xtime_lock to protect
> other stuff.

That's not a good argument for creating xtime_update().

The point is that do_timer() needs to write lock xtime_lock and we
want to avoid the duplicated code all over the place.

The fact that some of the architectures have other code in the
xtime_lock protected region is completely irrelevant for this
change. That needs to be addressed by the arch specific patches.

> +
> +/* xtime_update - updates the timer infrastructure.

Please use proper kernel doc style. Also it does not update the timer
infrastructure, it's the timekeeping update.

The comment should also document, that this code needs to be called
with interrupts disabled.

Thanks,

	tglx

[PATCH 03/18] alpha: change do_timer() to xtime_update()

[Torben Hohn]

xtime_update() takes the xtime_lock itself.
this code looks like its protecting state with this lock.
but this is only touched here and during init.

Signed-off-by: Torben Hohn <torbenh@gmx.de>
---
 arch/alpha/kernel/time.c |    8 ++------
 1 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/arch/alpha/kernel/time.c b/arch/alpha/kernel/time.c
index c1f3e7c..a58e84f 100644
--- a/arch/alpha/kernel/time.c
+++ b/arch/alpha/kernel/time.c
@@ -159,7 +159,7 @@ void read_persistent_clock(struct timespec *ts)
 
 /*
  * timer_interrupt() needs to keep up the real-time clock,
- * as well as call the "do_timer()" routine every clocktick
+ * as well as call the "xtime_update()" routine every clocktick
  */
 irqreturn_t timer_interrupt(int irq, void *dev)
 {
@@ -172,8 +172,6 @@ irqreturn_t timer_interrupt(int irq, void *dev)
 	profile_tick(CPU_PROFILING);
 #endif
 
-	write_seqlock(&xtime_lock);
-
 	/*
 	 * Calculate how many ticks have passed since the last update,
 	 * including any previous partial leftover.  Save any resulting
@@ -187,9 +185,7 @@ irqreturn_t timer_interrupt(int irq, void *dev)
 	nticks = delta >> FIX_SHIFT;
 
 	if (nticks)
-		do_timer(nticks);
-
-	write_sequnlock(&xtime_lock);
+		xtime_update(nticks);
 
 	if (test_irq_work_pending()) {
 		clear_irq_work_pending();
-- 
1.7.2.3

[PATCH 04/18] arm: switch from do_timer() to xtime_update()

[Torben Hohn]

xtime_update takes the xtime_lock itself.

Signed-off-by: Torben Hohn <torbenh@gmx.de>
---
 arch/arm/kernel/time.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/arch/arm/kernel/time.c b/arch/arm/kernel/time.c
index 3d76bf2..1ff46ca 100644
--- a/arch/arm/kernel/time.c
+++ b/arch/arm/kernel/time.c
@@ -107,9 +107,7 @@ void timer_tick(void)
 {
 	profile_tick(CPU_PROFILING);
 	do_leds();
-	write_seqlock(&xtime_lock);
-	do_timer(1);
-	write_sequnlock(&xtime_lock);
+	xtime_update(1);
 #ifndef CONFIG_SMP
 	update_process_times(user_mode(get_irq_regs()));
 #endif
-- 
1.7.2.3

[PATCH 05/18] arm/mach-clps711x: switch do_timer() to xtime_update()

[Torben Hohn]

do_timer() requires holding the xtime_lock, which this
code did not do.
use the safe version xtime_update()

Signed-off-by: Torben Hohn <torbenh@gmx.de>
---
 arch/arm/mach-clps711x/include/mach/time.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-clps711x/include/mach/time.h b/arch/arm/mach-clps711x/include/mach/time.h
index 8fe283c..61fef91 100644
--- a/arch/arm/mach-clps711x/include/mach/time.h
+++ b/arch/arm/mach-clps711x/include/mach/time.h
@@ -30,7 +30,7 @@ p720t_timer_interrupt(int irq, void *dev_id)
 {
 	struct pt_regs *regs = get_irq_regs();
 	do_leds();
-	do_timer(1);
+	xtime_update(1);
 #ifndef CONFIG_SMP
 	update_process_times(user_mode(regs));
 #endif
-- 
1.7.2.3

[PATCH 06/18] blackfin: switch from do_timer() to xtime_update()

[Torben Hohn]

xtime_update() takes the xtime_lock itself.

Signed-off-by: Torben Hohn <torbenh@gmx.de>
---
 arch/blackfin/kernel/time.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/blackfin/kernel/time.c b/arch/blackfin/kernel/time.c
index c911361..8d73724 100644
--- a/arch/blackfin/kernel/time.c
+++ b/arch/blackfin/kernel/time.c
@@ -114,16 +114,14 @@ u32 arch_gettimeoffset(void)
 
 /*
  * timer_interrupt() needs to keep up the real-time clock,
- * as well as call the "do_timer()" routine every clocktick
+ * as well as call the "xtime_update()" routine every clocktick
  */
 #ifdef CONFIG_CORE_TIMER_IRQ_L1
 __attribute__((l1_text))
 #endif
 irqreturn_t timer_interrupt(int irq, void *dummy)
 {
-	write_seqlock(&xtime_lock);
-	do_timer(1);
-	write_sequnlock(&xtime_lock);
+	xtime_update(1);
 
 #ifdef CONFIG_IPIPE
 	update_root_process_times(get_irq_regs());
-- 
1.7.2.3

[PATCH 07/18] cris/arch-v10: switch do_timer() to xtime_update()

[Torben Hohn]

this code failed to take the xtime_lock, which must be held
when calling do_timer().

use the safe version xtime_update()

Signed-off-by: Torben Hohn <torbenh@gmx.de>
---
 arch/cris/arch-v10/kernel/time.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/cris/arch-v10/kernel/time.c b/arch/cris/arch-v10/kernel/time.c
index 00eb36f..20c85b5 100644
--- a/arch/cris/arch-v10/kernel/time.c
+++ b/arch/cris/arch-v10/kernel/time.c
@@ -140,7 +140,7 @@ stop_watchdog(void)
 
 /*
  * timer_interrupt() needs to keep up the real-time clock,
- * as well as call the "do_timer()" routine every clocktick
+ * as well as call the "xtime_update()" routine every clocktick
  */
 
 //static unsigned short myjiff; /* used by our debug routine print_timestamp */
@@ -176,7 +176,7 @@ timer_interrupt(int irq, void *dev_id)
 
 	/* call the real timer interrupt handler */
 
-	do_timer(1);
+	xtime_update(1);
 	
         cris_do_profile(regs); /* Save profiling information */
         return IRQ_HANDLED;
-- 
1.7.2.3

[PATCH 08/18] cris/arch-v32: switch do_timer() to xtime_update()

[Torben Hohn]

xtime_update() takes the xtime_lock itself.

Signed-off-by: Torben Hohn <torbenh@gmx.de>
---
 arch/cris/arch-v32/kernel/time.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/cris/arch-v32/kernel/time.c b/arch/cris/arch-v32/kernel/time.c
index a545211..bb978ed 100644
--- a/arch/cris/arch-v32/kernel/time.c
+++ b/arch/cris/arch-v32/kernel/time.c
@@ -183,7 +183,7 @@ void handle_watchdog_bite(struct pt_regs *regs)
 
 /*
  * timer_interrupt() needs to keep up the real-time clock,
- * as well as call the "do_timer()" routine every clocktick.
+ * as well as call the "xtime_update()" routine every clocktick.
  */
 extern void cris_do_profile(struct pt_regs *regs);
 
@@ -216,9 +216,7 @@ static inline irqreturn_t timer_interrupt(int irq, void *dev_id)
 		return IRQ_HANDLED;
 
 	/* Call the real timer interrupt handler */
-	write_seqlock(&xtime_lock);
-	do_timer(1);
-	write_sequnlock(&xtime_lock);
+	xtime_update(1);
         return IRQ_HANDLED;
 }
 
-- 
1.7.2.3

[PATCH 09/18] frv: switch do_timer() to xtime_update()

[Torben Hohn]

this code looks like its protecting __set_LEDS() with this lock also.
i dont think thats necessary.

Signed-off-by: Torben Hohn <torbenh@gmx.de>
---
 arch/frv/kernel/time.c |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/arch/frv/kernel/time.c b/arch/frv/kernel/time.c
index 0ddbbae..0d95162 100644
--- a/arch/frv/kernel/time.c
+++ b/arch/frv/kernel/time.c
@@ -50,7 +50,7 @@ static struct irqaction timer_irq  = {
 
 /*
  * timer_interrupt() needs to keep up the real-time clock,
- * as well as call the "do_timer()" routine every clocktick
+ * as well as call the "xtime_update()" routine every clocktick
  */
 static irqreturn_t timer_interrupt(int irq, void *dummy)
 {
@@ -61,10 +61,11 @@ static irqreturn_t timer_interrupt(int irq, void *dummy)
 	 * CPU. We need to avoid to SMP race with it. NOTE: we don't need
 	 * the irq version of write_lock because as just said we have irq
 	 * locally disabled. -arca
+	 *
+	 * xtime_update takes the writelock.
 	 */
-	write_seqlock(&xtime_lock);
 
-	do_timer(1);
+	xtime_update(1);
 
 #ifdef CONFIG_HEARTBEAT
 	static unsigned short n;
@@ -72,8 +73,6 @@ static irqreturn_t timer_interrupt(int irq, void *dummy)
 	__set_LEDS(n);
 #endif /* CONFIG_HEARTBEAT */
 
-	write_sequnlock(&xtime_lock);
-
 	update_process_times(user_mode(get_irq_regs()));
 
 	return IRQ_HANDLED;
-- 
1.7.2.3

Re: [PATCH 09/18] frv: switch do_timer() to xtime_update()

[Thomas Gleixner]

On Sat, 22 Jan 2011, Torben Hohn wrote:

> this code looks like its protecting __set_LEDS() with this lock also.
> i dont think thats necessary.

This changelog is horrible. 

This code does not look like protecting the __set_LEDS() call with the
xtime_lock. The call happens to be inside the xtime_lock held region.

Now instead of bringing up a weak argument "I dont think ..." you
should provide a proper analysis why it's safe to move that code out.

It's pretty simple:

No other call site of __set_LEDS() is protected by xtime_lock, so
xtime_lock does not protect anything related to __set_LEDS(). It's
just inside the xtime_lock region for no good reason at all.

Please be more careful when writing change logs, so a reviewer can
understand the reasoning behind your change easily.

Also all arch/* patches are missing a "Cc: arch-maintainer@somewhere.xxx".

>  {
> @@ -61,10 +61,11 @@ static irqreturn_t timer_interrupt(int irq, void *dummy)
>  	 * CPU. We need to avoid to SMP race with it. NOTE: we don't need
>  	 * the irq version of write_lock because as just said we have irq
>  	 * locally disabled. -arca
> +	 *
> +	 * xtime_update takes the writelock.

  Errm. xtime_update write locks xtime_lock. Please be careful with comments.

Thanks,

	tglx

Re: [PATCH 09/18] frv: switch do_timer() to xtime_update()

[torbenh]

On Sat, Jan 22, 2011 at 11:01:49AM +0100, Thomas Gleixner wrote:
> On Sat, 22 Jan 2011, Torben Hohn wrote:
> 
> > this code looks like its protecting __set_LEDS() with this lock also.
> > i dont think thats necessary.
> 
> This changelog is horrible. 

agreed.
what i meant was that: if i do something while holding a lock,
it seems to be necessary to do it while holding the lock.


> 
> This code does not look like protecting the __set_LEDS() call with the
> xtime_lock. The call happens to be inside the xtime_lock held region.
> 
> Now instead of bringing up a weak argument "I dont think ..." you
> should provide a proper analysis why it's safe to move that code out.
> 
> It's pretty simple:
> 
> No other call site of __set_LEDS() is protected by xtime_lock, so
> xtime_lock does not protect anything related to __set_LEDS(). It's
> just inside the xtime_lock region for no good reason at all.
> 
> Please be more careful when writing change logs, so a reviewer can
> understand the reasoning behind your change easily.

ok. will do.

> 
> Also all arch/* patches are missing a "Cc: arch-maintainer@somewhere.xxx".

i was planning to send it out with --cc-cmd=./scripts/get_maintainers.pl
once you thought it was ok.
is that procedure ok ?
or should i add CC: lines to the commitlogs ?


> 
> >  {
> > @@ -61,10 +61,11 @@ static irqreturn_t timer_interrupt(int irq, void *dummy)
> >  	 * CPU. We need to avoid to SMP race with it. NOTE: we don't need
> >  	 * the irq version of write_lock because as just said we have irq
> >  	 * locally disabled. -arca
> > +	 *
> > +	 * xtime_update takes the writelock.
> 
>   Errm. xtime_update write locks xtime_lock. Please be careful with comments.

ok

> 
> Thanks,
> 
> 	tglx

-- 
torben Hohn

Re: [PATCH 09/18] frv: switch do_timer() to xtime_update()

[Thomas Gleixner]

On Mon, 24 Jan 2011, torbenh wrote:
> On Sat, Jan 22, 2011 at 11:01:49AM +0100, Thomas Gleixner wrote:
> > On Sat, 22 Jan 2011, Torben Hohn wrote:
> > 
> > > this code looks like its protecting __set_LEDS() with this lock also.
> > > i dont think thats necessary.
> > 
> > This changelog is horrible. 
> 
> agreed.
> what i meant was that: if i do something while holding a lock,
> it seems to be necessary to do it while holding the lock.

Well, it might be that way, but obviously that's not the case with the
code which is in the xtime lock write locked region (except for the
do_timer() call). :)
 
> > Also all arch/* patches are missing a "Cc: arch-maintainer@somewhere.xxx".
> 
> i was planning to send it out with --cc-cmd=./scripts/get_maintainers.pl
> once you thought it was ok.
> is that procedure ok ?

get_maintainers creates ugly long cc lists. I prefer hand selected
cc's. For the arch specific changes cc'ing only the relevant
maintainer (retrieved from MAINTAINERS) is enough.

> or should i add CC: lines to the commitlogs ?

Either way works. If I pick up such a series, my scripts generate the
Cc list in the changelog from the mail CC list.

Thanks,

	tglx

[PATCH 10/18] h8300: switch do_timer() to xtime_update()

[Torben Hohn]

xtime_update() takes the xtime_lock itself.

Signed-off-by: Torben Hohn <torbenh@gmx.de>
---
 arch/h8300/kernel/time.c         |    4 +---
 arch/h8300/kernel/timer/timer8.c |    2 +-
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/h8300/kernel/time.c b/arch/h8300/kernel/time.c
index 165005a..32263a1 100644
--- a/arch/h8300/kernel/time.c
+++ b/arch/h8300/kernel/time.c
@@ -35,9 +35,7 @@ void h8300_timer_tick(void)
 {
 	if (current->pid)
 		profile_tick(CPU_PROFILING);
-	write_seqlock(&xtime_lock);
-	do_timer(1);
-	write_sequnlock(&xtime_lock);
+	xtime_update(1);
 	update_process_times(user_mode(get_irq_regs()));
 }
 
diff --git a/arch/h8300/kernel/timer/timer8.c b/arch/h8300/kernel/timer/timer8.c
index 3946c0f..7a1533f 100644
--- a/arch/h8300/kernel/timer/timer8.c
+++ b/arch/h8300/kernel/timer/timer8.c
@@ -61,7 +61,7 @@
 
 /*
  * timer_interrupt() needs to keep up the real-time clock,
- * as well as call the "do_timer()" routine every clocktick
+ * as well as call the "xtime_update()" routine every clocktick
  */
 
 static irqreturn_t timer_interrupt(int irq, void *dev_id)
-- 
1.7.2.3

[PATCH 11/18] ia64: switch do_timer() to xtime_update()

[Torben Hohn]

local_cpu_data->itm_next = new_itm;  does not need to be protected
by this lock. and xtime_update() takes the lock itself.

Signed-off-by: Torben Hohn <torbenh@gmx.de>
---
 arch/ia64/kernel/time.c |   19 +++++--------------
 arch/ia64/xen/time.c    |   13 +++++--------
 2 files changed, 10 insertions(+), 22 deletions(-)

diff --git a/arch/ia64/kernel/time.c b/arch/ia64/kernel/time.c
index 9702fa9..156ad80 100644
--- a/arch/ia64/kernel/time.c
+++ b/arch/ia64/kernel/time.c
@@ -190,19 +190,10 @@ timer_interrupt (int irq, void *dev_id)
 
 		new_itm += local_cpu_data->itm_delta;
 
-		if (smp_processor_id() == time_keeper_id) {
-			/*
-			 * Here we are in the timer irq handler. We have irqs locally
-			 * disabled, but we don't know if the timer_bh is running on
-			 * another CPU. We need to avoid to SMP race by acquiring the
-			 * xtime_lock.
-			 */
-			write_seqlock(&xtime_lock);
-			do_timer(1);
-			local_cpu_data->itm_next = new_itm;
-			write_sequnlock(&xtime_lock);
-		} else
-			local_cpu_data->itm_next = new_itm;
+		if (smp_processor_id() == time_keeper_id)
+			xtime_update(1);
+
+		local_cpu_data->itm_next = new_itm;
 
 		if (time_after(new_itm, ia64_get_itc()))
 			break;
@@ -222,7 +213,7 @@ skip_process_time_accounting:
 		 * comfort, we increase the safety margin by
 		 * intentionally dropping the next tick(s).  We do NOT
 		 * update itm.next because that would force us to call
-		 * do_timer() which in turn would let our clock run
+		 * xtime_update() which in turn would let our clock run
 		 * too fast (with the potentially devastating effect
 		 * of losing monotony of time).
 		 */
diff --git a/arch/ia64/xen/time.c b/arch/ia64/xen/time.c
index c1c5445..1f8244a 100644
--- a/arch/ia64/xen/time.c
+++ b/arch/ia64/xen/time.c
@@ -139,14 +139,11 @@ consider_steal_time(unsigned long new_itm)
 		run_posix_cpu_timers(p);
 		delta_itm += local_cpu_data->itm_delta * (stolen + blocked);
 
-		if (cpu == time_keeper_id) {
-			write_seqlock(&xtime_lock);
-			do_timer(stolen + blocked);
-			local_cpu_data->itm_next = delta_itm + new_itm;
-			write_sequnlock(&xtime_lock);
-		} else {
-			local_cpu_data->itm_next = delta_itm + new_itm;
-		}
+		if (cpu == time_keeper_id)
+			xtime_update(stolen + blocked);
+
+		local_cpu_data->itm_next = delta_itm + new_itm;
+
 		per_cpu(xen_stolen_time, cpu) += NS_PER_TICK * stolen;
 		per_cpu(xen_blocked_time, cpu) += NS_PER_TICK * blocked;
 	}
-- 
1.7.2.3

[PATCH 12/18] m32r: switch from do_timer() to xtime_update()

[Torben Hohn]

xtime_update() does proper locking.

Signed-off-by: Torben Hohn <torbenh@gmx.de>
---
 arch/m32r/kernel/time.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/m32r/kernel/time.c b/arch/m32r/kernel/time.c
index bda8682..84dd040 100644
--- a/arch/m32r/kernel/time.c
+++ b/arch/m32r/kernel/time.c
@@ -107,15 +107,14 @@ u32 arch_gettimeoffset(void)
 
 /*
  * timer_interrupt() needs to keep up the real-time clock,
- * as well as call the "do_timer()" routine every clocktick
+ * as well as call the "xtime_update()" routine every clocktick
  */
 static irqreturn_t timer_interrupt(int irq, void *dev_id)
 {
 #ifndef CONFIG_SMP
 	profile_tick(CPU_PROFILING);
 #endif
-	/* XXX FIXME. Uh, the xtime_lock should be held here, no? */
-	do_timer(1);
+	xtime_update(1);
 
 #ifndef CONFIG_SMP
 	update_process_times(user_mode(get_irq_regs()));
-- 
1.7.2.3

[PATCH 13/18] m68k: switch do_timer() to xtime_update()

[Torben Hohn]

xtime_update() properly takes the xtime_lock

Signed-off-by: Torben Hohn <torbenh@gmx.de>
---
 arch/m68k/bvme6000/config.c  |    4 ++--
 arch/m68k/kernel/time.c      |    4 ++--
 arch/m68k/mvme147/config.c   |    4 ++--
 arch/m68k/mvme16x/config.c   |    4 ++--
 arch/m68k/sun3/sun3ints.c    |    2 +-
 arch/m68knommu/kernel/time.c |    8 ++------
 6 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/arch/m68k/bvme6000/config.c b/arch/m68k/bvme6000/config.c
index 9fe6fef..1edd950 100644
--- a/arch/m68k/bvme6000/config.c
+++ b/arch/m68k/bvme6000/config.c
@@ -45,8 +45,8 @@ extern int bvme6000_set_clock_mmss (unsigned long);
 extern void bvme6000_reset (void);
 void bvme6000_set_vectors (void);
 
-/* Save tick handler routine pointer, will point to do_timer() in
- * kernel/sched.c, called via bvme6000_process_int() */
+/* Save tick handler routine pointer, will point to xtime_update() in
+ * kernel/timer/timekeeping.c, called via bvme6000_process_int() */
 
 static irq_handler_t tick_handler;
 
diff --git a/arch/m68k/kernel/time.c b/arch/m68k/kernel/time.c
index 06438da..18b34ee 100644
--- a/arch/m68k/kernel/time.c
+++ b/arch/m68k/kernel/time.c
@@ -37,11 +37,11 @@ static inline int set_rtc_mmss(unsigned long nowtime)
 
 /*
  * timer_interrupt() needs to keep up the real-time clock,
- * as well as call the "do_timer()" routine every clocktick
+ * as well as call the "xtime_update()" routine every clocktick
  */
 static irqreturn_t timer_interrupt(int irq, void *dummy)
 {
-	do_timer(1);
+	xtime_update(1);
 	update_process_times(user_mode(get_irq_regs()));
 	profile_tick(CPU_PROFILING);
 
diff --git a/arch/m68k/mvme147/config.c b/arch/m68k/mvme147/config.c
index 100baaa..6cb9c3a 100644
--- a/arch/m68k/mvme147/config.c
+++ b/arch/m68k/mvme147/config.c
@@ -46,8 +46,8 @@ extern void mvme147_reset (void);
 
 static int bcd2int (unsigned char b);
 
-/* Save tick handler routine pointer, will point to do_timer() in
- * kernel/sched.c, called via mvme147_process_int() */
+/* Save tick handler routine pointer, will point to xtime_update() in
+ * kernel/time/timekeeping.c, called via mvme147_process_int() */
 
 irq_handler_t tick_handler;
 
diff --git a/arch/m68k/mvme16x/config.c b/arch/m68k/mvme16x/config.c
index 11edf61..0b28e26 100644
--- a/arch/m68k/mvme16x/config.c
+++ b/arch/m68k/mvme16x/config.c
@@ -51,8 +51,8 @@ extern void mvme16x_reset (void);
 
 int bcd2int (unsigned char b);
 
-/* Save tick handler routine pointer, will point to do_timer() in
- * kernel/sched.c, called via mvme16x_process_int() */
+/* Save tick handler routine pointer, will point to xtime_update() in
+ * kernel/time/timekeeping.c, called via mvme16x_process_int() */
 
 static irq_handler_t tick_handler;
 
diff --git a/arch/m68k/sun3/sun3ints.c b/arch/m68k/sun3/sun3ints.c
index 2d9e21b..6464ad3 100644
--- a/arch/m68k/sun3/sun3ints.c
+++ b/arch/m68k/sun3/sun3ints.c
@@ -66,7 +66,7 @@ static irqreturn_t sun3_int5(int irq, void *dev_id)
 #ifdef CONFIG_SUN3
 	intersil_clear();
 #endif
-        do_timer(1);
+	xtime_update(1);
 	update_process_times(user_mode(get_irq_regs()));
         if (!(kstat_cpu(0).irqs[irq] % 20))
                 sun3_leds(led_pattern[(kstat_cpu(0).irqs[irq] % 160) / 20]);
diff --git a/arch/m68knommu/kernel/time.c b/arch/m68knommu/kernel/time.c
index d6ac2a4..6623909 100644
--- a/arch/m68knommu/kernel/time.c
+++ b/arch/m68knommu/kernel/time.c
@@ -36,7 +36,7 @@ static inline int set_rtc_mmss(unsigned long nowtime)
 #ifndef CONFIG_GENERIC_CLOCKEVENTS
 /*
  * timer_interrupt() needs to keep up the real-time clock,
- * as well as call the "do_timer()" routine every clocktick
+ * as well as call the "xtime_update()" routine every clocktick
  */
 irqreturn_t arch_timer_interrupt(int irq, void *dummy)
 {
@@ -44,11 +44,7 @@ irqreturn_t arch_timer_interrupt(int irq, void *dummy)
 	if (current->pid)
 		profile_tick(CPU_PROFILING);
 
-	write_seqlock(&xtime_lock);
-
-	do_timer(1);
-
-	write_sequnlock(&xtime_lock);
+	xtime_update(1);
 
 	update_process_times(user_mode(get_irq_regs()));
 
-- 
1.7.2.3

[PATCH 14/18] mn10300: switch do_timer() to xtimer_update()

[Torben Hohn]

xtimer_update() properly takes the xtime_lock.
mn10300_last_tsc is only accessed from this function.

Signed-off-by: Torben Hohn <torbenh@gmx.de>
---
 arch/mn10300/kernel/time.c |    6 +-----
 1 files changed, 1 insertions(+), 5 deletions(-)

diff --git a/arch/mn10300/kernel/time.c b/arch/mn10300/kernel/time.c
index 75da468..5b95500 100644
--- a/arch/mn10300/kernel/time.c
+++ b/arch/mn10300/kernel/time.c
@@ -104,8 +104,6 @@ static irqreturn_t timer_interrupt(int irq, void *dev_id)
 	unsigned tsc, elapse;
 	irqreturn_t ret;
 
-	write_seqlock(&xtime_lock);
-
 	while (tsc = get_cycles(),
 	       elapse = tsc - mn10300_last_tsc, /* time elapsed since last
 						 * tick */
@@ -114,11 +112,9 @@ static irqreturn_t timer_interrupt(int irq, void *dev_id)
 		mn10300_last_tsc += MN10300_TSC_PER_HZ;
 
 		/* advance the kernel's time tracking system */
-		do_timer(1);
+		xtime_update(1);
 	}
 
-	write_sequnlock(&xtime_lock);
-
 	ret = local_timer_interrupt();
 #ifdef CONFIG_SMP
 	send_IPI_allbutself(LOCAL_TIMER_IPI);
-- 
1.7.2.3

[PATCH 15/18] parisc: switch do_timer() to xtime_update()

[Torben Hohn]

xtime_update() takes the xtime_lock itself.

Signed-off-by: Torben Hohn <torbenh@gmx.de>
---
 arch/parisc/kernel/time.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/arch/parisc/kernel/time.c b/arch/parisc/kernel/time.c
index 05511cc..72ec318 100644
--- a/arch/parisc/kernel/time.c
+++ b/arch/parisc/kernel/time.c
@@ -163,9 +163,7 @@ irqreturn_t __irq_entry timer_interrupt(int irq, void *dev_id)
 	}
 
 	if (cpu == 0) {
-		write_seqlock(&xtime_lock);
-		do_timer(ticks_elapsed);
-		write_sequnlock(&xtime_lock);
+		xtime_update(ticks_elapsed);
 	}
 
 	return IRQ_HANDLED;
-- 
1.7.2.3

[PATCH 16/18] sparc: switch do_timer() to xtime_update()

[Torben Hohn]

xtime_update() takes the xtime_lock itself.

i dont think that pcic_clear_clock_irq() and clear_clock_irq need
to be protected by a lock.
it might make sense to move them after xtime_update() though.

Signed-off-by: Torben Hohn <torbenh@gmx.de>
---
 arch/sparc/kernel/pcic.c    |    4 +---
 arch/sparc/kernel/time_32.c |    9 ++-------
 2 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/arch/sparc/kernel/pcic.c b/arch/sparc/kernel/pcic.c
index aeaa09a..2cdc131 100644
--- a/arch/sparc/kernel/pcic.c
+++ b/arch/sparc/kernel/pcic.c
@@ -700,10 +700,8 @@ static void pcic_clear_clock_irq(void)
 
 static irqreturn_t pcic_timer_handler (int irq, void *h)
 {
-	write_seqlock(&xtime_lock);	/* Dummy, to show that we remember */
 	pcic_clear_clock_irq();
-	do_timer(1);
-	write_sequnlock(&xtime_lock);
+	xtime_update(1);
 #ifndef CONFIG_SMP
 	update_process_times(user_mode(get_irq_regs()));
 #endif
diff --git a/arch/sparc/kernel/time_32.c b/arch/sparc/kernel/time_32.c
index 9c743b1..4211bfc 100644
--- a/arch/sparc/kernel/time_32.c
+++ b/arch/sparc/kernel/time_32.c
@@ -85,7 +85,7 @@ int update_persistent_clock(struct timespec now)
 
 /*
  * timer_interrupt() needs to keep up the real-time clock,
- * as well as call the "do_timer()" routine every clocktick
+ * as well as call the "xtime_update()" routine every clocktick
  */
 
 #define TICK_SIZE (tick_nsec / 1000)
@@ -96,14 +96,9 @@ static irqreturn_t timer_interrupt(int dummy, void *dev_id)
 	profile_tick(CPU_PROFILING);
 #endif
 
-	/* Protect counter clear so that do_gettimeoffset works */
-	write_seqlock(&xtime_lock);
-
 	clear_clock_irq();
 
-	do_timer(1);
-
-	write_sequnlock(&xtime_lock);
+	xtime_update(1);
 
 #ifndef CONFIG_SMP
 	update_process_times(user_mode(get_irq_regs()));
-- 
1.7.2.3

Re: [PATCH 16/18] sparc: switch do_timer() to xtime_update()

[David Miller]

From: Torben Hohn <torbenh@gmx.de>
Date: Sat, 22 Jan 2011 00:07:02 +0100

> xtime_update() takes the xtime_lock itself.
> 
> i dont think that pcic_clear_clock_irq() and clear_clock_irq need
> to be protected by a lock.
> it might make sense to move them after xtime_update() though.
> 
> Signed-off-by: Torben Hohn <torbenh@gmx.de>

Agreed:

Acked-by: David S. Miller <davem@davemloft.net>

[PATCH 17/18] xtensa: switch do_timer() to xtime_update()

[Torben Hohn]

xtime_update() takes the xtime_lock itself.

i dont believe that set_linux_timer() needs to be protected by a lock.

Signed-off-by: Torben Hohn <torbenh@gmx.de>
---
 arch/xtensa/kernel/time.c |    6 +-----
 1 files changed, 1 insertions(+), 5 deletions(-)

diff --git a/arch/xtensa/kernel/time.c b/arch/xtensa/kernel/time.c
index 19df764..f3e5eb4 100644
--- a/arch/xtensa/kernel/time.c
+++ b/arch/xtensa/kernel/time.c
@@ -96,16 +96,12 @@ again:
 		update_process_times(user_mode(get_irq_regs()));
 #endif
 
-		write_seqlock(&xtime_lock);
-
-		do_timer(1); /* Linux handler in kernel/timer.c */
+		xtime_update(1); /* Linux handler in kernel/time/timekeeping */
 
 		/* Note that writing CCOMPARE clears the interrupt. */
 
 		next += CCOUNT_PER_JIFFY;
 		set_linux_timer(next);
-
-		write_sequnlock(&xtime_lock);
 	}
 
 	/* Allow platform to do something useful (Wdog). */
-- 
1.7.2.3

[PATCH 18/18] make do_timer() and xtime_lock private to the timer code

[Torben Hohn]

this commit finishes the xtime_lock cleanup.
arch code is now using xtime_update().

and do_timer() and xtime_lock are declared in kernel/time/timer-internal.h
i did not use tick-internal.h because it would have meant that i needed
to include some more stuff in kernel/time/ntp.c

Signed-off-by: Torben Hohn <torbenh@gmx.de>
---
 include/linux/sched.h        |    1 -
 include/linux/time.h         |    2 --
 kernel/hrtimer.c             |    2 ++
 kernel/time/ntp.c            |    2 ++
 kernel/time/tick-common.c    |    1 +
 kernel/time/tick-sched.c     |    1 +
 kernel/time/timer-internal.h |    7 +++++++
 7 files changed, 13 insertions(+), 3 deletions(-)
 create mode 100644 kernel/time/timer-internal.h

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 9d9a078..cdef640 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -2049,7 +2049,6 @@ extern void release_uids(struct user_namespace *ns);
 
 #include <asm/current.h>
 
-extern void do_timer(unsigned long ticks);
 extern void xtime_update(unsigned long ticks);
 
 extern int wake_up_state(struct task_struct *tsk, unsigned int state);
diff --git a/include/linux/time.h b/include/linux/time.h
index 1e6d3b5..4a324ac 100644
--- a/include/linux/time.h
+++ b/include/linux/time.h
@@ -113,8 +113,6 @@ static inline struct timespec timespec_sub(struct timespec lhs,
 #define timespec_valid(ts) \
 	(((ts)->tv_sec >= 0) && (((unsigned long) (ts)->tv_nsec) < NSEC_PER_SEC))
 
-extern seqlock_t xtime_lock;
-
 extern void read_persistent_clock(struct timespec *ts);
 extern void read_boot_clock(struct timespec *ts);
 extern int update_persistent_clock(struct timespec now);
diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index 0c8d7c0..63f4714 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -50,6 +50,8 @@
 
 #include <trace/events/timer.h>
 
+#include "time/timer-internal.h"
+
 /*
  * The timer bases:
  *
diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 5c00242..f3b47ad 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -16,6 +16,8 @@
 #include <linux/mm.h>
 #include <linux/module.h>
 
+#include "timer-internal.h"
+
 /*
  * NTP timekeeping variables:
  */
diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c
index 051bc80..7ff565b 100644
--- a/kernel/time/tick-common.c
+++ b/kernel/time/tick-common.c
@@ -23,6 +23,7 @@
 #include <asm/irq_regs.h>
 
 #include "tick-internal.h"
+#include "timer-internal.h"
 
 /*
  * Tick devices
diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 3e216e0..ad67e55 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -25,6 +25,7 @@
 #include <asm/irq_regs.h>
 
 #include "tick-internal.h"
+#include "timer-internal.h"
 
 /*
  * Per cpu nohz control structure
diff --git a/kernel/time/timer-internal.h b/kernel/time/timer-internal.h
new file mode 100644
index 0000000..f84a499
--- /dev/null
+++ b/kernel/time/timer-internal.h
@@ -0,0 +1,7 @@
+/*
+ * timer internal variable and functions
+ */
+
+extern void do_timer(unsigned long ticks);
+extern seqlock_t xtime_lock;
+
-- 
1.7.2.3

Re: make arch code use xtime_update() instead of do_timer()

[Thomas Gleixner]

On Sat, 22 Jan 2011, Torben Hohn wrote:
 
> move do_timer() into kernel/time/timekeeping.c
> and provide a version which properly takes the xtime_lock()
> 
> finally makes do_timer() and xtime_lock into a header file
> private to kernel/time
> 
> xtime_lock is still used by kernel/hrtimer.c and kernel/time.c
> i am not sure, whether the small header file is the right
> thing though.

get_jiffies_64() and the jiffies export should move into
kernel/time/jiffies.c

The hrtimer bits should be replaced by a function in timekeeping.c
which collects xtime and wall_to_monotonic.

Thanks,

	tglx