[PATCH] simplify update_times (avoid jiffies/jiffies_64 aliasing problem)

[PATCH] simplify update_times (avoid jiffies/jiffies_64 aliasing problem)

[Atsushi Nemoto]

In kernel 2.6, update_times() is called directly in timer interrupt,
so there is no point calculating ticks here.  This also get rid of
difference of jiffies and jiffies_64 due to compiler's optimization
(which was reported previously with subject "jiffies_64 vs. jiffies").

Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>

diff --git a/kernel/timer.c b/kernel/timer.c
index fe3a9a9..6188c99 100644
--- a/kernel/timer.c
+++ b/kernel/timer.c
@@ -906,14 +906,9 @@ void run_local_timers(void)
  */
 static inline void update_times(void)
 {
-	unsigned long ticks;
-
-	ticks = jiffies - wall_jiffies;
-	if (ticks) {
-		wall_jiffies += ticks;
-		update_wall_time(ticks);
-	}
-	calc_load(ticks);
+	wall_jiffies++;
+	update_wall_time(1);
+	calc_load(1);
 }
   
 /*

---
Atsushi Nemoto

Re: [PATCH] simplify update_times (avoid jiffies/jiffies_64 aliasing problem)

[Atsushi Nemoto]

>>>>> On Thu, 02 Mar 2006 23:02:27 +0900 (JST), Atsushi Nemoto <anemo@mba.ocn.ne.jp> said:
anemo> In kernel 2.6, update_times() is called directly in timer
anemo> interrupt, so there is no point calculating ticks here.  This
anemo> also get rid of difference of jiffies and jiffies_64 due to
anemo> compiler's optimization (which was reported previously with
anemo> subject "jiffies_64 vs. jiffies").

Sorry, it seems the patch breaks lost-tick handling on x86_64.  Here
is a revised patch including its adjustment.


In kernel 2.6, update_times() is called directly in timer interrupt,
so there is no point calculating ticks here.  This also get rid of
difference of jiffies and jiffies_64 due to compiler's optimization
(which was reported previously with subject "jiffies_64 vs. jiffies").

Also adjust x86_64 timer interrupt handler with this change.

Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>

diff --git a/arch/x86_64/kernel/time.c b/arch/x86_64/kernel/time.c
index 3080f84..7a1d790 100644
--- a/arch/x86_64/kernel/time.c
+++ b/arch/x86_64/kernel/time.c
@@ -423,7 +423,8 @@ void main_timer_handler(struct pt_regs *
 
 	if (lost > 0) {
 		handle_lost_ticks(lost, regs);
-		jiffies += lost;
+		while (lost--)
+			do_timer(regs);
 	}
 
 /*
diff --git a/kernel/timer.c b/kernel/timer.c
index fe3a9a9..6188c99 100644
--- a/kernel/timer.c
+++ b/kernel/timer.c
@@ -906,14 +906,9 @@ void run_local_timers(void)
  */
 static inline void update_times(void)
 {
-	unsigned long ticks;
-
-	ticks = jiffies - wall_jiffies;
-	if (ticks) {
-		wall_jiffies += ticks;
-		update_wall_time(ticks);
-	}
-	calc_load(ticks);
+	wall_jiffies++;
+	update_wall_time(1);
+	calc_load(1);
 }
   
 /*

Re: [PATCH] simplify update_times (avoid jiffies/jiffies_64 aliasing problem)

[Christoph Lameter]

On Thu, 2 Mar 2006, Atsushi Nemoto wrote:

> In kernel 2.6, update_times() is called directly in timer interrupt,
> so there is no point calculating ticks here.  This also get rid of
> difference of jiffies and jiffies_64 due to compiler's optimization
> (which was reported previously with subject "jiffies_64 vs. jiffies").

If update_wall_time() and calc_load() are always called with the constant 
one  then you may be able to optimize these two functions as well.

Re: [PATCH] simplify update_times (avoid jiffies/jiffies_64 aliasing problem)

[Atsushi Nemoto]

>>>>> On Thu, 2 Mar 2006 11:09:16 -0800 (PST), Christoph Lameter <clameter@engr.sgi.com> said:
>> In kernel 2.6, update_times() is called directly in timer
>> interrupt, so there is no point calculating ticks here.  This also
>> get rid of difference of jiffies and jiffies_64 due to compiler's
>> optimization (which was reported previously with subject
>> "jiffies_64 vs. jiffies").

clameter> If update_wall_time() and calc_load() are always called with
clameter> the constant one then you may be able to optimize these two
clameter> functions as well.

Sure.  I tried to do only one thing at a time, but it might be better
to clean them up together.  Patch revised.


In kernel 2.6, update_times() is called directly in timer interrupt,
so there is no point calculating ticks here.  Then update_wall_time()
and calc_load() can also be optimized.  This also get rid of
difference of jiffies and jiffies_64 due to compiler's optimization
(which was reported previously with subject "jiffies_64 vs. jiffies").

Also adjust x86_64 timer interrupt handler with this change.

Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>

diff --git a/arch/x86_64/kernel/time.c b/arch/x86_64/kernel/time.c
index 3080f84..7a1d790 100644
--- a/arch/x86_64/kernel/time.c
+++ b/arch/x86_64/kernel/time.c
@@ -423,7 +423,8 @@ void main_timer_handler(struct pt_regs *
 
 	if (lost > 0) {
 		handle_lost_ticks(lost, regs);
-		jiffies += lost;
+		while (lost--)
+			do_timer(regs);
 	}
 
 /*
diff --git a/kernel/timer.c b/kernel/timer.c
index fc6646f..54544a7 100644
--- a/kernel/timer.c
+++ b/kernel/timer.c
@@ -787,24 +787,14 @@ u64 current_tick_length(void)
 	return ((u64) delta_nsec << (SHIFT_SCALE - 10)) + time_adj;
 }
 
-/*
- * Using a loop looks inefficient, but "ticks" is
- * usually just one (we shouldn't be losing ticks,
- * we're doing this this way mainly for interrupt
- * latency reasons, not because we think we'll
- * have lots of lost timer ticks
- */
-static void update_wall_time(unsigned long ticks)
+static void update_wall_time(void)
 {
-	do {
-		ticks--;
-		update_wall_time_one_tick();
-		if (xtime.tv_nsec >= 1000000000) {
-			xtime.tv_nsec -= 1000000000;
-			xtime.tv_sec++;
-			second_overflow();
-		}
-	} while (ticks);
+	update_wall_time_one_tick();
+	if (xtime.tv_nsec >= 1000000000) {
+		xtime.tv_nsec -= 1000000000;
+		xtime.tv_sec++;
+		second_overflow();
+	}
 }
 
 /*
@@ -849,15 +839,15 @@ unsigned long avenrun[3];
 EXPORT_SYMBOL(avenrun);
 
 /*
- * calc_load - given tick count, update the avenrun load estimates.
+ * calc_load - update the avenrun load estimates.
  * This is called while holding a write_lock on xtime_lock.
  */
-static inline void calc_load(unsigned long ticks)
+static inline void calc_load(void)
 {
 	unsigned long active_tasks; /* fixed-point */
 	static int count = LOAD_FREQ;
 
-	count -= ticks;
+	count--;
 	if (count < 0) {
 		count += LOAD_FREQ;
 		active_tasks = count_active_tasks();
@@ -906,14 +896,9 @@ void run_local_timers(void)
  */
 static inline void update_times(void)
 {
-	unsigned long ticks;
-
-	ticks = jiffies - wall_jiffies;
-	if (ticks) {
-		wall_jiffies += ticks;
-		update_wall_time(ticks);
-	}
-	calc_load(ticks);
+	wall_jiffies++;
+	update_wall_time();
+	calc_load();
 }
   
 /*

Re: [PATCH] simplify update_times (avoid jiffies/jiffies_64 aliasing problem)

[Andrew Morton]

Atsushi Nemoto <anemo@mba.ocn.ne.jp> wrote:
>
> ...
>
> In kernel 2.6, update_times() is called directly in timer interrupt,
> so there is no point calculating ticks here.  Then update_wall_time()
> and calc_load() can also be optimized.  This also get rid of
> difference of jiffies and jiffies_64 due to compiler's optimization
> (which was reported previously with subject "jiffies_64 vs. jiffies").
> 
> Also adjust x86_64 timer interrupt handler with this change.
> 
> diff --git a/arch/x86_64/kernel/time.c b/arch/x86_64/kernel/time.c
> index 3080f84..7a1d790 100644
> --- a/arch/x86_64/kernel/time.c
> +++ b/arch/x86_64/kernel/time.c
> @@ -423,7 +423,8 @@ void main_timer_handler(struct pt_regs *
>  
>  	if (lost > 0) {
>  		handle_lost_ticks(lost, regs);
> -		jiffies += lost;
> +		while (lost--)
> +			do_timer(regs);
>  	}
>  
>  /*
> diff --git a/kernel/timer.c b/kernel/timer.c
> index fc6646f..54544a7 100644
> --- a/kernel/timer.c
> +++ b/kernel/timer.c
> @@ -787,24 +787,14 @@ u64 current_tick_length(void)
>  	return ((u64) delta_nsec << (SHIFT_SCALE - 10)) + time_adj;
>  }
>  
> -/*
> - * Using a loop looks inefficient, but "ticks" is
> - * usually just one (we shouldn't be losing ticks,
> - * we're doing this this way mainly for interrupt
> - * latency reasons, not because we think we'll
> - * have lots of lost timer ticks
> - */
> -static void update_wall_time(unsigned long ticks)
> +static void update_wall_time(void)
>  {
> -	do {
> -		ticks--;
> -		update_wall_time_one_tick();
> -		if (xtime.tv_nsec >= 1000000000) {
> -			xtime.tv_nsec -= 1000000000;
> -			xtime.tv_sec++;
> -			second_overflow();
> -		}
> -	} while (ticks);
> +	update_wall_time_one_tick();
> +	if (xtime.tv_nsec >= 1000000000) {
> +		xtime.tv_nsec -= 1000000000;
> +		xtime.tv_sec++;
> +		second_overflow();
> +	}
>  }
>  
>  /*
> @@ -849,15 +839,15 @@ unsigned long avenrun[3];
>  EXPORT_SYMBOL(avenrun);
>  
>  /*
> - * calc_load - given tick count, update the avenrun load estimates.
> + * calc_load - update the avenrun load estimates.
>   * This is called while holding a write_lock on xtime_lock.
>   */
> -static inline void calc_load(unsigned long ticks)
> +static inline void calc_load(void)
>  {
>  	unsigned long active_tasks; /* fixed-point */
>  	static int count = LOAD_FREQ;
>  
> -	count -= ticks;
> +	count--;
>  	if (count < 0) {
>  		count += LOAD_FREQ;
>  		active_tasks = count_active_tasks();
> @@ -906,14 +896,9 @@ void run_local_timers(void)
>   */
>  static inline void update_times(void)
>  {
> -	unsigned long ticks;
> -
> -	ticks = jiffies - wall_jiffies;
> -	if (ticks) {
> -		wall_jiffies += ticks;
> -		update_wall_time(ticks);
> -	}
> -	calc_load(ticks);
> +	wall_jiffies++;
> +	update_wall_time();
> +	calc_load();
>  }
>    
>  /*

I'm actually creaking under the load of timer patches over here.  A lot of
the above code has been heavily redone in John's time patches.  I guess the
above optimisation is still relevant after John's work (?) but we need to
decide what to do.   Now is as good a time as any.

John, that timer stuff is so fundamental and hits on code which has
historically been so fragile that I'm not sure it's even 2.6.17 material. 
In which case we should sneak patches like the above underneath it all.

Or we decide to take your work into 2.6.17, in which case the above needs
to be redone for that context.

I'm not sure how to resolve this, really.  Worried.  Have you socialised
those changes with architecture maintainers?  If so, what was the feedback?

Andi, have you had time to think about it all?

Thanks.

Re: [PATCH] simplify update_times (avoid jiffies/jiffies_64 aliasing problem)

[Andi Kleen]

> Andi, have you had time to think about it all?

No, sorry.

-Andi

Re: [PATCH] simplify update_times (avoid jiffies/jiffies_64 aliasing problem)

[Atsushi Nemoto]

>>>>> On Thu, 2 Mar 2006 19:04:08 -0800, Andrew Morton <akpm@osdl.org> said:
akpm> John, that timer stuff is so fundamental and hits on code which
akpm> has historically been so fragile that I'm not sure it's even
akpm> 2.6.17 material.  In which case we should sneak patches like the
akpm> above underneath it all.

akpm> Or we decide to take your work into 2.6.17, in which case the
akpm> above needs to be redone for that context.

I think most important part is update_times() cleanup (to avoid
jiffies/wall_jiffies mismatch) and it (and x86_64 part) seems not
conflict with john's work for now.

akpm> I'm not sure how to resolve this, really.  Worried.  Have you
akpm> socialised those changes with architecture maintainers?  If so,
akpm> what was the feedback?

I and Ralf talked a bit about the jiffies issue.  Making an union
containing jiffies and jiffies_64 looks good to avoid such an
optimization problem, but it would affect so many existing codes.

---
Atsushi Nemoto

Re: [PATCH] simplify update_times (avoid jiffies/jiffies_64 aliasing problem)

[David S. Miller]

From: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
Date: Fri, 03 Mar 2006 13:31:25 +0900 (JST)

> I and Ralf talked a bit about the jiffies issue.  Making an union
> containing jiffies and jiffies_64 looks good to avoid such an
> optimization problem, but it would affect so many existing codes.

Maybe use an anonymous union?  That might help...

Re: [PATCH] simplify update_times (avoid jiffies/jiffies_64 aliasing problem)

[Atsushi Nemoto]

>>>>> On Thu, 02 Mar 2006 21:45:56 -0800 (PST), "David S. Miller" <davem@davemloft.net> said:

>> I and Ralf talked a bit about the jiffies issue.  Making an union
>> containing jiffies and jiffies_64 looks good to avoid such an
>> optimization problem, but it would affect so many existing codes.

davem> Maybe use an anonymous union?  That might help...

Do you mean something like this:

union {
  struct {
    unsigned long pad;
    unsigned long jiffies;
  };
  u64 jiffies_64;
};

Unfortunately a toplevel anonymous union looks not allowed...

---
Atsushi Nemoto

Re: [PATCH] simplify update_times (avoid jiffies/jiffies_64 aliasing problem)

[Atsushi Nemoto]

akpm> I'm not sure how to resolve this, really.  Worried.  Have you
akpm> socialised those changes with architecture maintainers?  If so,
akpm> what was the feedback?

For a short term fix, barrier() might be a safe option.


Add an optimization barrier to prevent prefetching jiffies before
incrementing jiffies_64.

Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>

diff --git a/kernel/timer.c b/kernel/timer.c
index fc6646f..fdd12a5 100644
--- a/kernel/timer.c
+++ b/kernel/timer.c
@@ -925,6 +925,7 @@ static inline void update_times(void)
 void do_timer(struct pt_regs *regs)
 {
 	jiffies_64++;
+	barrier();
 	update_times();
 	softlockup_tick(regs);
 }

Re: [PATCH] simplify update_times (avoid jiffies/jiffies_64 aliasing problem)

[Andrew Morton]

Atsushi Nemoto <anemo@mba.ocn.ne.jp> wrote:
>
> akpm> I'm not sure how to resolve this, really.  Worried.  Have you
> akpm> socialised those changes with architecture maintainers?  If so,
> akpm> what was the feedback?
> 
> For a short term fix, barrier() might be a safe option.
> 
> 
> Add an optimization barrier to prevent prefetching jiffies before
> incrementing jiffies_64.
> 
> Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
> 
> diff --git a/kernel/timer.c b/kernel/timer.c
> index fc6646f..fdd12a5 100644
> --- a/kernel/timer.c
> +++ b/kernel/timer.c
> @@ -925,6 +925,7 @@ static inline void update_times(void)
>  void do_timer(struct pt_regs *regs)
>  {
>  	jiffies_64++;
> +	barrier();
>  	update_times();
>  	softlockup_tick(regs);
>  }

a) Please never ever add any form of barrier without adding a comment
   explaining why it's there.  Unless it's extremely obvious (and it rarely
   is), it's hard for the reader to work out what on earth it's doing
   there.

   Example?  Take a look at the smp_rmb() in ext3_get_inode_block(),
   work out why it's there.  Time yourself.

b) On 64-bit machines jiffies and jiffies_64 always have the same
   address (don't they?) Is the compiler really going to move a read of an
   absolute address ahead of a modification of the same address?

   <looks>

   The address of jiffies isn't known until link time, so yup, the risk
   is there.

c) jiffies is declared volatile.  In practice, if I know my gcc, it's
   just not going to play these reordering games with a volatile.

   If that's true, and if some standard (presumably c99) says that it
   must be true then I don't think we need the patch.

Re: [PATCH] simplify update_times (avoid jiffies/jiffies_64 aliasing problem)

[Andi Kleen]

> b) On 64-bit machines jiffies and jiffies_64 always have the same
>    address (don't they?) Is the compiler really going to move a read of an
>    absolute address ahead of a modification of the same address?
> 
>    <looks>
> 
>    The address of jiffies isn't known until link time, so yup, the risk
>    is there.

Yes maybe it would be better to just use  #define there.
jiffies_64 always was a bit too clever.

> 
> c) jiffies is declared volatile.  In practice, if I know my gcc, it's
>    just not going to play these reordering games with a volatile.
> 
>    If that's true, and if some standard (presumably c99) says that it
>    must be true then I don't think we need the patch.

The standards definition of volatile is unfortunately quite vague,
so at least from this side you cannot rely on much.

Also I assume Atsushi-san did the patch because he saw a real problem?

-Andi

Re: [PATCH] simplify update_times (avoid jiffies/jiffies_64 aliasing problem)

[Andrew Morton]

Andi Kleen <ak@muc.de> wrote:
>
> > b) On 64-bit machines jiffies and jiffies_64 always have the same
> >    address (don't they?) Is the compiler really going to move a read of an
> >    absolute address ahead of a modification of the same address?
> > 
> >    <looks>
> > 
> >    The address of jiffies isn't known until link time, so yup, the risk
> >    is there.
> 
> Yes maybe it would be better to just use  #define there.
> jiffies_64 always was a bit too clever.

hm.   It's actually rather hard.

One could do something like:

extern u64 __jiffy_data jiffies_64;
#define jiffies (*(u32 *)((long)&jiffies_64 + 2))

or


#if BITS_PER_LONG == 64
extern u64 __jiffy_data jiffies;
#define jiffies_64 jiffies
#else
extern union jiffy_thing {
#ifdef BIG_ENDIAN
	struct {
		u32 pad;
		u32 jiffies;
	},
	u64 jiffies_64;
#else
	u64 jiffies_64;
	struct {
		u32 pad;
		u32 jiffies;
	},
#endif
} __jiffy_data jiffies_thing;
#define jiffies_64 jiffies_thing.jiffies_64
#define jiffies jiffies_thing.jiffies
#endif

But then any code which uses `jiffies' as a local variable will explode.  I
guess we should rename those anyway.  include/linux/cyclades.h is one such.

Making `jiffies_64' a #define is less risky, but that doesn't work for
32-bit.

> > 
> > c) jiffies is declared volatile.  In practice, if I know my gcc, it's
> >    just not going to play these reordering games with a volatile.
> > 
> >    If that's true, and if some standard (presumably c99) says that it
> >    must be true then I don't think we need the patch.
> 
> The standards definition of volatile is unfortunately quite vague,
> so at least from this side you cannot rely on much.

Yup.

> Also I assume Atsushi-san did the patch because he saw a real problem?

I don't know.  I suspect its only observeable effect would be an
off-by-one-tick in wall_jiffies which will cancel out whereever anyone
takes a delta.

Re: [PATCH] simplify update_times (avoid jiffies/jiffies_64 aliasing problem)

[Andi Kleen]

On Saturday 04 March 2006 12:40, Andrew Morton wrote:

> Making `jiffies_64' a #define is less risky, but that doesn't work for
> 32-bit.

I meant the define on 64bit only. You're right on 32bit it would be somewhat messy.
But then we have to solve the problem on 32bit anyways, so it wasn't that hot
an idea.

-Andi

Re: [PATCH] simplify update_times (avoid jiffies/jiffies_64 aliasing problem)

[Paul Mackerras]

Andrew Morton writes:

> Andi Kleen <ak@muc.de> wrote:
> > Yes maybe it would be better to just use  #define there.
> > jiffies_64 always was a bit too clever.
> 
> hm.   It's actually rather hard.

I think the thing that makes most sense is:

#define jiffies	((unsigned long) jiffies_64)

and fix the few drivers that use `jiffies' as a local variable.
No-one should be trying to write to jiffies, and the compiler will do
the right thing for reads of jiffies on 32-bit platforms (it does on
ppc32 at least).

Paul.

Re: [PATCH] simplify update_times (avoid jiffies/jiffies_64 aliasing problem)

[Atsushi Nemoto]

>>>>> On Sun, 5 Mar 2006 10:13:56 +1100, Paul Mackerras <paulus@samba.org> said:
paul> I think the thing that makes most sense is:

paul> #define jiffies ((unsigned long) jiffies_64)

paul> and fix the few drivers that use `jiffies' as a local variable.
paul> No-one should be trying to write to jiffies, and the compiler
paul> will do the right thing for reads of jiffies on 32-bit platforms
paul> (it does on ppc32 at least).

I think making jiffies a non l-value is good, but this drops volatile
attribute from jiffies (since jiffies_64 is not volatile).  It might
break some codes which do polling on jiffies.

Something like this would be better?

#if defined(__BIG_ENDIAN) && BITS_PER_LONG == 32
#define jiffies (*((const volatile unsigned long *)&jiffies_64 + 1))
#else
#define jiffies (*(const volatile unsigned long *)&jiffies_64)
#endif

And if we do not want to depend on -fno-strict-aliasing, the union
would be The Only Neat Thing to Do.

---
Atsushi Nemoto

Re: [PATCH] simplify update_times (avoid jiffies/jiffies_64 aliasing problem)

[Paul Mackerras]

Andi Kleen writes:

> Also I assume Atsushi-san did the patch because he saw a real problem?

Yes, one which I also saw on PPC.  The compiler (gcc-4) emits loads
for jiffies, jiffies64 and wall_jiffies before storing the incremented
jiffies64 value back.

Paul.

Re: [PATCH] simplify update_times (avoid jiffies/jiffies_64 aliasing problem)

[Andrew Morton]

Paul Mackerras <paulus@samba.org> wrote:
>
> Andi Kleen writes:
> 
> > Also I assume Atsushi-san did the patch because he saw a real problem?
> 
> Yes, one which I also saw on PPC.  The compiler (gcc-4) emits loads
> for jiffies, jiffies64 and wall_jiffies before storing the incremented
> jiffies64 value back.
> 

What was the effect of that?

Re: [PATCH] simplify update_times (avoid jiffies/jiffies_64 aliasing problem)

[Paul Mackerras]

Andrew Morton writes:
> Paul Mackerras <paulus@samba.org> wrote:
> >
> > Andi Kleen writes:
> > 
> > > Also I assume Atsushi-san did the patch because he saw a real problem?
> > 
> > Yes, one which I also saw on PPC.  The compiler (gcc-4) emits loads
> > for jiffies, jiffies64 and wall_jiffies before storing the incremented
> > jiffies64 value back.
> > 
> 
> What was the effect of that?

The effect is that the first call to do_timer doesn't increment xtime.
This explains why the code I have to detect disagreements between
xtime and the time of day as computed from the timebase register was
finding a disagreement on the first tick, which I was scratching my
head over.

There may be other effects on architectures which use wall_jiffies to
detect lost timer ticks.  We don't have that problem on PPC and we
don't use wall_jiffies in computing time of day.

Paul.

Re: [PATCH] simplify update_times (avoid jiffies/jiffies_64 aliasing problem)

[Atsushi Nemoto]

>>>>> On Sat, 4 Mar 2006 23:33:04 +1100, Paul Mackerras <paulus@samba.org> said:
>> > > Also I assume Atsushi-san did the patch because he saw a real problem?
>> > 
>> > Yes, one which I also saw on PPC.  The compiler (gcc-4) emits loads
>> > for jiffies, jiffies64 and wall_jiffies before storing the incremented
>> > jiffies64 value back.
>> > 
>> 
>> What was the effect of that?

paulus> The effect is that the first call to do_timer doesn't increment xtime.
paulus> This explains why the code I have to detect disagreements between
paulus> xtime and the time of day as computed from the timebase register was
paulus> finding a disagreement on the first tick, which I was scratching my
paulus> head over.

paulus> There may be other effects on architectures which use wall_jiffies to
paulus> detect lost timer ticks.  We don't have that problem on PPC and we
paulus> don't use wall_jiffies in computing time of day.

Well, I found this problem during investigating an another MIPS
do_gettimeofday() bug which was fixed recently.  It was:

	lost = jiffies - wall_jiffies;
	...
	} else if (unlikely(lost))
		usecs += lost * tick_usec;

The tick_usec should not be used here since it is based on USER_HZ.
It is 'usecs += lost * (USECS_PER_SEC /HZ)' now.

I wondered why this bug really affects gettimeofday() always though
the 'lost' should usually be 0.  And finally I noticed the
jiffies/jiffies_64 optimization issue.

---
Atsushi Nemoto

Re: [PATCH] simplify update_times (avoid jiffies/jiffies_64 aliasing problem)

[john stultz]

On Thu, 2006-03-02 at 19:04 -0800, Andrew Morton wrote:
> I'm actually creaking under the load of timer patches over here.  A lot of
> the above code has been heavily redone in John's time patches.  I guess the
> above optimisation is still relevant after John's work (?) but we need to
> decide what to do.   Now is as good a time as any.
> 
> John, that timer stuff is so fundamental and hits on code which has
> historically been so fragile that I'm not sure it's even 2.6.17 material. 
> In which case we should sneak patches like the above underneath it all.
>
> Or we decide to take your work into 2.6.17, in which case the above needs
> to be redone for that context.

I'm not opposed to queuing it up as it seems like a logical cleanup. I'd
be fine with it going in before my patch, however it still needs to
address i386 lost tick compensation.  I worry that addressing that issue
before my patchset (which makes the lost tick compensation unnecessary)
might be a bit more complex. I think it would be easier going in after
my patch. I do think the barrier fix (with a comment) is a good short
term fix.

Atsushi: Your thoughts? 


> I'm not sure how to resolve this, really.  Worried.  Have you socialised
> those changes with architecture maintainers?  If so, what was the feedback?

As to the larger issue of if my patch set is 2.6.17 ready, I'd like to
think it is. There are some optimizations I'm working on that Roman has
suggested that should improve some of the periodic_hook overhead and the
NTP accuracy, but so far I've not noticed the changes helping or hurting
much (also I haven't gotten any feedback on my last attempt). I plan on
continuing that work, but I feel the benefits (as in the number of real
problems that it would resolve) for pushing the patchset that is in -mm
without the finer performance tuning is large enough for it to be
considered.

But again, you're concerns are valid, there appears to be a lack of
enthusiasm in the community both for and against the changes. And I
understand, as I've got lots of other things I need to do as well, and
reviewing a large change like this can take some time that I'm sure
folks are short on.

Maybe I should work on selling it more, I just have been at it for so
long with this patch set that I feel I'm boring folks with the constant
and repetitive "provides robust behavior in the face of lost ticks and
enables other development like high-res timers and realtime" schtick.

But I guess I'll try to ping some folks individually see if I can't stir
up some discussion and get some additional feedback on the issue.

thanks
-john

Re: [PATCH] simplify update_times (avoid jiffies/jiffies_64 aliasing problem)

[Andrew Morton]

john stultz <johnstul@us.ibm.com> wrote:
>
> But again, you're concerns are valid, there appears to be a lack of
>  enthusiasm in the community both for and against the changes. And I
>  understand, as I've got lots of other things I need to do as well, and
>  reviewing a large change like this can take some time that I'm sure
>  folks are short on.
> 
>  Maybe I should work on selling it more, I just have been at it for so
>  long with this patch set that I feel I'm boring folks with the constant
>  and repetitive "provides robust behavior in the face of lost ticks and
>  enables other development like high-res timers and realtime" schtick.
>

I'm not particularly worried from the will-it-break-the-kernel POV.  Any
remaining problems will affect a relatively small number of machines and
once the proverbial million monkeys start running it, things will be shaken
out of the current x86 implementation.

So we can do this, but the question is do we _want_ to do it?  If the arch
maintainers can look at it from a high level and say "yup, I can use that
and it'll improve/simplify/speedup/reduce my code" then yes, it's worth
making the effort.

It's a hard one.

Re: [PATCH] simplify update_times (avoid jiffies/jiffies_64 aliasing problem)

[Atsushi Nemoto]

>>>>> On Fri, 03 Mar 2006 12:17:28 -0800, john stultz <johnstul@us.ibm.com> said:

john> I'm not opposed to queuing it up as it seems like a logical
john> cleanup. I'd be fine with it going in before my patch, however
john> it still needs to address i386 lost tick compensation.  I worry
john> that addressing that issue before my patchset (which makes the
john> lost tick compensation unnecessary) might be a bit more
john> complex. I think it would be easier going in after my patch. I
john> do think the barrier fix (with a comment) is a good short term
john> fix.

john> Atsushi: Your thoughts?

I agree.  I missed i386 lost tick case and it seems more complex than
x86_64 case.  Your patchset looks to make this cleanup very easy.
Then, here is an updated barrier fix patch.


Add an optimization barrier to prevent prefetching jiffies before
incrementing jiffies_64.

Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>

diff --git a/kernel/timer.c b/kernel/timer.c
index fc6646f..decd19e 100644
--- a/kernel/timer.c
+++ b/kernel/timer.c
@@ -925,6 +925,8 @@ static inline void update_times(void)
 void do_timer(struct pt_regs *regs)
 {
 	jiffies_64++;
+	/* prevent loading jiffies before storing new jiffies_64 value. */
+	barrier();
 	update_times();
 	softlockup_tick(regs);
 }

Re: [PATCH] simplify update_times (avoid jiffies/jiffies_64 aliasing problem)

[Atsushi Nemoto]

Let me restart this discussion after about 5 months interval.

On Sun, 05 Mar 2006 02:15:42 +0900 (JST), Atsushi Nemoto <anemo@mba.ocn.ne.jp> wrote:
> john> I'm not opposed to queuing it up as it seems like a logical
> john> cleanup. I'd be fine with it going in before my patch, however
> john> it still needs to address i386 lost tick compensation.  I worry
> john> that addressing that issue before my patchset (which makes the
> john> lost tick compensation unnecessary) might be a bit more
> john> complex. I think it would be easier going in after my patch. I
> john> do think the barrier fix (with a comment) is a good short term
> john> fix.
> 
> john> Atsushi: Your thoughts?
> 
> I agree.  I missed i386 lost tick case and it seems more complex than
> x86_64 case.  Your patchset looks to make this cleanup very easy.
> Then, here is an updated barrier fix patch.

Now it seems the conversion of the i386 timer code has been finished.
We can think this topic again.  Here is a patch against current git
tree.  How about this?  And I wonder if there are any point
maintaining wall_jiffies now.  It seems jiffies and wall_jiffies are
always synced.


[PATCH] simplify update_times

In kernel 2.6, update_times() is called directly from timer interrupt,
so there is no point calculating ticks here.  This also make a barrier
added by 5aee405c662ca644980c184774277fc6d0769a84 needless.

Also adjust x86_64 timer interrupt handler with this change.

Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>

diff --git a/arch/x86_64/kernel/time.c b/arch/x86_64/kernel/time.c
index 7a9b182..298027f 100644
--- a/arch/x86_64/kernel/time.c
+++ b/arch/x86_64/kernel/time.c
@@ -423,7 +423,8 @@ #endif
 
 	if (lost > 0) {
 		handle_lost_ticks(lost, regs);
-		jiffies += lost;
+		while (lost--)
+			do_timer(regs);
 	}
 
 /*
diff --git a/kernel/timer.c b/kernel/timer.c
index 05809c2..3981cae 100644
--- a/kernel/timer.c
+++ b/kernel/timer.c
@@ -1267,12 +1267,9 @@ void run_local_timers(void)
  */
 static inline void update_times(void)
 {
-	unsigned long ticks;
-
-	ticks = jiffies - wall_jiffies;
-	wall_jiffies += ticks;
+	wall_jiffies++;
 	update_wall_time();
-	calc_load(ticks);
+	calc_load(1);
 }
   
 /*
@@ -1284,8 +1281,6 @@ static inline void update_times(void)
 void do_timer(struct pt_regs *regs)
 {
 	jiffies_64++;
-	/* prevent loading jiffies before storing new jiffies_64 value. */
-	barrier();
 	update_times();
 }
 

Re: [PATCH] simplify update_times (avoid jiffies/jiffies_64 aliasing problem)

[Martin Schwidefsky]

On 7/30/06, Atsushi Nemoto <anemo@mba.ocn.ne.jp> wrote:
> diff --git a/arch/x86_64/kernel/time.c b/arch/x86_64/kernel/time.c
> index 7a9b182..298027f 100644
> --- a/arch/x86_64/kernel/time.c
> +++ b/arch/x86_64/kernel/time.c
> @@ -423,7 +423,8 @@ #endif
>
>         if (lost > 0) {
>                 handle_lost_ticks(lost, regs);
> -               jiffies += lost;
> +               while (lost--)
> +                       do_timer(regs);
>         }
>
>  /*

I think that this is going into the wrong direction. There are a
number of architectures that call do_timer(regs) in a while loop. It
would be much nicer if do_timer would get the number of passed ticks
as an argument. And the "regs" argument to do_timer is just useless.

> diff --git a/kernel/timer.c b/kernel/timer.c
> index 05809c2..3981cae 100644
> --- a/kernel/timer.c
> +++ b/kernel/timer.c
> @@ -1267,12 +1267,9 @@ void run_local_timers(void)
>   */
>  static inline void update_times(void)
>  {
> -       unsigned long ticks;
> -
> -       ticks = jiffies - wall_jiffies;
> -       wall_jiffies += ticks;
> +       wall_jiffies++;
>         update_wall_time();
> -       calc_load(ticks);
> +       calc_load(1);
>  }
>
>  /*

Pass "ticks" from do_timer and do "wall_jiffies += ticks". To make
calc_load work correctly the  "if (count < 0)" in calc_load needs to
be replaced with "while (count < 0)".

> @@ -1284,8 +1281,6 @@ static inline void update_times(void)
>  void do_timer(struct pt_regs *regs)
>  {
>         jiffies_64++;
> -       /* prevent loading jiffies before storing new jiffies_64 value. */
> -       barrier();
>         update_times();
>  }
>

Change do_timer and make architectures to pass "ticks", do "jiffies64
+= ticks" and add ticks to the call of update_times.

-- 
blue skies,
  Martin

Re: [PATCH] simplify update_times (avoid jiffies/jiffies_64 aliasing problem)

[Atsushi Nemoto]

On Mon, 31 Jul 2006 12:36:38 +0200, "Martin Schwidefsky" <schwidefsky@googlemail.com> wrote:
> > --- a/arch/x86_64/kernel/time.c
> > +++ b/arch/x86_64/kernel/time.c
> > @@ -423,7 +423,8 @@ #endif
> >
> >         if (lost > 0) {
> >                 handle_lost_ticks(lost, regs);
> > -               jiffies += lost;
> > +               while (lost--)
> > +                       do_timer(regs);
> >         }
> >
> >  /*
> 
> I think that this is going into the wrong direction. There are a
> number of architectures that call do_timer(regs) in a while loop. It
> would be much nicer if do_timer would get the number of passed ticks
> as an argument. And the "regs" argument to do_timer is just useless.

But normally do_timer() is called just once, isn't it?  These loops
are just for lost ticks, which would be rarely happened.  So I think
tunning for usual case is better.

I agree that the "regs" argument is useless.  Another candidate for
cleanup.

---
Atsushi Nemoto

Re: [PATCH] simplify update_times (avoid jiffies/jiffies_64 aliasing problem)

[Martin Schwidefsky]

On 8/1/06, Atsushi Nemoto <anemo@mba.ocn.ne.jp> wrote:
> > I think that this is going into the wrong direction. There are a
> > number of architectures that call do_timer(regs) in a while loop. It
> > would be much nicer if do_timer would get the number of passed ticks
> > as an argument. And the "regs" argument to do_timer is just useless.
>
> But normally do_timer() is called just once, isn't it?  These loops
> are just for lost ticks, which would be rarely happened.  So I think
> tunning for usual case is better.

If you switch of the hz timer in idle you'll get lots of lost ticks.
And if you are
running a virtualized system you can loose you cpu for some ticks as well.
Pass the number of ticks to do_timer.

-- 
blue skies,
  Martin

Re: [PATCH] simplify update_times (avoid jiffies/jiffies_64 aliasing problem)

[Atsushi Nemoto]

On Wed, 2 Aug 2006 14:50:32 +0200, "Martin Schwidefsky" <schwidefsky@googlemail.com> wrote:
> If you switch of the hz timer in idle you'll get lots of lost ticks.
> And if you are
> running a virtualized system you can loose you cpu for some ticks as well.
> Pass the number of ticks to do_timer.

I see.  Then how about this?


[PATCH] cleanup do_timer and update_times

Rename do_timer() to do_timer_ticks() and pass ticks.  Now do_timer()
is just wrapper of do_timer_ticks().

This also make a barrier added by
5aee405c662ca644980c184774277fc6d0769a84 needless.

Also adjust x86_64 timer interrupt handler with this change.

Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>

diff --git a/arch/x86_64/kernel/time.c b/arch/x86_64/kernel/time.c
index 7a9b182..3dbe30e 100644
--- a/arch/x86_64/kernel/time.c
+++ b/arch/x86_64/kernel/time.c
@@ -421,16 +421,14 @@ #endif
 				(((long) offset << US_SCALE) / vxtime.tsc_quot) - 1;
 	}
 
-	if (lost > 0) {
+	if (lost > 0)
 		handle_lost_ticks(lost, regs);
-		jiffies += lost;
-	}
 
 /*
  * Do the timer stuff.
  */
 
-	do_timer(regs);
+	do_timer_ticks(lost + 1);
 #ifndef CONFIG_SMP
 	update_process_times(user_mode(regs));
 #endif
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 6afa72e..5d7dfa1 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1180,7 +1180,8 @@ extern void switch_uid(struct user_struc
 
 #include <asm/current.h>
 
-extern void do_timer(struct pt_regs *);
+extern void do_timer_ticks(unsigned long ticks);
+#define do_timer(regs)	do_timer_ticks(1)
 
 extern int FASTCALL(wake_up_state(struct task_struct * tsk, unsigned int state));
 extern int FASTCALL(wake_up_process(struct task_struct * tsk));
diff --git a/kernel/timer.c b/kernel/timer.c
index b650f04..50ed235 100644
--- a/kernel/timer.c
+++ b/kernel/timer.c
@@ -1218,7 +1218,7 @@ static inline void calc_load(unsigned lo
 	static int count = LOAD_FREQ;
 
 	count -= ticks;
-	if (count < 0) {
+	while (count < 0) {
 		count += LOAD_FREQ;
 		active_tasks = count_active_tasks();
 		CALC_LOAD(avenrun[0], EXP_1, active_tasks);
@@ -1265,11 +1265,8 @@ void run_local_timers(void)
  * Called by the timer interrupt. xtime_lock must already be taken
  * by the timer IRQ!
  */
-static inline void update_times(void)
+static inline void update_times(unsigned long ticks)
 {
-	unsigned long ticks;
-
-	ticks = jiffies - wall_jiffies;
 	wall_jiffies += ticks;
 	update_wall_time();
 	calc_load(ticks);
@@ -1281,12 +1278,10 @@ static inline void update_times(void)
  * jiffies is defined in the linker script...
  */
 
-void do_timer(struct pt_regs *regs)
+void do_timer_ticks(unsigned long ticks)
 {
-	jiffies_64++;
-	/* prevent loading jiffies before storing new jiffies_64 value. */
-	barrier();
-	update_times();
+	jiffies_64 += ticks;
+	update_times(ticks);
 }
 
 #ifdef __ARCH_WANT_SYS_ALARM

Re: [PATCH] simplify update_times (avoid jiffies/jiffies_64 aliasing problem)

[Martin Schwidefsky]

On 8/3/06, Atsushi Nemoto <anemo@mba.ocn.ne.jp> wrote:
> I see.  Then how about this?
>
>
> [PATCH] cleanup do_timer and update_times
> ...

Good start, now you only have the change the 30+ calls to do_timer in
the various architecture backends.

-- 
blue skies,
  Martin

Re: [PATCH] simplify update_times (avoid jiffies/jiffies_64 aliasing problem)

[Atsushi Nemoto]

On Fri, 4 Aug 2006 16:02:43 +0200, "Martin Schwidefsky" <schwidefsky@googlemail.com> wrote:
> Good start, now you only have the change the 30+ calls to do_timer in
> the various architecture backends.

OK, then this is a patch contains the changes.
Adding S390 maintainer Martin Schwidefsky to CC.

This patch is against current git tree, so does not contains a change
to arch/avr32 which is in mm tree.  I can create a patch against mm
tree if expected.


[PATCH] cleanup do_timer and update_times

Pass ticks to do_timer() and update_times().

This also make a barrier added by
5aee405c662ca644980c184774277fc6d0769a84 needless.

Also adjust x86_64 and s390 timer interrupt handler with this change.

Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>

 arch/alpha/kernel/time.c                 |    2 +-
 arch/arm/kernel/time.c                   |    2 +-
 arch/arm26/kernel/time.c                 |    2 +-
 arch/cris/arch-v10/kernel/time.c         |    2 +-
 arch/cris/arch-v32/kernel/time.c         |    2 +-
 arch/frv/kernel/time.c                   |    2 +-
 arch/h8300/kernel/time.c                 |    2 +-
 arch/ia64/kernel/time.c                  |    2 +-
 arch/m32r/kernel/time.c                  |    2 +-
 arch/m68k/kernel/time.c                  |    2 +-
 arch/m68k/sun3/sun3ints.c                |    2 +-
 arch/m68knommu/kernel/time.c             |    2 +-
 arch/mips/au1000/common/time.c           |    6 +++---
 arch/mips/galileo-boards/ev96100/time.c  |    2 +-
 arch/mips/gt64120/common/time.c          |    2 +-
 arch/mips/kernel/time.c                  |    2 +-
 arch/mips/momentum/ocelot_g/gt-irq.c     |    2 +-
 arch/mips/sgi-ip27/ip27-timer.c          |    2 +-
 arch/parisc/kernel/time.c                |    2 +-
 arch/powerpc/kernel/time.c               |    2 +-
 arch/ppc/kernel/time.c                   |    2 +-
 arch/s390/kernel/time.c                  |    9 ++++-----
 arch/sh/kernel/time.c                    |    2 +-
 arch/sh64/kernel/time.c                  |    2 +-
 arch/sparc/kernel/pcic.c                 |    2 +-
 arch/sparc/kernel/time.c                 |    2 +-
 arch/sparc64/kernel/time.c               |    4 ++--
 arch/um/kernel/time.c                    |    2 +-
 arch/v850/kernel/time.c                  |    2 +-
 arch/x86_64/kernel/time.c                |    6 ++----
 arch/xtensa/kernel/time.c                |    2 +-
 include/asm-arm/arch-clps711x/time.h     |    2 +-
 include/asm-arm/arch-l7200/time.h        |    2 +-
 include/asm-i386/mach-default/do_timer.h |    2 +-
 include/asm-i386/mach-visws/do_timer.h   |    2 +-
 include/asm-i386/mach-voyager/do_timer.h |    2 +-
 include/linux/sched.h                    |    2 +-
 kernel/timer.c                           |   15 +++++----------
 38 files changed, 49 insertions(+), 57 deletions(-)

diff --git a/arch/alpha/kernel/time.c b/arch/alpha/kernel/time.c
index b191cc7..7c1e444 100644
--- a/arch/alpha/kernel/time.c
+++ b/arch/alpha/kernel/time.c
@@ -132,7 +132,7 @@ #endif
 	nticks = delta >> FIX_SHIFT;
 
 	while (nticks > 0) {
-		do_timer(regs);
+		do_timer(1);
 #ifndef CONFIG_SMP
 		update_process_times(user_mode(regs));
 #endif
diff --git a/arch/arm/kernel/time.c b/arch/arm/kernel/time.c
index 09a67d7..4892a1f 100644
--- a/arch/arm/kernel/time.c
+++ b/arch/arm/kernel/time.c
@@ -333,7 +333,7 @@ void timer_tick(struct pt_regs *regs)
 	profile_tick(CPU_PROFILING, regs);
 	do_leds();
 	do_set_rtc();
-	do_timer(regs);
+	do_timer(1);
 #ifndef CONFIG_SMP
 	update_process_times(user_mode(regs));
 #endif
diff --git a/arch/arm26/kernel/time.c b/arch/arm26/kernel/time.c
index db63d75..80adbd0 100644
--- a/arch/arm26/kernel/time.c
+++ b/arch/arm26/kernel/time.c
@@ -194,7 +194,7 @@ EXPORT_SYMBOL(do_settimeofday);
 
 static irqreturn_t timer_interrupt(int irq, void *dev_id, struct pt_regs *regs)
 {
-        do_timer(regs);
+        do_timer(1);
 #ifndef CONFIG_SMP
 	update_process_times(user_mode(regs));
 #endif
diff --git a/arch/cris/arch-v10/kernel/time.c b/arch/cris/arch-v10/kernel/time.c
index 9c22b76..ebacf14 100644
--- a/arch/cris/arch-v10/kernel/time.c
+++ b/arch/cris/arch-v10/kernel/time.c
@@ -227,7 +227,7 @@ #endif
 	
 	/* call the real timer interrupt handler */
 
-	do_timer(regs);
+	do_timer(1);
 	
         cris_do_profile(regs); /* Save profiling information */
 
diff --git a/arch/cris/arch-v32/kernel/time.c b/arch/cris/arch-v32/kernel/time.c
index 50f3f93..be0a016 100644
--- a/arch/cris/arch-v32/kernel/time.c
+++ b/arch/cris/arch-v32/kernel/time.c
@@ -219,7 +219,7 @@ timer_interrupt(int irq, void *dev_id, s
 		return IRQ_HANDLED;
 
 	/* call the real timer interrupt handler */
-	do_timer(regs);
+	do_timer(1);
 
 	/*
 	 * If we have an externally synchronized Linux clock, then update
diff --git a/arch/frv/kernel/time.c b/arch/frv/kernel/time.c
index d5b64e1..77d18f6 100644
--- a/arch/frv/kernel/time.c
+++ b/arch/frv/kernel/time.c
@@ -73,7 +73,7 @@ static irqreturn_t timer_interrupt(int i
 	 */
 	write_seqlock(&xtime_lock);
 
-	do_timer(regs);
+	do_timer(1);
 	update_process_times(user_mode(regs));
 	profile_tick(CPU_PROFILING, regs);
 
diff --git a/arch/h8300/kernel/time.c b/arch/h8300/kernel/time.c
index 688a510..e569d17 100644
--- a/arch/h8300/kernel/time.c
+++ b/arch/h8300/kernel/time.c
@@ -41,7 +41,7 @@ static void timer_interrupt(int irq, voi
 	/* may need to kick the hardware timer */
 	platform_timer_eoi();
 
-	do_timer(regs);
+	do_timer(1);
 #ifndef CONFIG_SMP
 	update_process_times(user_mode(regs));
 #endif
diff --git a/arch/ia64/kernel/time.c b/arch/ia64/kernel/time.c
index 6928ef0..1626268 100644
--- a/arch/ia64/kernel/time.c
+++ b/arch/ia64/kernel/time.c
@@ -78,7 +78,7 @@ timer_interrupt (int irq, void *dev_id, 
 			 * xtime_lock.
 			 */
 			write_seqlock(&xtime_lock);
-			do_timer(regs);
+			do_timer(1);
 			local_cpu_data->itm_next = new_itm;
 			write_sequnlock(&xtime_lock);
 		} else
diff --git a/arch/m32r/kernel/time.c b/arch/m32r/kernel/time.c
index ded0be0..7a89689 100644
--- a/arch/m32r/kernel/time.c
+++ b/arch/m32r/kernel/time.c
@@ -202,7 +202,7 @@ irqreturn_t timer_interrupt(int irq, voi
 #ifndef CONFIG_SMP
 	profile_tick(CPU_PROFILING, regs);
 #endif
-	do_timer(regs);
+	do_timer(1);
 
 #ifndef CONFIG_SMP
 	update_process_times(user_mode(regs));
diff --git a/arch/m68k/kernel/time.c b/arch/m68k/kernel/time.c
index 98e4b1a..1072e49 100644
--- a/arch/m68k/kernel/time.c
+++ b/arch/m68k/kernel/time.c
@@ -40,7 +40,7 @@ static inline int set_rtc_mmss(unsigned 
  */
 static irqreturn_t timer_interrupt(int irq, void *dummy, struct pt_regs * regs)
 {
-	do_timer(regs);
+	do_timer(1);
 #ifndef CONFIG_SMP
 	update_process_times(user_mode(regs));
 #endif
diff --git a/arch/m68k/sun3/sun3ints.c b/arch/m68k/sun3/sun3ints.c
index f18b9d3..dc4ea7e 100644
--- a/arch/m68k/sun3/sun3ints.c
+++ b/arch/m68k/sun3/sun3ints.c
@@ -65,7 +65,7 @@ #endif
 #ifdef CONFIG_SUN3
 	intersil_clear();
 #endif
-        do_timer(fp);
+        do_timer(1);
 #ifndef CONFIG_SMP
 	update_process_times(user_mode(fp));
 #endif
diff --git a/arch/m68knommu/kernel/time.c b/arch/m68knommu/kernel/time.c
index 1db9872..db1e1ce 100644
--- a/arch/m68knommu/kernel/time.c
+++ b/arch/m68knommu/kernel/time.c
@@ -51,7 +51,7 @@ static irqreturn_t timer_interrupt(int i
 
 	write_seqlock(&xtime_lock);
 
-	do_timer(regs);
+	do_timer(1);
 #ifndef CONFIG_SMP
 	update_process_times(user_mode(regs));
 #endif
diff --git a/arch/mips/au1000/common/time.c b/arch/mips/au1000/common/time.c
index 7fbea1b..0a067f3 100644
--- a/arch/mips/au1000/common/time.c
+++ b/arch/mips/au1000/common/time.c
@@ -96,7 +96,7 @@ void mips_timer_interrupt(struct pt_regs
 		timerlo = count;
 
 		kstat_this_cpu.irqs[irq]++;
-		do_timer(regs);
+		do_timer(1);
 #ifndef CONFIG_SMP
 		update_process_times(user_mode(regs));
 #endif
@@ -137,7 +137,7 @@ irqreturn_t counter0_irq(int irq, void *
 	}
 
 	while (time_elapsed > 0) {
-		do_timer(regs);
+		do_timer(1);
 #ifndef CONFIG_SMP
 		update_process_times(user_mode(regs));
 #endif
@@ -156,7 +156,7 @@ #endif
 
 	if (jiffie_drift >= 999) {
 		jiffie_drift -= 999;
-		do_timer(regs); /* increment jiffies by one */
+		do_timer(1); /* increment jiffies by one */
 #ifndef CONFIG_SMP
 		update_process_times(user_mode(regs));
 #endif
diff --git a/arch/mips/galileo-boards/ev96100/time.c b/arch/mips/galileo-boards/ev96100/time.c
index 8cbe842..60c564b 100644
--- a/arch/mips/galileo-boards/ev96100/time.c
+++ b/arch/mips/galileo-boards/ev96100/time.c
@@ -72,7 +72,7 @@ void mips_timer_interrupt(struct pt_regs
 
 	do {
 		kstat_this_cpu.irqs[irq]++;
-		do_timer(regs);
+		do_timer(1);
 #ifndef CONFIG_SMP
 		update_process_times(user_mode(regs));
 #endif
diff --git a/arch/mips/gt64120/common/time.c b/arch/mips/gt64120/common/time.c
index d837b26..7feca49 100644
--- a/arch/mips/gt64120/common/time.c
+++ b/arch/mips/gt64120/common/time.c
@@ -34,7 +34,7 @@ static void gt64120_irq(int irq, void *d
 	if (irq_src & 0x00000800) {	/* Check for timer interrupt */
 		handled = 1;
 		irq_src &= ~0x00000800;
-		do_timer(regs);
+		do_timer(1);
 #ifndef CONFIG_SMP
 		update_process_times(user_mode(regs));
 #endif
diff --git a/arch/mips/kernel/time.c b/arch/mips/kernel/time.c
index 170cb67..6ab8d97 100644
--- a/arch/mips/kernel/time.c
+++ b/arch/mips/kernel/time.c
@@ -434,7 +434,7 @@ irqreturn_t timer_interrupt(int irq, voi
 	/*
 	 * call the generic timer interrupt handling
 	 */
-	do_timer(regs);
+	do_timer(1);
 
 	/*
 	 * If we have an externally synchronized Linux clock, then update
diff --git a/arch/mips/momentum/ocelot_g/gt-irq.c b/arch/mips/momentum/ocelot_g/gt-irq.c
index 9fb2493..6cd87cf 100644
--- a/arch/mips/momentum/ocelot_g/gt-irq.c
+++ b/arch/mips/momentum/ocelot_g/gt-irq.c
@@ -133,7 +133,7 @@ static irqreturn_t gt64240_p0int_irq(int
 		MV_WRITE(TIMER_COUNTER_0_3_INTERRUPT_CAUSE, 0x0);
 
 		/* handle the timer call */
-		do_timer(regs);
+		do_timer(1);
 #ifndef CONFIG_SMP
 		update_process_times(user_mode(regs));
 #endif
diff --git a/arch/mips/sgi-ip27/ip27-timer.c b/arch/mips/sgi-ip27/ip27-timer.c
index b029ba7..c62a3a9 100644
--- a/arch/mips/sgi-ip27/ip27-timer.c
+++ b/arch/mips/sgi-ip27/ip27-timer.c
@@ -111,7 +111,7 @@ again:
 	kstat_this_cpu.irqs[irq]++;		/* kstat only for bootcpu? */
 
 	if (cpu == 0)
-		do_timer(regs);
+		do_timer(1);
 
 	update_process_times(user_mode(regs));
 
diff --git a/arch/parisc/kernel/time.c b/arch/parisc/kernel/time.c
index 5facc9b..700df10 100644
--- a/arch/parisc/kernel/time.c
+++ b/arch/parisc/kernel/time.c
@@ -79,7 +79,7 @@ #else
 #endif
 		if (cpu == 0) {
 			write_seqlock(&xtime_lock);
-			do_timer(regs);
+			do_timer(1);
 			write_sequnlock(&xtime_lock);
 		}
 	}
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index 774c0a3..914178c 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -693,7 +693,7 @@ #endif
 		write_seqlock(&xtime_lock);
 		tb_last_jiffy += tb_ticks_per_jiffy;
 		tb_last_stamp = per_cpu(last_jiffy, cpu);
-		do_timer(regs);
+		do_timer(1);
 		timer_recalc_offset(tb_last_jiffy);
 		timer_check_rtc();
 		write_sequnlock(&xtime_lock);
diff --git a/arch/ppc/kernel/time.c b/arch/ppc/kernel/time.c
index 6ab8cc7..1e1f315 100644
--- a/arch/ppc/kernel/time.c
+++ b/arch/ppc/kernel/time.c
@@ -153,7 +153,7 @@ void timer_interrupt(struct pt_regs * re
 		/* We are in an interrupt, no need to save/restore flags */
 		write_seqlock(&xtime_lock);
 		tb_last_stamp = jiffy_stamp;
-		do_timer(regs);
+		do_timer(1);
 
 		/*
 		 * update the rtc when needed, this should be performed on the
diff --git a/arch/s390/kernel/time.c b/arch/s390/kernel/time.c
index 74e6178..ade3f07 100644
--- a/arch/s390/kernel/time.c
+++ b/arch/s390/kernel/time.c
@@ -166,7 +166,7 @@ #endif /* CONFIG_PROFILING */
 void account_ticks(struct pt_regs *regs)
 {
 	__u64 tmp;
-	__u32 ticks, xticks;
+	__u32 ticks;
 
 	/* Calculate how many ticks have passed. */
 	if (S390_lowcore.int_clock < S390_lowcore.jiffy_timer) {
@@ -204,6 +204,7 @@ #ifdef CONFIG_SMP
 	 */
 	write_seqlock(&xtime_lock);
 	if (S390_lowcore.jiffy_timer > xtime_cc) {
+		__u32 xticks;
 		tmp = S390_lowcore.jiffy_timer - xtime_cc;
 		if (tmp >= 2*CLK_TICKS_PER_JIFFY) {
 			xticks = __div(tmp, CLK_TICKS_PER_JIFFY);
@@ -212,13 +213,11 @@ #ifdef CONFIG_SMP
 			xticks = 1;
 			xtime_cc += CLK_TICKS_PER_JIFFY;
 		}
-		while (xticks--)
-			do_timer(regs);
+		do_timer(xticks);
 	}
 	write_sequnlock(&xtime_lock);
 #else
-	for (xticks = ticks; xticks > 0; xticks--)
-		do_timer(regs);
+	do_timer(ticks);
 #endif
 
 #ifdef CONFIG_VIRT_CPU_ACCOUNTING
diff --git a/arch/sh/kernel/time.c b/arch/sh/kernel/time.c
index a1589f8..9351ee9 100644
--- a/arch/sh/kernel/time.c
+++ b/arch/sh/kernel/time.c
@@ -115,7 +115,7 @@ static long last_rtc_update;
  */
 void handle_timer_tick(struct pt_regs *regs)
 {
-	do_timer(regs);
+	do_timer(1);
 #ifndef CONFIG_SMP
 	update_process_times(user_mode(regs));
 #endif
diff --git a/arch/sh64/kernel/time.c b/arch/sh64/kernel/time.c
index b8162e5..3b61e06 100644
--- a/arch/sh64/kernel/time.c
+++ b/arch/sh64/kernel/time.c
@@ -298,7 +298,7 @@ static inline void do_timer_interrupt(in
 	asm ("getcon cr62, %0" : "=r" (current_ctc));
 	ctc_last_interrupt = (unsigned long) current_ctc;
 
-	do_timer(regs);
+	do_timer(1);
 #ifndef CONFIG_SMP
 	update_process_times(user_mode(regs));
 #endif
diff --git a/arch/sparc/kernel/pcic.c b/arch/sparc/kernel/pcic.c
index bfd31aa..e19b1ba 100644
--- a/arch/sparc/kernel/pcic.c
+++ b/arch/sparc/kernel/pcic.c
@@ -712,7 +712,7 @@ static irqreturn_t pcic_timer_handler (i
 {
 	write_seqlock(&xtime_lock);	/* Dummy, to show that we remember */
 	pcic_clear_clock_irq();
-	do_timer(regs);
+	do_timer(1);
 #ifndef CONFIG_SMP
 	update_process_times(user_mode(regs));
 #endif
diff --git a/arch/sparc/kernel/time.c b/arch/sparc/kernel/time.c
index 845081b..6f84fa1 100644
--- a/arch/sparc/kernel/time.c
+++ b/arch/sparc/kernel/time.c
@@ -128,7 +128,7 @@ #ifdef CONFIG_SUN4
 #endif
 	clear_clock_irq();
 
-	do_timer(regs);
+	do_timer(1);
 #ifndef CONFIG_SMP
 	update_process_times(user_mode(regs));
 #endif
diff --git a/arch/sparc64/kernel/time.c b/arch/sparc64/kernel/time.c
index 094d3e3..f5ed96f 100644
--- a/arch/sparc64/kernel/time.c
+++ b/arch/sparc64/kernel/time.c
@@ -465,7 +465,7 @@ #ifndef CONFIG_SMP
 		profile_tick(CPU_PROFILING, regs);
 		update_process_times(user_mode(regs));
 #endif
-		do_timer(regs);
+		do_timer(1);
 
 		/* Guarantee that the following sequences execute
 		 * uninterrupted.
@@ -496,7 +496,7 @@ void timer_tick_interrupt(struct pt_regs
 {
 	write_seqlock(&xtime_lock);
 
-	do_timer(regs);
+	do_timer(1);
 
 	timer_check_rtc();
 
diff --git a/arch/um/kernel/time.c b/arch/um/kernel/time.c
index 552ca1c..2e2caf0 100644
--- a/arch/um/kernel/time.c
+++ b/arch/um/kernel/time.c
@@ -96,7 +96,7 @@ irqreturn_t um_timer(int irq, void *dev,
 
 	write_seqlock_irqsave(&xtime_lock, flags);
 
-	do_timer(regs);
+	do_timer(1);
 
 	nsecs = get_time() + local_offset;
 	xtime.tv_sec = nsecs / NSEC_PER_SEC;
diff --git a/arch/v850/kernel/time.c b/arch/v850/kernel/time.c
index a0b4669..f4d1a4d 100644
--- a/arch/v850/kernel/time.c
+++ b/arch/v850/kernel/time.c
@@ -51,7 +51,7 @@ #endif
 	if (mach_tick)
 	  mach_tick ();
 
-	do_timer (regs);
+	do_timer (1);
 #ifndef CONFIG_SMP
 	update_process_times(user_mode(regs));
 #endif
diff --git a/arch/x86_64/kernel/time.c b/arch/x86_64/kernel/time.c
index 7a9b182..fdd9e98 100644
--- a/arch/x86_64/kernel/time.c
+++ b/arch/x86_64/kernel/time.c
@@ -421,16 +421,14 @@ #endif
 				(((long) offset << US_SCALE) / vxtime.tsc_quot) - 1;
 	}
 
-	if (lost > 0) {
+	if (lost > 0)
 		handle_lost_ticks(lost, regs);
-		jiffies += lost;
-	}
 
 /*
  * Do the timer stuff.
  */
 
-	do_timer(regs);
+	do_timer(lost + 1);
 #ifndef CONFIG_SMP
 	update_process_times(user_mode(regs));
 #endif
diff --git a/arch/xtensa/kernel/time.c b/arch/xtensa/kernel/time.c
index 412ab32..241db20 100644
--- a/arch/xtensa/kernel/time.c
+++ b/arch/xtensa/kernel/time.c
@@ -175,7 +175,7 @@ #endif
 
 		last_ccount_stamp = next;
 		next += CCOUNT_PER_JIFFY;
-		do_timer (regs); /* Linux handler in kernel/timer.c */
+		do_timer (1); /* Linux handler in kernel/timer.c */
 
 		if (ntp_synced() &&
 		    xtime.tv_sec - last_rtc_update >= 659 &&
diff --git a/include/asm-arm/arch-clps711x/time.h b/include/asm-arm/arch-clps711x/time.h
index 9cb27cd..0e4a390 100644
--- a/include/asm-arm/arch-clps711x/time.h
+++ b/include/asm-arm/arch-clps711x/time.h
@@ -29,7 +29,7 @@ static irqreturn_t
 p720t_timer_interrupt(int irq, void *dev_id, struct pt_regs *regs)
 {
 	do_leds();
-	do_timer(regs);
+	do_timer(1);
 #ifndef CONFIG_SMP
 	update_process_times(user_mode(regs));
 #endif
diff --git a/include/asm-arm/arch-l7200/time.h b/include/asm-arm/arch-l7200/time.h
index 7b98b53..c69cb50 100644
--- a/include/asm-arm/arch-l7200/time.h
+++ b/include/asm-arm/arch-l7200/time.h
@@ -45,7 +45,7 @@ #define RTC_EN_STWDOG	0x08      /* Enabl
 static irqreturn_t
 timer_interrupt(int irq, void *dev_id, struct pt_regs *regs)
 {
-	do_timer(regs);
+	do_timer(1);
 #ifndef CONFIG_SMP
 	update_process_times(user_mode(regs));
 #endif
diff --git a/include/asm-i386/mach-default/do_timer.h b/include/asm-i386/mach-default/do_timer.h
index 6312c3e..4182c34 100644
--- a/include/asm-i386/mach-default/do_timer.h
+++ b/include/asm-i386/mach-default/do_timer.h
@@ -16,7 +16,7 @@ #include <asm/i8259.h>
 
 static inline void do_timer_interrupt_hook(struct pt_regs *regs)
 {
-	do_timer(regs);
+	do_timer(1);
 #ifndef CONFIG_SMP
 	update_process_times(user_mode_vm(regs));
 #endif
diff --git a/include/asm-i386/mach-visws/do_timer.h b/include/asm-i386/mach-visws/do_timer.h
index 95568e6..8db618c 100644
--- a/include/asm-i386/mach-visws/do_timer.h
+++ b/include/asm-i386/mach-visws/do_timer.h
@@ -9,7 +9,7 @@ static inline void do_timer_interrupt_ho
 	/* Clear the interrupt */
 	co_cpu_write(CO_CPU_STAT,co_cpu_read(CO_CPU_STAT) & ~CO_STAT_TIMEINTR);
 
-	do_timer(regs);
+	do_timer(1);
 #ifndef CONFIG_SMP
 	update_process_times(user_mode_vm(regs));
 #endif
diff --git a/include/asm-i386/mach-voyager/do_timer.h b/include/asm-i386/mach-voyager/do_timer.h
index eaf5180..099fe9f 100644
--- a/include/asm-i386/mach-voyager/do_timer.h
+++ b/include/asm-i386/mach-voyager/do_timer.h
@@ -3,7 +3,7 @@ #include <asm/voyager.h>
 
 static inline void do_timer_interrupt_hook(struct pt_regs *regs)
 {
-	do_timer(regs);
+	do_timer(1);
 #ifndef CONFIG_SMP
 	update_process_times(user_mode_vm(regs));
 #endif
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 6afa72e..8de3d91 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1180,7 +1180,7 @@ extern void switch_uid(struct user_struc
 
 #include <asm/current.h>
 
-extern void do_timer(struct pt_regs *);
+extern void do_timer(unsigned long ticks);
 
 extern int FASTCALL(wake_up_state(struct task_struct * tsk, unsigned int state));
 extern int FASTCALL(wake_up_process(struct task_struct * tsk));
diff --git a/kernel/timer.c b/kernel/timer.c
index b650f04..be7d885 100644
--- a/kernel/timer.c
+++ b/kernel/timer.c
@@ -1218,7 +1218,7 @@ static inline void calc_load(unsigned lo
 	static int count = LOAD_FREQ;
 
 	count -= ticks;
-	if (count < 0) {
+	while (count < 0) {
 		count += LOAD_FREQ;
 		active_tasks = count_active_tasks();
 		CALC_LOAD(avenrun[0], EXP_1, active_tasks);
@@ -1265,11 +1265,8 @@ void run_local_timers(void)
  * Called by the timer interrupt. xtime_lock must already be taken
  * by the timer IRQ!
  */
-static inline void update_times(void)
+static inline void update_times(unsigned long ticks)
 {
-	unsigned long ticks;
-
-	ticks = jiffies - wall_jiffies;
 	wall_jiffies += ticks;
 	update_wall_time();
 	calc_load(ticks);
@@ -1281,12 +1278,10 @@ static inline void update_times(void)
  * jiffies is defined in the linker script...
  */
 
-void do_timer(struct pt_regs *regs)
+void do_timer(unsigned long ticks)
 {
-	jiffies_64++;
-	/* prevent loading jiffies before storing new jiffies_64 value. */
-	barrier();
-	update_times();
+	jiffies_64 += ticks;
+	update_times(ticks);
 }
 
 #ifdef __ARCH_WANT_SYS_ALARM

Re: [PATCH] simplify update_times (avoid jiffies/jiffies_64 aliasing problem)

[Martin Schwidefsky]

On Mon, 2006-08-07 at 01:13 +0900, Atsushi Nemoto wrote:
> On Fri, 4 Aug 2006 16:02:43 +0200, "Martin Schwidefsky" <schwidefsky@googlemail.com> wrote:
> > Good start, now you only have the change the 30+ calls to do_timer in
> > the various architecture backends.
> 
> OK, then this is a patch contains the changes.
> Adding S390 maintainer Martin Schwidefsky to CC.
> 
> This patch is against current git tree, so does not contains a change
> to arch/avr32 which is in mm tree.  I can create a patch against mm
> tree if expected.
> 
> 
> [PATCH] cleanup do_timer and update_times
> 
> Pass ticks to do_timer() and update_times().
> 
> This also make a barrier added by
> 5aee405c662ca644980c184774277fc6d0769a84 needless.
> 
> Also adjust x86_64 and s390 timer interrupt handler with this change.
> 
> Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>

Acked-by: Martin Schwidefsky <schwidefsky@de.ibm.com>

-- 
blue skies,
  Martin.

Martin Schwidefsky
Linux for zSeries Development & Services
IBM Deutschland Entwicklung GmbH

"Reality continues to ruin my life." - Calvin.

Re: [PATCH] simplify update_times (avoid jiffies/jiffies_64 aliasing problem)

[Andrew Morton]

On Mon, 07 Aug 2006 01:13:19 +0900 (JST)
Atsushi Nemoto <anemo@mba.ocn.ne.jp> wrote:

> On Fri, 4 Aug 2006 16:02:43 +0200, "Martin Schwidefsky" <schwidefsky@googlemail.com> wrote:
> > Good start, now you only have the change the 30+ calls to do_timer in
> > the various architecture backends.
> 
> OK, then this is a patch contains the changes.
> Adding S390 maintainer Martin Schwidefsky to CC.
> 
> This patch is against current git tree, so does not contains a change
> to arch/avr32 which is in mm tree.  I can create a patch against mm
> tree if expected.
> 
> 
> [PATCH] cleanup do_timer and update_times
> 
> Pass ticks to do_timer() and update_times().
> 
> This also make a barrier added by
> 5aee405c662ca644980c184774277fc6d0769a84 needless.
> 
> Also adjust x86_64 and s390 timer interrupt handler with this change.
> 

This is a rather terse description for a change of this nature..

Why was this patch created?  What problem is it solving?  etcetera.

> ...
>
> --- a/kernel/timer.c
> +++ b/kernel/timer.c
> @@ -1218,7 +1218,7 @@ static inline void calc_load(unsigned lo
>  	static int count = LOAD_FREQ;
>  
>  	count -= ticks;
> -	if (count < 0) {
> +	while (count < 0) {
>  		count += LOAD_FREQ;
>  		active_tasks = count_active_tasks();
>  		CALC_LOAD(avenrun[0], EXP_1, active_tasks);

OK, we do need the loop here to get the arithmetic in CALC_LOAD to work
correctly.

But I don't think the expensive count_active_tasks() needs to be evaluated
each time around.

Re: [PATCH] simplify update_times (avoid jiffies/jiffies_64 aliasing problem)

[Martin Schwidefsky]

On Mon, 2006-08-07 at 12:58 -0700, Andrew Morton wrote:
> > [PATCH] cleanup do_timer and update_times
> > 
> > Pass ticks to do_timer() and update_times().
> > 
> > This also make a barrier added by
> > 5aee405c662ca644980c184774277fc6d0769a84 needless.
> > 
> > Also adjust x86_64 and s390 timer interrupt handler with this change.
> > 
> 
> This is a rather terse description for a change of this nature..
> 
> Why was this patch created?  What problem is it solving?  etcetera.

The problem is that we are wasting time calling do_timer in a loop. This
is especially true on system that switch off the timer interrupt while
the cpu is idle. You can easily have thousands of lost ticks.

> > ...
> >
> > --- a/kernel/timer.c
> > +++ b/kernel/timer.c
> > @@ -1218,7 +1218,7 @@ static inline void calc_load(unsigned lo
> >  	static int count = LOAD_FREQ;
> >  
> >  	count -= ticks;
> > -	if (count < 0) {
> > +	while (count < 0) {
> >  		count += LOAD_FREQ;
> >  		active_tasks = count_active_tasks();
> >  		CALC_LOAD(avenrun[0], EXP_1, active_tasks);
> 
> OK, we do need the loop here to get the arithmetic in CALC_LOAD to work
> correctly.
> 
> But I don't think the expensive count_active_tasks() needs to be evaluated
> each time around.

This is what I hoped would happen. The loops gets pushed through the
layers to the place where it is actually needed. The next step could be
to get rid of the loop altogether by changing CALC_LOAD.

-- 
blue skies,
  Martin.

Martin Schwidefsky
Linux for zSeries Development & Services
IBM Deutschland Entwicklung GmbH

"Reality continues to ruin my life." - Calvin.

Re: [PATCH] simplify update_times (avoid jiffies/jiffies_64 aliasing problem)

[Atsushi Nemoto]

On Mon, 7 Aug 2006 12:58:10 -0700, Andrew Morton <akpm@osdl.org> wrote:
> > [PATCH] cleanup do_timer and update_times
> > 
> > Pass ticks to do_timer() and update_times().
> > 
> > This also make a barrier added by
> > 5aee405c662ca644980c184774277fc6d0769a84 needless.
> > 
> > Also adjust x86_64 and s390 timer interrupt handler with this change.
> > 
> 
> This is a rather terse description for a change of this nature..
> 
> Why was this patch created?  What problem is it solving?  etcetera.

Thanks.  Updated with Martin's comment.  Also cleanup calc_load() a
bit more.


[PATCH] cleanup do_timer and update_times

Pass ticks to do_timer() and update_times(), and adjust x86_64 and
s390 timer interrupt handler with this change.

Currently update_times() calculates ticks by "jiffies - wall_jiffies",
but callers of do_timer() should know how many ticks to update.
Passing ticks get rid of this redundant calculation.  Also there are
another redundancy pointed out by Martin Schwidefsky.

>From Martin Schwidefsky:
> The problem is that we are wasting time calling do_timer in a loop. This
> is especially true on system that switch off the timer interrupt while
> the cpu is idle. You can easily have thousands of lost ticks.

This cleanup make a barrier added by
5aee405c662ca644980c184774277fc6d0769a84 needless.  So this patch
removes it.

As a bonus, this cleanup make wall_jiffies can be removed easily,
since now wall_jiffies is always synced with jiffies.  (This patch
does not really remove wall_jiffies.  It would be another cleanup
patch)

Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>

 arch/alpha/kernel/time.c                 |    2 +-
 arch/arm/kernel/time.c                   |    2 +-
 arch/arm26/kernel/time.c                 |    2 +-
 arch/cris/arch-v10/kernel/time.c         |    2 +-
 arch/cris/arch-v32/kernel/time.c         |    2 +-
 arch/frv/kernel/time.c                   |    2 +-
 arch/h8300/kernel/time.c                 |    2 +-
 arch/ia64/kernel/time.c                  |    2 +-
 arch/m32r/kernel/time.c                  |    2 +-
 arch/m68k/kernel/time.c                  |    2 +-
 arch/m68k/sun3/sun3ints.c                |    2 +-
 arch/m68knommu/kernel/time.c             |    2 +-
 arch/mips/au1000/common/time.c           |    6 +++---
 arch/mips/galileo-boards/ev96100/time.c  |    2 +-
 arch/mips/gt64120/common/time.c          |    2 +-
 arch/mips/kernel/time.c                  |    2 +-
 arch/mips/momentum/ocelot_g/gt-irq.c     |    2 +-
 arch/mips/sgi-ip27/ip27-timer.c          |    2 +-
 arch/parisc/kernel/time.c                |    2 +-
 arch/powerpc/kernel/time.c               |    2 +-
 arch/ppc/kernel/time.c                   |    2 +-
 arch/s390/kernel/time.c                  |    9 ++++-----
 arch/sh/kernel/time.c                    |    2 +-
 arch/sh64/kernel/time.c                  |    2 +-
 arch/sparc/kernel/pcic.c                 |    2 +-
 arch/sparc/kernel/time.c                 |    2 +-
 arch/sparc64/kernel/time.c               |    4 ++--
 arch/um/kernel/time.c                    |    2 +-
 arch/v850/kernel/time.c                  |    2 +-
 arch/x86_64/kernel/time.c                |    6 ++----
 arch/xtensa/kernel/time.c                |    2 +-
 include/asm-arm/arch-clps711x/time.h     |    2 +-
 include/asm-arm/arch-l7200/time.h        |    2 +-
 include/asm-i386/mach-default/do_timer.h |    2 +-
 include/asm-i386/mach-visws/do_timer.h   |    2 +-
 include/asm-i386/mach-voyager/do_timer.h |    2 +-
 include/linux/sched.h                    |    2 +-
 kernel/timer.c                           |   19 ++++++-------------
 38 files changed, 50 insertions(+), 60 deletions(-)

diff --git a/arch/alpha/kernel/time.c b/arch/alpha/kernel/time.c
index b191cc7..7c1e444 100644
--- a/arch/alpha/kernel/time.c
+++ b/arch/alpha/kernel/time.c
@@ -132,7 +132,7 @@ #endif
 	nticks = delta >> FIX_SHIFT;
 
 	while (nticks > 0) {
-		do_timer(regs);
+		do_timer(1);
 #ifndef CONFIG_SMP
 		update_process_times(user_mode(regs));
 #endif
diff --git a/arch/arm/kernel/time.c b/arch/arm/kernel/time.c
index 09a67d7..4892a1f 100644
--- a/arch/arm/kernel/time.c
+++ b/arch/arm/kernel/time.c
@@ -333,7 +333,7 @@ void timer_tick(struct pt_regs *regs)
 	profile_tick(CPU_PROFILING, regs);
 	do_leds();
 	do_set_rtc();
-	do_timer(regs);
+	do_timer(1);
 #ifndef CONFIG_SMP
 	update_process_times(user_mode(regs));
 #endif
diff --git a/arch/arm26/kernel/time.c b/arch/arm26/kernel/time.c
index db63d75..80adbd0 100644
--- a/arch/arm26/kernel/time.c
+++ b/arch/arm26/kernel/time.c
@@ -194,7 +194,7 @@ EXPORT_SYMBOL(do_settimeofday);
 
 static irqreturn_t timer_interrupt(int irq, void *dev_id, struct pt_regs *regs)
 {
-        do_timer(regs);
+        do_timer(1);
 #ifndef CONFIG_SMP
 	update_process_times(user_mode(regs));
 #endif
diff --git a/arch/cris/arch-v10/kernel/time.c b/arch/cris/arch-v10/kernel/time.c
index 9c22b76..ebacf14 100644
--- a/arch/cris/arch-v10/kernel/time.c
+++ b/arch/cris/arch-v10/kernel/time.c
@@ -227,7 +227,7 @@ #endif
 	
 	/* call the real timer interrupt handler */
 
-	do_timer(regs);
+	do_timer(1);
 	
         cris_do_profile(regs); /* Save profiling information */
 
diff --git a/arch/cris/arch-v32/kernel/time.c b/arch/cris/arch-v32/kernel/time.c
index 50f3f93..be0a016 100644
--- a/arch/cris/arch-v32/kernel/time.c
+++ b/arch/cris/arch-v32/kernel/time.c
@@ -219,7 +219,7 @@ timer_interrupt(int irq, void *dev_id, s
 		return IRQ_HANDLED;
 
 	/* call the real timer interrupt handler */
-	do_timer(regs);
+	do_timer(1);
 
 	/*
 	 * If we have an externally synchronized Linux clock, then update
diff --git a/arch/frv/kernel/time.c b/arch/frv/kernel/time.c
index d5b64e1..77d18f6 100644
--- a/arch/frv/kernel/time.c
+++ b/arch/frv/kernel/time.c
@@ -73,7 +73,7 @@ static irqreturn_t timer_interrupt(int i
 	 */
 	write_seqlock(&xtime_lock);
 
-	do_timer(regs);
+	do_timer(1);
 	update_process_times(user_mode(regs));
 	profile_tick(CPU_PROFILING, regs);
 
diff --git a/arch/h8300/kernel/time.c b/arch/h8300/kernel/time.c
index 688a510..e569d17 100644
--- a/arch/h8300/kernel/time.c
+++ b/arch/h8300/kernel/time.c
@@ -41,7 +41,7 @@ static void timer_interrupt(int irq, voi
 	/* may need to kick the hardware timer */
 	platform_timer_eoi();
 
-	do_timer(regs);
+	do_timer(1);
 #ifndef CONFIG_SMP
 	update_process_times(user_mode(regs));
 #endif
diff --git a/arch/ia64/kernel/time.c b/arch/ia64/kernel/time.c
index 6928ef0..1626268 100644
--- a/arch/ia64/kernel/time.c
+++ b/arch/ia64/kernel/time.c
@@ -78,7 +78,7 @@ timer_interrupt (int irq, void *dev_id, 
 			 * xtime_lock.
 			 */
 			write_seqlock(&xtime_lock);
-			do_timer(regs);
+			do_timer(1);
 			local_cpu_data->itm_next = new_itm;
 			write_sequnlock(&xtime_lock);
 		} else
diff --git a/arch/m32r/kernel/time.c b/arch/m32r/kernel/time.c
index ded0be0..7a89689 100644
--- a/arch/m32r/kernel/time.c
+++ b/arch/m32r/kernel/time.c
@@ -202,7 +202,7 @@ irqreturn_t timer_interrupt(int irq, voi
 #ifndef CONFIG_SMP
 	profile_tick(CPU_PROFILING, regs);
 #endif
-	do_timer(regs);
+	do_timer(1);
 
 #ifndef CONFIG_SMP
 	update_process_times(user_mode(regs));
diff --git a/arch/m68k/kernel/time.c b/arch/m68k/kernel/time.c
index 98e4b1a..1072e49 100644
--- a/arch/m68k/kernel/time.c
+++ b/arch/m68k/kernel/time.c
@@ -40,7 +40,7 @@ static inline int set_rtc_mmss(unsigned 
  */
 static irqreturn_t timer_interrupt(int irq, void *dummy, struct pt_regs * regs)
 {
-	do_timer(regs);
+	do_timer(1);
 #ifndef CONFIG_SMP
 	update_process_times(user_mode(regs));
 #endif
diff --git a/arch/m68k/sun3/sun3ints.c b/arch/m68k/sun3/sun3ints.c
index f18b9d3..dc4ea7e 100644
--- a/arch/m68k/sun3/sun3ints.c
+++ b/arch/m68k/sun3/sun3ints.c
@@ -65,7 +65,7 @@ #endif
 #ifdef CONFIG_SUN3
 	intersil_clear();
 #endif
-        do_timer(fp);
+        do_timer(1);
 #ifndef CONFIG_SMP
 	update_process_times(user_mode(fp));
 #endif
diff --git a/arch/m68knommu/kernel/time.c b/arch/m68knommu/kernel/time.c
index 1db9872..db1e1ce 100644
--- a/arch/m68knommu/kernel/time.c
+++ b/arch/m68knommu/kernel/time.c
@@ -51,7 +51,7 @@ static irqreturn_t timer_interrupt(int i
 
 	write_seqlock(&xtime_lock);
 
-	do_timer(regs);
+	do_timer(1);
 #ifndef CONFIG_SMP
 	update_process_times(user_mode(regs));
 #endif
diff --git a/arch/mips/au1000/common/time.c b/arch/mips/au1000/common/time.c
index 7fbea1b..0a067f3 100644
--- a/arch/mips/au1000/common/time.c
+++ b/arch/mips/au1000/common/time.c
@@ -96,7 +96,7 @@ void mips_timer_interrupt(struct pt_regs
 		timerlo = count;
 
 		kstat_this_cpu.irqs[irq]++;
-		do_timer(regs);
+		do_timer(1);
 #ifndef CONFIG_SMP
 		update_process_times(user_mode(regs));
 #endif
@@ -137,7 +137,7 @@ irqreturn_t counter0_irq(int irq, void *
 	}
 
 	while (time_elapsed > 0) {
-		do_timer(regs);
+		do_timer(1);
 #ifndef CONFIG_SMP
 		update_process_times(user_mode(regs));
 #endif
@@ -156,7 +156,7 @@ #endif
 
 	if (jiffie_drift >= 999) {
 		jiffie_drift -= 999;
-		do_timer(regs); /* increment jiffies by one */
+		do_timer(1); /* increment jiffies by one */
 #ifndef CONFIG_SMP
 		update_process_times(user_mode(regs));
 #endif
diff --git a/arch/mips/galileo-boards/ev96100/time.c b/arch/mips/galileo-boards/ev96100/time.c
index 8cbe842..60c564b 100644
--- a/arch/mips/galileo-boards/ev96100/time.c
+++ b/arch/mips/galileo-boards/ev96100/time.c
@@ -72,7 +72,7 @@ void mips_timer_interrupt(struct pt_regs
 
 	do {
 		kstat_this_cpu.irqs[irq]++;
-		do_timer(regs);
+		do_timer(1);
 #ifndef CONFIG_SMP
 		update_process_times(user_mode(regs));
 #endif
diff --git a/arch/mips/gt64120/common/time.c b/arch/mips/gt64120/common/time.c
index d837b26..7feca49 100644
--- a/arch/mips/gt64120/common/time.c
+++ b/arch/mips/gt64120/common/time.c
@@ -34,7 +34,7 @@ static void gt64120_irq(int irq, void *d
 	if (irq_src & 0x00000800) {	/* Check for timer interrupt */
 		handled = 1;
 		irq_src &= ~0x00000800;
-		do_timer(regs);
+		do_timer(1);
 #ifndef CONFIG_SMP
 		update_process_times(user_mode(regs));
 #endif
diff --git a/arch/mips/kernel/time.c b/arch/mips/kernel/time.c
index 170cb67..6ab8d97 100644
--- a/arch/mips/kernel/time.c
+++ b/arch/mips/kernel/time.c
@@ -434,7 +434,7 @@ irqreturn_t timer_interrupt(int irq, voi
 	/*
 	 * call the generic timer interrupt handling
 	 */
-	do_timer(regs);
+	do_timer(1);
 
 	/*
 	 * If we have an externally synchronized Linux clock, then update
diff --git a/arch/mips/momentum/ocelot_g/gt-irq.c b/arch/mips/momentum/ocelot_g/gt-irq.c
index 9fb2493..6cd87cf 100644
--- a/arch/mips/momentum/ocelot_g/gt-irq.c
+++ b/arch/mips/momentum/ocelot_g/gt-irq.c
@@ -133,7 +133,7 @@ static irqreturn_t gt64240_p0int_irq(int
 		MV_WRITE(TIMER_COUNTER_0_3_INTERRUPT_CAUSE, 0x0);
 
 		/* handle the timer call */
-		do_timer(regs);
+		do_timer(1);
 #ifndef CONFIG_SMP
 		update_process_times(user_mode(regs));
 #endif
diff --git a/arch/mips/sgi-ip27/ip27-timer.c b/arch/mips/sgi-ip27/ip27-timer.c
index b029ba7..c62a3a9 100644
--- a/arch/mips/sgi-ip27/ip27-timer.c
+++ b/arch/mips/sgi-ip27/ip27-timer.c
@@ -111,7 +111,7 @@ again:
 	kstat_this_cpu.irqs[irq]++;		/* kstat only for bootcpu? */
 
 	if (cpu == 0)
-		do_timer(regs);
+		do_timer(1);
 
 	update_process_times(user_mode(regs));
 
diff --git a/arch/parisc/kernel/time.c b/arch/parisc/kernel/time.c
index 5facc9b..700df10 100644
--- a/arch/parisc/kernel/time.c
+++ b/arch/parisc/kernel/time.c
@@ -79,7 +79,7 @@ #else
 #endif
 		if (cpu == 0) {
 			write_seqlock(&xtime_lock);
-			do_timer(regs);
+			do_timer(1);
 			write_sequnlock(&xtime_lock);
 		}
 	}
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index 774c0a3..914178c 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -693,7 +693,7 @@ #endif
 		write_seqlock(&xtime_lock);
 		tb_last_jiffy += tb_ticks_per_jiffy;
 		tb_last_stamp = per_cpu(last_jiffy, cpu);
-		do_timer(regs);
+		do_timer(1);
 		timer_recalc_offset(tb_last_jiffy);
 		timer_check_rtc();
 		write_sequnlock(&xtime_lock);
diff --git a/arch/ppc/kernel/time.c b/arch/ppc/kernel/time.c
index 6ab8cc7..1e1f315 100644
--- a/arch/ppc/kernel/time.c
+++ b/arch/ppc/kernel/time.c
@@ -153,7 +153,7 @@ void timer_interrupt(struct pt_regs * re
 		/* We are in an interrupt, no need to save/restore flags */
 		write_seqlock(&xtime_lock);
 		tb_last_stamp = jiffy_stamp;
-		do_timer(regs);
+		do_timer(1);
 
 		/*
 		 * update the rtc when needed, this should be performed on the
diff --git a/arch/s390/kernel/time.c b/arch/s390/kernel/time.c
index 74e6178..ade3f07 100644
--- a/arch/s390/kernel/time.c
+++ b/arch/s390/kernel/time.c
@@ -166,7 +166,7 @@ #endif /* CONFIG_PROFILING */
 void account_ticks(struct pt_regs *regs)
 {
 	__u64 tmp;
-	__u32 ticks, xticks;
+	__u32 ticks;
 
 	/* Calculate how many ticks have passed. */
 	if (S390_lowcore.int_clock < S390_lowcore.jiffy_timer) {
@@ -204,6 +204,7 @@ #ifdef CONFIG_SMP
 	 */
 	write_seqlock(&xtime_lock);
 	if (S390_lowcore.jiffy_timer > xtime_cc) {
+		__u32 xticks;
 		tmp = S390_lowcore.jiffy_timer - xtime_cc;
 		if (tmp >= 2*CLK_TICKS_PER_JIFFY) {
 			xticks = __div(tmp, CLK_TICKS_PER_JIFFY);
@@ -212,13 +213,11 @@ #ifdef CONFIG_SMP
 			xticks = 1;
 			xtime_cc += CLK_TICKS_PER_JIFFY;
 		}
-		while (xticks--)
-			do_timer(regs);
+		do_timer(xticks);
 	}
 	write_sequnlock(&xtime_lock);
 #else
-	for (xticks = ticks; xticks > 0; xticks--)
-		do_timer(regs);
+	do_timer(ticks);
 #endif
 
 #ifdef CONFIG_VIRT_CPU_ACCOUNTING
diff --git a/arch/sh/kernel/time.c b/arch/sh/kernel/time.c
index a1589f8..9351ee9 100644
--- a/arch/sh/kernel/time.c
+++ b/arch/sh/kernel/time.c
@@ -115,7 +115,7 @@ static long last_rtc_update;
  */
 void handle_timer_tick(struct pt_regs *regs)
 {
-	do_timer(regs);
+	do_timer(1);
 #ifndef CONFIG_SMP
 	update_process_times(user_mode(regs));
 #endif
diff --git a/arch/sh64/kernel/time.c b/arch/sh64/kernel/time.c
index b8162e5..3b61e06 100644
--- a/arch/sh64/kernel/time.c
+++ b/arch/sh64/kernel/time.c
@@ -298,7 +298,7 @@ static inline void do_timer_interrupt(in
 	asm ("getcon cr62, %0" : "=r" (current_ctc));
 	ctc_last_interrupt = (unsigned long) current_ctc;
 
-	do_timer(regs);
+	do_timer(1);
 #ifndef CONFIG_SMP
 	update_process_times(user_mode(regs));
 #endif
diff --git a/arch/sparc/kernel/pcic.c b/arch/sparc/kernel/pcic.c
index bfd31aa..e19b1ba 100644
--- a/arch/sparc/kernel/pcic.c
+++ b/arch/sparc/kernel/pcic.c
@@ -712,7 +712,7 @@ static irqreturn_t pcic_timer_handler (i
 {
 	write_seqlock(&xtime_lock);	/* Dummy, to show that we remember */
 	pcic_clear_clock_irq();
-	do_timer(regs);
+	do_timer(1);
 #ifndef CONFIG_SMP
 	update_process_times(user_mode(regs));
 #endif
diff --git a/arch/sparc/kernel/time.c b/arch/sparc/kernel/time.c
index 845081b..6f84fa1 100644
--- a/arch/sparc/kernel/time.c
+++ b/arch/sparc/kernel/time.c
@@ -128,7 +128,7 @@ #ifdef CONFIG_SUN4
 #endif
 	clear_clock_irq();
 
-	do_timer(regs);
+	do_timer(1);
 #ifndef CONFIG_SMP
 	update_process_times(user_mode(regs));
 #endif
diff --git a/arch/sparc64/kernel/time.c b/arch/sparc64/kernel/time.c
index 094d3e3..f5ed96f 100644
--- a/arch/sparc64/kernel/time.c
+++ b/arch/sparc64/kernel/time.c
@@ -465,7 +465,7 @@ #ifndef CONFIG_SMP
 		profile_tick(CPU_PROFILING, regs);
 		update_process_times(user_mode(regs));
 #endif
-		do_timer(regs);
+		do_timer(1);
 
 		/* Guarantee that the following sequences execute
 		 * uninterrupted.
@@ -496,7 +496,7 @@ void timer_tick_interrupt(struct pt_regs
 {
 	write_seqlock(&xtime_lock);
 
-	do_timer(regs);
+	do_timer(1);
 
 	timer_check_rtc();
 
diff --git a/arch/um/kernel/time.c b/arch/um/kernel/time.c
index 552ca1c..2e2caf0 100644
--- a/arch/um/kernel/time.c
+++ b/arch/um/kernel/time.c
@@ -96,7 +96,7 @@ irqreturn_t um_timer(int irq, void *dev,
 
 	write_seqlock_irqsave(&xtime_lock, flags);
 
-	do_timer(regs);
+	do_timer(1);
 
 	nsecs = get_time() + local_offset;
 	xtime.tv_sec = nsecs / NSEC_PER_SEC;
diff --git a/arch/v850/kernel/time.c b/arch/v850/kernel/time.c
index a0b4669..f4d1a4d 100644
--- a/arch/v850/kernel/time.c
+++ b/arch/v850/kernel/time.c
@@ -51,7 +51,7 @@ #endif
 	if (mach_tick)
 	  mach_tick ();
 
-	do_timer (regs);
+	do_timer (1);
 #ifndef CONFIG_SMP
 	update_process_times(user_mode(regs));
 #endif
diff --git a/arch/x86_64/kernel/time.c b/arch/x86_64/kernel/time.c
index 7a9b182..fdd9e98 100644
--- a/arch/x86_64/kernel/time.c
+++ b/arch/x86_64/kernel/time.c
@@ -421,16 +421,14 @@ #endif
 				(((long) offset << US_SCALE) / vxtime.tsc_quot) - 1;
 	}
 
-	if (lost > 0) {
+	if (lost > 0)
 		handle_lost_ticks(lost, regs);
-		jiffies += lost;
-	}
 
 /*
  * Do the timer stuff.
  */
 
-	do_timer(regs);
+	do_timer(lost + 1);
 #ifndef CONFIG_SMP
 	update_process_times(user_mode(regs));
 #endif
diff --git a/arch/xtensa/kernel/time.c b/arch/xtensa/kernel/time.c
index 412ab32..241db20 100644
--- a/arch/xtensa/kernel/time.c
+++ b/arch/xtensa/kernel/time.c
@@ -175,7 +175,7 @@ #endif
 
 		last_ccount_stamp = next;
 		next += CCOUNT_PER_JIFFY;
-		do_timer (regs); /* Linux handler in kernel/timer.c */
+		do_timer (1); /* Linux handler in kernel/timer.c */
 
 		if (ntp_synced() &&
 		    xtime.tv_sec - last_rtc_update >= 659 &&
diff --git a/include/asm-arm/arch-clps711x/time.h b/include/asm-arm/arch-clps711x/time.h
index 9cb27cd..0e4a390 100644
--- a/include/asm-arm/arch-clps711x/time.h
+++ b/include/asm-arm/arch-clps711x/time.h
@@ -29,7 +29,7 @@ static irqreturn_t
 p720t_timer_interrupt(int irq, void *dev_id, struct pt_regs *regs)
 {
 	do_leds();
-	do_timer(regs);
+	do_timer(1);
 #ifndef CONFIG_SMP
 	update_process_times(user_mode(regs));
 #endif
diff --git a/include/asm-arm/arch-l7200/time.h b/include/asm-arm/arch-l7200/time.h
index 7b98b53..c69cb50 100644
--- a/include/asm-arm/arch-l7200/time.h
+++ b/include/asm-arm/arch-l7200/time.h
@@ -45,7 +45,7 @@ #define RTC_EN_STWDOG	0x08      /* Enabl
 static irqreturn_t
 timer_interrupt(int irq, void *dev_id, struct pt_regs *regs)
 {
-	do_timer(regs);
+	do_timer(1);
 #ifndef CONFIG_SMP
 	update_process_times(user_mode(regs));
 #endif
diff --git a/include/asm-i386/mach-default/do_timer.h b/include/asm-i386/mach-default/do_timer.h
index 6312c3e..4182c34 100644
--- a/include/asm-i386/mach-default/do_timer.h
+++ b/include/asm-i386/mach-default/do_timer.h
@@ -16,7 +16,7 @@ #include <asm/i8259.h>
 
 static inline void do_timer_interrupt_hook(struct pt_regs *regs)
 {
-	do_timer(regs);
+	do_timer(1);
 #ifndef CONFIG_SMP
 	update_process_times(user_mode_vm(regs));
 #endif
diff --git a/include/asm-i386/mach-visws/do_timer.h b/include/asm-i386/mach-visws/do_timer.h
index 95568e6..8db618c 100644
--- a/include/asm-i386/mach-visws/do_timer.h
+++ b/include/asm-i386/mach-visws/do_timer.h
@@ -9,7 +9,7 @@ static inline void do_timer_interrupt_ho
 	/* Clear the interrupt */
 	co_cpu_write(CO_CPU_STAT,co_cpu_read(CO_CPU_STAT) & ~CO_STAT_TIMEINTR);
 
-	do_timer(regs);
+	do_timer(1);
 #ifndef CONFIG_SMP
 	update_process_times(user_mode_vm(regs));
 #endif
diff --git a/include/asm-i386/mach-voyager/do_timer.h b/include/asm-i386/mach-voyager/do_timer.h
index eaf5180..099fe9f 100644
--- a/include/asm-i386/mach-voyager/do_timer.h
+++ b/include/asm-i386/mach-voyager/do_timer.h
@@ -3,7 +3,7 @@ #include <asm/voyager.h>
 
 static inline void do_timer_interrupt_hook(struct pt_regs *regs)
 {
-	do_timer(regs);
+	do_timer(1);
 #ifndef CONFIG_SMP
 	update_process_times(user_mode_vm(regs));
 #endif
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 6674fc1..145db1a 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1180,7 +1180,7 @@ extern void switch_uid(struct user_struc
 
 #include <asm/current.h>
 
-extern void do_timer(struct pt_regs *);
+extern void do_timer(unsigned long ticks);
 
 extern int FASTCALL(wake_up_state(struct task_struct * tsk, unsigned int state));
 extern int FASTCALL(wake_up_process(struct task_struct * tsk));
diff --git a/kernel/timer.c b/kernel/timer.c
index b650f04..e2464c1 100644
--- a/kernel/timer.c
+++ b/kernel/timer.c
@@ -1217,10 +1217,8 @@ static inline void calc_load(unsigned lo
 	unsigned long active_tasks; /* fixed-point */
 	static int count = LOAD_FREQ;
 
-	count -= ticks;
-	if (count < 0) {
-		count += LOAD_FREQ;
-		active_tasks = count_active_tasks();
+	active_tasks = count_active_tasks();
+	for (count -= ticks; count < 0; count += LOAD_FREQ) {
 		CALC_LOAD(avenrun[0], EXP_1, active_tasks);
 		CALC_LOAD(avenrun[1], EXP_5, active_tasks);
 		CALC_LOAD(avenrun[2], EXP_15, active_tasks);
@@ -1265,11 +1263,8 @@ void run_local_timers(void)
  * Called by the timer interrupt. xtime_lock must already be taken
  * by the timer IRQ!
  */
-static inline void update_times(void)
+static inline void update_times(unsigned long ticks)
 {
-	unsigned long ticks;
-
-	ticks = jiffies - wall_jiffies;
 	wall_jiffies += ticks;
 	update_wall_time();
 	calc_load(ticks);
@@ -1281,12 +1276,10 @@ static inline void update_times(void)
  * jiffies is defined in the linker script...
  */
 
-void do_timer(struct pt_regs *regs)
+void do_timer(unsigned long ticks)
 {
-	jiffies_64++;
-	/* prevent loading jiffies before storing new jiffies_64 value. */
-	barrier();
-	update_times();
+	jiffies_64 += ticks;
+	update_times(ticks);
 }
 
 #ifdef __ARCH_WANT_SYS_ALARM

Re: [PATCH] simplify update_times (avoid jiffies/jiffies_64 aliasing problem)

[john stultz]

On Fri, 2006-03-03 at 11:44 +0900, Atsushi Nemoto wrote:
> >>>>> On Thu, 2 Mar 2006 11:09:16 -0800 (PST), Christoph Lameter <clameter@engr.sgi.com> said:
> >> In kernel 2.6, update_times() is called directly in timer
> >> interrupt, so there is no point calculating ticks here.  This also
> >> get rid of difference of jiffies and jiffies_64 due to compiler's
> >> optimization (which was reported previously with subject
> >> "jiffies_64 vs. jiffies").
> 
> clameter> If update_wall_time() and calc_load() are always called with
> clameter> the constant one then you may be able to optimize these two
> clameter> functions as well.
> 
> Sure.  I tried to do only one thing at a time, but it might be better
> to clean them up together.  Patch revised.
> 
> 
> In kernel 2.6, update_times() is called directly in timer interrupt,
> so there is no point calculating ticks here.  Then update_wall_time()
> and calc_load() can also be optimized.  This also get rid of
> difference of jiffies and jiffies_64 due to compiler's optimization
> (which was reported previously with subject "jiffies_64 vs. jiffies").


I'm not opposed to this change, but I'm not sure if the barrier with a
clear comment as to why its needed might be better in the short term.

> Also adjust x86_64 timer interrupt handler with this change.
> 
> Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
> 
> diff --git a/arch/x86_64/kernel/time.c b/arch/x86_64/kernel/time.c
> index 3080f84..7a1d790 100644
> --- a/arch/x86_64/kernel/time.c
> +++ b/arch/x86_64/kernel/time.c
> @@ -423,7 +423,8 @@ void main_timer_handler(struct pt_regs *
>  
>  	if (lost > 0) {
>  		handle_lost_ticks(lost, regs);
> -		jiffies += lost;
> +		while (lost--)
> +			do_timer(regs);
>  	}

i386 also has lost tick processing that will need to be handed as well. 

thanks
-john

Re: [PATCH] simplify update_times (avoid jiffies/jiffies_64 aliasing problem)

[Ralf Baechle]

On Fri, Mar 03, 2006 at 10:13:58AM -0800, john stultz wrote:

> > In kernel 2.6, update_times() is called directly in timer interrupt,
> > so there is no point calculating ticks here.  Then update_wall_time()
> > and calc_load() can also be optimized.  This also get rid of
> > difference of jiffies and jiffies_64 due to compiler's optimization
> > (which was reported previously with subject "jiffies_64 vs. jiffies").
> 
> 
> I'm not opposed to this change, but I'm not sure if the barrier with a
> clear comment as to why its needed might be better in the short term.

A good fix is harder than it looks and with your patches on the horizon
it seems the safest to get a stable 2.6.16 out of the door is the barrier.

  Ralf