+ use-cycle_t-instead-of-u64-in-struct-time_interpolator.patch added to -mm tree

+ use-cycle_t-instead-of-u64-in-struct-time_interpolator.patch added to -mm tree

[akpm]

The patch titled
     use cycle_t instead of u64 in struct time_interpolator
has been added to the -mm tree.  Its filename is
     use-cycle_t-instead-of-u64-in-struct-time_interpolator.patch

See http://www.zip.com.au/~akpm/linux/patches/stuff/added-to-mm.txt to find
out what to do about this

------------------------------------------------------
Subject: use cycle_t instead of u64 in struct time_interpolator
From: Helge Deller <deller@gmx.de>

The 32bit and 64bit PARISC Linux kernels suffers from the problem, that the
gettimeofday() call sometimes returns non-monotonic times.

The easiest way to fix this, is to drop the PARISC-specific implementation
and switch over to the generic TIME_INTERPOLATION framework.

But in order to make it even compile on 32bit PARISC, the patch below which
touches the generic Linux code, is mandatory.

More information and the full patch with the parisc-specific changes is included in this thread: http://lists.parisc-linux.org/pipermail/parisc-linux/2006-December/031003.html

As far as I could see, this patch does not change anything for the existing
architectures which use this framework (IA64 and SPARC64), since "cycles_t"
is defined there as unsigned 64bit-integer anyway (which then makes this
patch a no-change for them).

Signed-off-by: Helge Deller <deller@gmx.de>
Cc: <linux-arch@vger.kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Andrew Morton <akpm@osdl.org>
---

 include/linux/timex.h |    4 ++--
 kernel/timer.c        |    6 +++---
 2 files changed, 5 insertions(+), 5 deletions(-)

diff -puN include/linux/timex.h~use-cycle_t-instead-of-u64-in-struct-time_interpolator include/linux/timex.h
--- a/include/linux/timex.h~use-cycle_t-instead-of-u64-in-struct-time_interpolator
+++ a/include/linux/timex.h
@@ -255,10 +255,10 @@ struct time_interpolator {
 	u8 jitter;			/* if set compensate for fluctuations */
 	u32 nsec_per_cyc;		/* set by register_time_interpolator() */
 	void *addr;			/* address of counter or function */
-	u64 mask;			/* mask the valid bits of the counter */
+	cycles_t mask;			/* mask the valid bits of the counter */
 	unsigned long offset;		/* nsec offset at last update of interpolator */
 	u64 last_counter;		/* counter value in units of the counter at last update */
-	u64 last_cycle;			/* Last timer value if TIME_SOURCE_JITTER is set */
+	cycles_t last_cycle;		/* Last timer value if TIME_SOURCE_JITTER is set */
 	u64 frequency;			/* frequency in counts/second */
 	long drift;			/* drift in parts-per-million (or -1) */
 	unsigned long skips;		/* skips forward */
diff -puN kernel/timer.c~use-cycle_t-instead-of-u64-in-struct-time_interpolator kernel/timer.c
--- a/kernel/timer.c~use-cycle_t-instead-of-u64-in-struct-time_interpolator
+++ a/kernel/timer.c
@@ -1624,7 +1624,7 @@ struct time_interpolator *time_interpola
 static struct time_interpolator *time_interpolator_list __read_mostly;
 static DEFINE_SPINLOCK(time_interpolator_lock);
 
-static inline u64 time_interpolator_get_cycles(unsigned int src)
+static inline cycles_t time_interpolator_get_cycles(unsigned int src)
 {
 	unsigned long (*x)(void);
 
@@ -1650,8 +1650,8 @@ static inline u64 time_interpolator_get_
 
 	if (time_interpolator->jitter)
 	{
-		u64 lcycle;
-		u64 now;
+		cycles_t lcycle;
+		cycles_t now;
 
 		do {
 			lcycle = time_interpolator->last_cycle;
_

Patches currently in -mm which might be from deller@gmx.de are

use-cycle_t-instead-of-u64-in-struct-time_interpolator.patch

Re: + use-cycle_t-instead-of-u64-in-struct-time_interpolator.patch added to -mm tree

[Matthew Wilcox]

On Wed, Jan 03, 2007 at 01:00:27PM -0800, akpm@osdl.org wrote:
> The patch titled
>      use cycle_t instead of u64 in struct time_interpolator
> has been added to the -mm tree.  Its filename is
>      use-cycle_t-instead-of-u64-in-struct-time_interpolator.patch

Helge withdrew this patch.

Re: + use-cycle_t-instead-of-u64-in-struct-time_interpolator.patch added to -mm tree

[Helge Deller]

John, Christoph, all,

The patch is not necessary any longer for the parisc Linux kernel.
Nevertheless it's not wrong and makes it generelly possible to us this framework on 32bit architectures as well.

I would prefer if it goes in, but I leave it up to you to decide...

Helge

On Wed Jan 3 2007, akpm@osdl.org wrote:
> 
> The patch titled
>      use cycle_t instead of u64 in struct time_interpolator
> has been added to the -mm tree.  Its filename is
>      use-cycle_t-instead-of-u64-in-struct-time_interpolator.patch
> 
> See http://www.zip.com.au/~akpm/linux/patches/stuff/added-to-mm.txt to find
> out what to do about this
> 
> ------------------------------------------------------
> Subject: use cycle_t instead of u64 in struct time_interpolator
> From: Helge Deller <deller@gmx.de>
> 
> The 32bit and 64bit PARISC Linux kernels suffers from the problem, that the
> gettimeofday() call sometimes returns non-monotonic times.
> 
> The easiest way to fix this, is to drop the PARISC-specific implementation
> and switch over to the generic TIME_INTERPOLATION framework.
> 
> But in order to make it even compile on 32bit PARISC, the patch below which
> touches the generic Linux code, is mandatory.
> 
> More information and the full patch with the parisc-specific changes is included in this thread: http://lists.parisc-linux.org/pipermail/parisc-linux/2006-December/031003.html
> 
> As far as I could see, this patch does not change anything for the existing
> architectures which use this framework (IA64 and SPARC64), since "cycles_t"
> is defined there as unsigned 64bit-integer anyway (which then makes this
> patch a no-change for them).
> 
> Signed-off-by: Helge Deller <deller@gmx.de>
> Cc: <linux-arch@vger.kernel.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@elte.hu>
> Signed-off-by: Andrew Morton <akpm@osdl.org>
> ---
> 
>  include/linux/timex.h |    4 ++--
>  kernel/timer.c        |    6 +++---
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff -puN include/linux/timex.h~use-cycle_t-instead-of-u64-in-struct-time_interpolator include/linux/timex.h
> --- a/include/linux/timex.h~use-cycle_t-instead-of-u64-in-struct-time_interpolator
> +++ a/include/linux/timex.h
> @@ -255,10 +255,10 @@ struct time_interpolator {
>  	u8 jitter;			/* if set compensate for fluctuations */
>  	u32 nsec_per_cyc;		/* set by register_time_interpolator() */
>  	void *addr;			/* address of counter or function */
> -	u64 mask;			/* mask the valid bits of the counter */
> +	cycles_t mask;			/* mask the valid bits of the counter */
>  	unsigned long offset;		/* nsec offset at last update of interpolator */
>  	u64 last_counter;		/* counter value in units of the counter at last update */
> -	u64 last_cycle;			/* Last timer value if TIME_SOURCE_JITTER is set */
> +	cycles_t last_cycle;		/* Last timer value if TIME_SOURCE_JITTER is set */
>  	u64 frequency;			/* frequency in counts/second */
>  	long drift;			/* drift in parts-per-million (or -1) */
>  	unsigned long skips;		/* skips forward */
> diff -puN kernel/timer.c~use-cycle_t-instead-of-u64-in-struct-time_interpolator kernel/timer.c
> --- a/kernel/timer.c~use-cycle_t-instead-of-u64-in-struct-time_interpolator
> +++ a/kernel/timer.c
> @@ -1624,7 +1624,7 @@ struct time_interpolator *time_interpola
>  static struct time_interpolator *time_interpolator_list __read_mostly;
>  static DEFINE_SPINLOCK(time_interpolator_lock);
>  
> -static inline u64 time_interpolator_get_cycles(unsigned int src)
> +static inline cycles_t time_interpolator_get_cycles(unsigned int src)
>  {
>  	unsigned long (*x)(void);
>  
> @@ -1650,8 +1650,8 @@ static inline u64 time_interpolator_get_
>  
>  	if (time_interpolator->jitter)
>  	{
> -		u64 lcycle;
> -		u64 now;
> +		cycles_t lcycle;
> +		cycles_t now;
>  
>  		do {
>  			lcycle = time_interpolator->last_cycle;
> _
> 
> Patches currently in -mm which might be from deller@gmx.de are
> 
> use-cycle_t-instead-of-u64-in-struct-time_interpolator.patch
> 

Re: + use-cycle_t-instead-of-u64-in-struct-time_interpolator.patch added to -mm tree

[Andrew Morton]

On Wed, 3 Jan 2007 14:05:52 -0700
Matthew Wilcox <matthew@wil.cx> wrote:

> On Wed, Jan 03, 2007 at 01:00:27PM -0800, akpm@osdl.org wrote:
> > The patch titled
> >      use cycle_t instead of u64 in struct time_interpolator
> > has been added to the -mm tree.  Its filename is
> >      use-cycle_t-instead-of-u64-in-struct-time_interpolator.patch
> 
> Helge withdrew this patch.

Well yeah, but I think he did that because of some unrelated parisc borkage.

The reason I hung onto the change is because I suspect that cycle_t is the
appropriate type to be using in this code.  If someone wants to disagree
with that, I'm all ears.

Re: + use-cycle_t-instead-of-u64-in-struct-time_interpolator.patch added to -mm tree

[Matthew Wilcox]

On Wed, Jan 03, 2007 at 01:25:37PM -0800, Andrew Morton wrote:
> Well yeah, but I think he did that because of some unrelated parisc borkage.

It's simply impossible to implement cmpxchg() in the general case on
parisc.  This was hashed out in great detail recently ...

Re: + use-cycle_t-instead-of-u64-in-struct-time_interpolator.patch added to -mm tree

[Andrew Morton]

On Wed, 3 Jan 2007 14:33:13 -0700
Matthew Wilcox <matthew@wil.cx> wrote:

> On Wed, Jan 03, 2007 at 01:25:37PM -0800, Andrew Morton wrote:
> > Well yeah, but I think he did that because of some unrelated parisc borkage.
> 
> It's simply impossible to implement cmpxchg() in the general case on
> parisc.  This was hashed out in great detail recently ...

That's a bug in the time_interpolator implementation.  Fixable, I assume,
by whacking a spinlock into struct time_interpolator.

RE: + use-cycle_t-instead-of-u64-in-struct-time_interpolator.patch added to -mm tree

[Luck, Tony]

> > It's simply impossible to implement cmpxchg() in the general case on
> > parisc.  This was hashed out in great detail recently ...
>
> That's a bug in the time_interpolator implementation.  Fixable, I assume,
> by whacking a spinlock into struct time_interpolator.

The cmpxchg is already a bit painful if you have a lot of cpus (and a bunch
of users that call gettimeofday() incessantly).  A spinlock would most likely
be even worse (wouldn't it?).

-Tony

Re: + use-cycle_t-instead-of-u64-in-struct-time_interpolator.patch added to -mm tree

[Andrew Morton]

On Wed, 3 Jan 2007 15:04:33 -0800
"Luck, Tony" <tony.luck@intel.com> wrote:

> > > It's simply impossible to implement cmpxchg() in the general case on
> > > parisc.  This was hashed out in great detail recently ...
> >
> > That's a bug in the time_interpolator implementation.  Fixable, I assume,
> > by whacking a spinlock into struct time_interpolator.
> 
> The cmpxchg is already a bit painful if you have a lot of cpus (and a bunch
> of users that call gettimeofday() incessantly).  A spinlock would most likely
> be even worse (wouldn't it?).
> 

It might be slightly worse, although most of the cost will be common
between the two implementations.  Perhaps some design change is needed in
there (a sequence lock?)

But cmpxchg just isn't suitable for use outside per-arch code.