[PATCH] 2.6.9-rc1 Ia64 build broken due to HPET

[PATCH] 2.6.9-rc1 Ia64 build broken due to HPET

[Darren Williams]

The ia64 build broke overnight due to this patch
http://marc.theaimsgroup.com/?l=linux-ia64&m\x109211170801213&w=2
any ia64 arch that includes CONFIG_HPET will fail
on the build, this effect generic_defconfig also.


  CC      drivers/char/hpet.o
drivers/char/hpet.c:669: error: `MC' undeclared here (not in a function)
drivers/char/hpet.c:669: error: initializer element is not constant
drivers/char/hpet.c:669: error: (near initialization for `hpet_interpolator.addr')
drivers/char/hpet.c: In function `hpet_init':
drivers/char/hpet.c:956: error: `hpet_cycles_per_sec' undeclared (first use in this function)
drivers/char/hpet.c:956: error: (Each undeclared identifier is reported only once
drivers/char/hpet.c:956: error: for each function it appears in.)
drivers/char/hpet.c:960: error: `hpet_nsecs_per_cycle' undeclared (first use in this function)
make[2]: *** [drivers/char/hpet.o] Error 1

The included patch will need to be scrutinised due
to the fact I had no idea what to assign to:
hpet_interpolator.addr
signed-off-by Darren Williams <dswATgelato.unsw.edu.au>

# ChangeSet
#   2004/09/10 13:11:25+10:00 dsw@vanilla.gelato 
#   Define hpet_cycles_per_sec for hpet clock calculation and assign
#   nsec_per_cyc directly to hpet_interpolator.nsec_per_cyc
#   
#   The assignment of .addr = hpet_control is more than likely bogus
#   time_interpolator.addr requires an address of a counter or function
# 
# drivers/char/hpet.c
#   2004/09/10 13:11:18+10:00 dsw@vanilla.gelato +4 -3
#   Define hpet_cycles_per_sec for hpet clock calculation and assign
#   nsec_per_cyc directly to hpet_interpolator.nsec_per_cyc
#   
#   The assignment of .addr = hpet_control is more than likely bogus
#   time_interpolator.addr requires an address of a counter or function
#   
diff -Nru a/drivers/char/hpet.c b/drivers/char/hpet.c
--- a/drivers/char/hpet.c	2004-09-10 13:36:48 +10:00
+++ b/drivers/char/hpet.c	2004-09-10 13:36:48 +10:00
@@ -665,7 +665,7 @@
 static struct time_interpolator hpet_interpolator = {
 	.source = TIME_SOURCE_MMIO64,
 	.shift = 10,
-	.addr = MC
+	.addr = hpet_control
 };
 
 #endif
@@ -951,13 +951,14 @@
 #ifdef	CONFIG_TIME_INTERPOLATION
 		{
 			struct hpet *hpet;
-
+			long hpet_cycles_per_sec;
+
 			hpet = hpets->hp_hpet;
 			hpet_cycles_per_sec = hpet_time_div(hpets->hp_period);
 			hpet_interpolator.frequency = hpet_cycles_per_sec;
 			hpet_interpolator.drift = hpet_cycles_per_sec *
 			    HPET_DRIFT / 1000000;
-			hpet_nsecs_per_cycle = 1000000000 / hpet_cycles_per_sec;
+			hpet_interpolator.nsec_per_cyc = 1000000000 / hpet_cycles_per_sec;
 			register_time_interpolator(&hpet_interpolator);
 		}
 #endif


--------------------------------------------------
Darren Williams <dsw AT gelato.unsw.edu.au>
Gelato@UNSW <www.gelato.unsw.edu.au>
--------------------------------------------------

Re: [PATCH] 2.6.9-rc1 Ia64 build broken due to HPET

[Darren Williams]

Hi Darren

On Fri, 10 Sep 2004, Darren Williams wrote:

> The ia64 build broke overnight due to this patch
> http://marc.theaimsgroup.com/?l=linux-ia64&m\x109211170801213&w=2
> any ia64 arch that includes CONFIG_HPET will fail
> on the build, this effect generic_defconfig also.
> 
> 
Me slow see:
http://marc.theaimsgroup.com/?l=linux-ia64&m\x109468628322563&w=2

--------------------------------------------------
Darren Williams <dsw AT gelato.unsw.edu.au>
Gelato@UNSW <www.gelato.unsw.edu.au>
--------------------------------------------------

Re: [PATCH] 2.6.9-rc1 Ia64 build broken due to HPET

[Christoph Lameter]

Could you try the patch that was already posted to linux-ia64? Please make
sure you run a glibc build for 2.6.x and then send me the output of the
short test program following the patch to verify that this really works.

Index: linux-2.6.9-rc1/drivers/char/hpet.c
=================================--- linux-2.6.9-rc1.orig/drivers/char/hpet.c	2004-09-07 18:37:00.000000000 -0700
+++ linux-2.6.9-rc1/drivers/char/hpet.c	2004-09-08 16:17:57.000000000 -0700
@@ -664,8 +664,7 @@

 static struct time_interpolator hpet_interpolator = {
 	.source = TIME_SOURCE_MMIO64,
-	.shift = 10,
-	.addr = MC
+	.shift = 10
 };

 #endif
@@ -953,11 +952,10 @@
 			struct hpet *hpet;

 			hpet = hpets->hp_hpet;
-			hpet_cycles_per_sec = hpet_time_div(hpets->hp_period);
-			hpet_interpolator.frequency = hpet_cycles_per_sec;
-			hpet_interpolator.drift = hpet_cycles_per_sec *
+			hpet_interpolator.addr = &hpets->hp_hpet->hpet_mc;
+			hpet_interpolator.frequency = hpet_time_div(hpets->hp_period);
+			hpet_interpolator.drift = hpet_interpolator.frequency *
 			    HPET_DRIFT / 1000000;
-			hpet_nsecs_per_cycle = 1000000000 / hpet_cycles_per_sec;
 			register_time_interpolator(&hpet_interpolator);
 		}
 #endif

== END OF patch


/*
 * Display Timers
 */

#include <stdio.h>
#include <stdlib.h>
#include <time.h>

void pr(int clock,const char *n)
{
	struct timespec tv,res;

	clock_gettime(clock,&tv);
	clock_getres(clock,&res);
	printf("%25s=% 11d.%09d resolution=% 2d.%09d\n",n,tv.tv_sec,tv.tv_nsec,res.tv_sec,res.tv_nsec);
}

int main(char argc, char *argv[])
{
	struct timespec tv;

	gettimeofday((struct timeval *)&tv);
	tv.tv_nsec = tv.tv_nsec*1000;
	printf("          Gettimeofday() =% 11d.%09d\n",tv.tv_sec,tv.tv_nsec);
	pr(CLOCK_REALTIME,"CLOCK_REALTIME");
	pr(CLOCK_MONOTONIC,"CLOCK_MONOTONIC");
	pr(CLOCK_PROCESS_CPUTIME_ID,"CLOCK_PROCESS_CPUTIME_ID");
	pr(CLOCK_THREAD_CPUTIME_ID,"CLOCK_THREAD_CPUTIME_ID");
	printf("\n");
}

On Fri, 10 Sep 2004, Darren Williams wrote:

> The ia64 build broke overnight due to this patch
> http://marc.theaimsgroup.com/?l=linux-ia64&m\x109211170801213&w=2
> any ia64 arch that includes CONFIG_HPET will fail
> on the build, this effect generic_defconfig also.
>
>
>   CC      drivers/char/hpet.o
> drivers/char/hpet.c:669: error: `MC' undeclared here (not in a function)
> drivers/char/hpet.c:669: error: initializer element is not constant
> drivers/char/hpet.c:669: error: (near initialization for `hpet_interpolator.addr')
> drivers/char/hpet.c: In function `hpet_init':
> drivers/char/hpet.c:956: error: `hpet_cycles_per_sec' undeclared (first use in this function)
> drivers/char/hpet.c:956: error: (Each undeclared identifier is reported only once
> drivers/char/hpet.c:956: error: for each function it appears in.)
> drivers/char/hpet.c:960: error: `hpet_nsecs_per_cycle' undeclared (first use in this function)
> make[2]: *** [drivers/char/hpet.o] Error 1
>
> The included patch will need to be scrutinised due
> to the fact I had no idea what to assign to:
> hpet_interpolator.addr
> signed-off-by Darren Williams <dswATgelato.unsw.edu.au>
>
> # ChangeSet
> #   2004/09/10 13:11:25+10:00 dsw@vanilla.gelato
> #   Define hpet_cycles_per_sec for hpet clock calculation and assign
> #   nsec_per_cyc directly to hpet_interpolator.nsec_per_cyc
> #
> #   The assignment of .addr = hpet_control is more than likely bogus
> #   time_interpolator.addr requires an address of a counter or function
> #
> # drivers/char/hpet.c
> #   2004/09/10 13:11:18+10:00 dsw@vanilla.gelato +4 -3
> #   Define hpet_cycles_per_sec for hpet clock calculation and assign
> #   nsec_per_cyc directly to hpet_interpolator.nsec_per_cyc
> #
> #   The assignment of .addr = hpet_control is more than likely bogus
> #   time_interpolator.addr requires an address of a counter or function
> #
> diff -Nru a/drivers/char/hpet.c b/drivers/char/hpet.c
> --- a/drivers/char/hpet.c	2004-09-10 13:36:48 +10:00
> +++ b/drivers/char/hpet.c	2004-09-10 13:36:48 +10:00
> @@ -665,7 +665,7 @@
>  static struct time_interpolator hpet_interpolator = {
>  	.source = TIME_SOURCE_MMIO64,
>  	.shift = 10,
> -	.addr = MC
> +	.addr = hpet_control
>  };
>
>  #endif
> @@ -951,13 +951,14 @@
>  #ifdef	CONFIG_TIME_INTERPOLATION
>  		{
>  			struct hpet *hpet;
> -
> +			long hpet_cycles_per_sec;
> +
>  			hpet = hpets->hp_hpet;
>  			hpet_cycles_per_sec = hpet_time_div(hpets->hp_period);
>  			hpet_interpolator.frequency = hpet_cycles_per_sec;
>  			hpet_interpolator.drift = hpet_cycles_per_sec *
>  			    HPET_DRIFT / 1000000;
> -			hpet_nsecs_per_cycle = 1000000000 / hpet_cycles_per_sec;
> +			hpet_interpolator.nsec_per_cyc = 1000000000 / hpet_cycles_per_sec;
>  			register_time_interpolator(&hpet_interpolator);
>  		}
>  #endif
>
>
> --------------------------------------------------
> Darren Williams <dsw AT gelato.unsw.edu.au>
> Gelato@UNSW <www.gelato.unsw.edu.au>
> --------------------------------------------------
>

Re: [PATCH] 2.6.9-rc1 Ia64 build broken due to HPET

[Darren Williams]

Hi Christoph
I have tested the patch and all compiles for zx1_defconfig+hpet, I see
that it has been applied to main stream already, however I do not see
any init messages in dmesg for hpet initialisation.
At what point should I be seeing the init message?

I have posted the config and dmesg output at:
http://quasar.cse.unsw.edu.au/~dsw/public-files/hpet/

Its our weekend so replies will be slow until Monday:)

Darren

On Fri, 10 Sep 2004, Christoph Lameter wrote:

> Could you try the patch that was already posted to linux-ia64? Please make
> sure you run a glibc build for 2.6.x and then send me the output of the
> short test program following the patch to verify that this really works.
> 
> Index: linux-2.6.9-rc1/drivers/char/hpet.c
> =================================> --- linux-2.6.9-rc1.orig/drivers/char/hpet.c	2004-09-07 18:37:00.000000000 -0700
> +++ linux-2.6.9-rc1/drivers/char/hpet.c	2004-09-08 16:17:57.000000000 -0700
> @@ -664,8 +664,7 @@
> 
>  static struct time_interpolator hpet_interpolator = {
>  	.source = TIME_SOURCE_MMIO64,
> -	.shift = 10,
> -	.addr = MC
> +	.shift = 10
>  };
> 
>  #endif
> @@ -953,11 +952,10 @@
>  			struct hpet *hpet;
> 
>  			hpet = hpets->hp_hpet;
> -			hpet_cycles_per_sec = hpet_time_div(hpets->hp_period);
> -			hpet_interpolator.frequency = hpet_cycles_per_sec;
> -			hpet_interpolator.drift = hpet_cycles_per_sec *
> +			hpet_interpolator.addr = &hpets->hp_hpet->hpet_mc;
> +			hpet_interpolator.frequency = hpet_time_div(hpets->hp_period);
> +			hpet_interpolator.drift = hpet_interpolator.frequency *
>  			    HPET_DRIFT / 1000000;
> -			hpet_nsecs_per_cycle = 1000000000 / hpet_cycles_per_sec;
>  			register_time_interpolator(&hpet_interpolator);
>  		}
>  #endif
> 
> == END OF patch
> 
> 
> /*
>  * Display Timers
>  */
> 
> #include <stdio.h>
> #include <stdlib.h>
> #include <time.h>
> 
> void pr(int clock,const char *n)
> {
> 	struct timespec tv,res;
> 
> 	clock_gettime(clock,&tv);
> 	clock_getres(clock,&res);
> 	printf("%25s=% 11d.%09d resolution=% 2d.%09d\n",n,tv.tv_sec,tv.tv_nsec,res.tv_sec,res.tv_nsec);
> }
> 
> int main(char argc, char *argv[])
> {
> 	struct timespec tv;
> 
> 	gettimeofday((struct timeval *)&tv);
> 	tv.tv_nsec = tv.tv_nsec*1000;
> 	printf("          Gettimeofday() =% 11d.%09d\n",tv.tv_sec,tv.tv_nsec);
> 	pr(CLOCK_REALTIME,"CLOCK_REALTIME");
> 	pr(CLOCK_MONOTONIC,"CLOCK_MONOTONIC");
> 	pr(CLOCK_PROCESS_CPUTIME_ID,"CLOCK_PROCESS_CPUTIME_ID");
> 	pr(CLOCK_THREAD_CPUTIME_ID,"CLOCK_THREAD_CPUTIME_ID");
> 	printf("\n");
> }
> 
> On Fri, 10 Sep 2004, Darren Williams wrote:
> 
> > The ia64 build broke overnight due to this patch
> > http://marc.theaimsgroup.com/?l=linux-ia64&m\x109211170801213&w=2
> > any ia64 arch that includes CONFIG_HPET will fail
> > on the build, this effect generic_defconfig also.
> >
> >
> >   CC      drivers/char/hpet.o
> > drivers/char/hpet.c:669: error: `MC' undeclared here (not in a function)
> > drivers/char/hpet.c:669: error: initializer element is not constant
> > drivers/char/hpet.c:669: error: (near initialization for `hpet_interpolator.addr')
> > drivers/char/hpet.c: In function `hpet_init':
> > drivers/char/hpet.c:956: error: `hpet_cycles_per_sec' undeclared (first use in this function)
> > drivers/char/hpet.c:956: error: (Each undeclared identifier is reported only once
> > drivers/char/hpet.c:956: error: for each function it appears in.)
> > drivers/char/hpet.c:960: error: `hpet_nsecs_per_cycle' undeclared (first use in this function)
> > make[2]: *** [drivers/char/hpet.o] Error 1
> >
> > The included patch will need to be scrutinised due
> > to the fact I had no idea what to assign to:
> > hpet_interpolator.addr
> > signed-off-by Darren Williams <dswATgelato.unsw.edu.au>
> >
> > # ChangeSet
> > #   2004/09/10 13:11:25+10:00 dsw@vanilla.gelato
> > #   Define hpet_cycles_per_sec for hpet clock calculation and assign
> > #   nsec_per_cyc directly to hpet_interpolator.nsec_per_cyc
> > #
> > #   The assignment of .addr = hpet_control is more than likely bogus
> > #   time_interpolator.addr requires an address of a counter or function
> > #
> > # drivers/char/hpet.c
> > #   2004/09/10 13:11:18+10:00 dsw@vanilla.gelato +4 -3
> > #   Define hpet_cycles_per_sec for hpet clock calculation and assign
> > #   nsec_per_cyc directly to hpet_interpolator.nsec_per_cyc
> > #
> > #   The assignment of .addr = hpet_control is more than likely bogus
> > #   time_interpolator.addr requires an address of a counter or function
> > #
> > diff -Nru a/drivers/char/hpet.c b/drivers/char/hpet.c
> > --- a/drivers/char/hpet.c	2004-09-10 13:36:48 +10:00
> > +++ b/drivers/char/hpet.c	2004-09-10 13:36:48 +10:00
> > @@ -665,7 +665,7 @@
> >  static struct time_interpolator hpet_interpolator = {
> >  	.source = TIME_SOURCE_MMIO64,
> >  	.shift = 10,
> > -	.addr = MC
> > +	.addr = hpet_control
> >  };
> >
> >  #endif
> > @@ -951,13 +951,14 @@
> >  #ifdef	CONFIG_TIME_INTERPOLATION
> >  		{
> >  			struct hpet *hpet;
> > -
> > +			long hpet_cycles_per_sec;
> > +
> >  			hpet = hpets->hp_hpet;
> >  			hpet_cycles_per_sec = hpet_time_div(hpets->hp_period);
> >  			hpet_interpolator.frequency = hpet_cycles_per_sec;
> >  			hpet_interpolator.drift = hpet_cycles_per_sec *
> >  			    HPET_DRIFT / 1000000;
> > -			hpet_nsecs_per_cycle = 1000000000 / hpet_cycles_per_sec;
> > +			hpet_interpolator.nsec_per_cyc = 1000000000 / hpet_cycles_per_sec;
> >  			register_time_interpolator(&hpet_interpolator);
> >  		}
> >  #endif
> >
> >
> > --------------------------------------------------
> > Darren Williams <dsw AT gelato.unsw.edu.au>
> > Gelato@UNSW <www.gelato.unsw.edu.au>
> > --------------------------------------------------
> >
> -
> To unsubscribe from this list: send the line "unsubscribe linux-ia64" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--------------------------------------------------
Darren Williams <dsw AT gelato.unsw.edu.au>
Gelato@UNSW <www.gelato.unsw.edu.au>
--------------------------------------------------

Re: [PATCH] 2.6.9-rc1 Ia64 build broken due to HPET

[Christoph Lameter]

On Sun, 12 Sep 2004, Darren Williams wrote:

> Hi Christoph
> I have tested the patch and all compiles for zx1_defconfig+hpet, I see
> that it has been applied to main stream already, however I do not see
> any init messages in dmesg for hpet initialisation.
> At what point should I be seeing the init message?

The patch does not change the hpet driver per se. There is no init message
showing that the hpet driver is being used as the time interpolator.

Please run the small program that I sent to you and post the
output as I requested.

> > /*
> >  * Display Timers
> >  */
> >
> > #include <stdio.h>
> > #include <stdlib.h>
> > #include <time.h>
> >
> > void pr(int clock,const char *n)
> > {
> > 	struct timespec tv,res;
> >
> > 	clock_gettime(clock,&tv);
> > 	clock_getres(clock,&res);
> > 	printf("%25s=% 11d.%09d resolution=% 2d.%09d\n",n,tv.tv_sec,tv.tv_nsec,res.tv_sec,res.tv_nsec);
> > }
> >
> > int main(char argc, char *argv[])
> > {
> > 	struct timespec tv;
> >
> > 	gettimeofday((struct timeval *)&tv);
> > 	tv.tv_nsec = tv.tv_nsec*1000;
> > 	printf("          Gettimeofday() =% 11d.%09d\n",tv.tv_sec,tv.tv_nsec);
> > 	pr(CLOCK_REALTIME,"CLOCK_REALTIME");
> > 	pr(CLOCK_MONOTONIC,"CLOCK_MONOTONIC");
> > 	pr(CLOCK_PROCESS_CPUTIME_ID,"CLOCK_PROCESS_CPUTIME_ID");
> > 	pr(CLOCK_THREAD_CPUTIME_ID,"CLOCK_THREAD_CPUTIME_ID");
> > 	printf("\n");
> > }

Re: [PATCH] 2.6.9-rc1 Ia64 build broken due to HPET

[Christoph Lameter]

On Sun, 12 Sep 2004, Darren Williams wrote:

> Hi Christoph
> I have tested the patch and all compiles for zx1_defconfig+hpet, I see
> that it has been applied to main stream already, however I do not see
> any init messages in dmesg for hpet initialisation.
> At what point should I be seeing the init message?
>
> I have posted the config and dmesg output at:
> http://quasar.cse.unsw.edu.au/~dsw/public-files/hpet/

There is no dmesg line showing that a hpet device has been detected. Are
you sure that the box provides a hpet timer? If its not detected then you
better talk with the author of the hpet driver.

Re: [PATCH] 2.6.9-rc1 Ia64 build broken due to HPET

[Matthew Wilcox]

On Sat, Sep 11, 2004 at 08:43:21AM -0700, Christoph Lameter wrote:
> There is no dmesg line showing that a hpet device has been detected. Are
> you sure that the box provides a hpet timer? If its not detected then you
> better talk with the author of the hpet driver.

There is no hpet device in currently shipping zx1 boxes.  I'm not sure
there's one in currently shipping sx1000 boxes either.

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain

Re: [PATCH] 2.6.9-rc1 Ia64 build broken due to HPET

[Gerald Pfeifer]

On Sat, 11 Sep 2004, Matthew Wilcox wrote:
> There is no hpet device in currently shipping zx1 boxes.  I'm not sure
> there's one in currently shipping sx1000 boxes either.

I'm believe Eigers (rx7620) and Olympias (rx8620) come with HPET,
at least they claim to. ;-)

  ACPI: HPET (v001     HP   rx8620 0x00000000   HP 0x00000000) @ ....

Gerald

Re: [PATCH] 2.6.9-rc1 Ia64 build broken due to HPET

[Alex Williamson]

On Sat, 2004-09-11 at 21:28 +0200, Gerald Pfeifer wrote:
> On Sat, 11 Sep 2004, Matthew Wilcox wrote:
> > There is no hpet device in currently shipping zx1 boxes.  I'm not sure
> > there's one in currently shipping sx1000 boxes either.
> 
> I'm believe Eigers (rx7620) and Olympias (rx8620) come with HPET,
> at least they claim to. ;-)
> 
>   ACPI: HPET (v001     HP   rx8620 0x00000000   HP 0x00000000) @ ....
> 

   Yes, all the sx1000 based boxes should have one.  I'll try to get a
newer glibc on one and run the test program again, unless someone beats
be to it.

	Alex

Re: [PATCH] 2.6.9-rc1 Ia64 build broken due to HPET

[Alex Williamson]

On Sat, 2004-09-11 at 15:26 -0600, Alex Williamson wrote:

>    Yes, all the sx1000 based boxes should have one.  I'll try to get a
> newer glibc on one and run the test program again, unless someone beats
> be to it.


  Here's the test program output (running on an rx7620) with 2.6.9-rc2
built on a sles9 system:

          Gettimeofday() = 1095104931.812600000
           CLOCK_REALTIME= 1095104931.812639404 resolution= 3712.000240992
          CLOCK_MONOTONIC=        369.103451704 resolution= 3712.000240992
 CLOCK_PROCESS_CPUTIME_ID=          0.000782450 resolution= 0.000000001
  CLOCK_THREAD_CPUTIME_ID=          0.000794115 resolution= 0.000000001


-- 
Alex Williamson                             HP Linux & Open Source Lab

Re: [PATCH] 2.6.9-rc1 Ia64 build broken due to HPET

[Christoph Lameter]

On Mon, 13 Sep 2004, Alex Williamson wrote:

>
>           Gettimeofday() = 1095104931.812600000
>            CLOCK_REALTIME= 1095104931.812639404 resolution= 3712.000240992
>           CLOCK_MONOTONIC=        369.103451704 resolution= 3712.000240992
>  CLOCK_PROCESS_CPUTIME_ID=          0.000782450 resolution= 0.000000001
>   CLOCK_THREAD_CPUTIME_ID=          0.000794115 resolution= 0.000000001
>

That looks better but the resolution is still screwed up. The nanoseconds
should be 3712 not the seconds.... The kernel clock_getres always sets the
second part to zero. So does glibc when emulating CLOCK_REALTIME....

Could you figure out what is going on there? If the nsecs are 3712 or 4000
or so then all is fine. My small program does not check for errors and
may be screwing up here. Adding some error reporting may help. Or could
you give me access to a system with hpet timer? I could also test some
other things like the accuracy of time interpolation.

Re: [PATCH] 2.6.9-rc1 Ia64 build broken due to HPET

[Alex Williamson]

On Mon, 2004-09-13 at 13:28 -0700, Christoph Lameter wrote:
> On Mon, 13 Sep 2004, Alex Williamson wrote:
> 
> >
> >           Gettimeofday() = 1095104931.812600000
> >            CLOCK_REALTIME= 1095104931.812639404 resolution= 3712.000240992
> >           CLOCK_MONOTONIC=        369.103451704 resolution= 3712.000240992
> >  CLOCK_PROCESS_CPUTIME_ID=          0.000782450 resolution= 0.000000001
> >   CLOCK_THREAD_CPUTIME_ID=          0.000794115 resolution= 0.000000001
> >
> 
> That looks better but the resolution is still screwed up. The nanoseconds
> should be 3712 not the seconds.... The kernel clock_getres always sets the
> second part to zero. So does glibc when emulating CLOCK_REALTIME....
> 
> Could you figure out what is going on there? If the nsecs are 3712 or 4000
> or so then all is fine. My small program does not check for errors and
> may be screwing up here. Adding some error reporting may help. Or could
> you give me access to a system with hpet timer? I could also test some
> other things like the accuracy of time interpolation.

   There are several problems here.  The HPET really isn't coming into
play, and won't until we have systems that have a drifty ITC.  Here's
what's happening:

      * The itc_interpolator gets registered with a frequency of 1.2GHz
      * init_posix_timers() runs, setting the realtime and monotonic
        clock resolutions via time_interpolator_resolution().
        Unfortunately, 1*10^9/1.2*10^9 = 0.
      * hpet_init gets called after the fact, registering the hpet
        interpolator, but it's got nowhere near the frequency of the
        ITC.
      * sys_clock_getres() returns EINVAL because the clock resolutions
        are zero.

So, first and foremost, the ITC interpolator is broken for any ITC
greater than 1GHz.  Secondary, the HPET interpolator comes in too late,
and the code doesn't really seem to be able to support that.

	Alex

-- 
Alex Williamson                             HP Linux & Open Source Lab

Re: [PATCH] 2.6.9-rc1 Ia64 build broken due to HPET

[Christoph Lameter]

On Mon, 13 Sep 2004, Alex Williamson wrote:

> So, first and foremost, the ITC interpolator is broken for any ITC
> greater than 1GHz.  Secondary, the HPET interpolator comes in too late,
> and the code doesn't really seem to be able to support that.

Well. This means that the resolution is broken for any clock > 1GHZ.

The hpet interpolator is in a module? or is it compiled into the kernel?
register_time_interpolator allows a later registration of a time
interpolator. The question though is: Will it win against the one
already installed. It may win if you specify a smaller drift than the ITC
interpolator. Gotta look at that.

Could you try if the following patch fixes the precision problem:

Index: linux-2.6.9-rc1/kernel/timer.c
=================================--- linux-2.6.9-rc1.orig/kernel/timer.c 2004-09-13 08:14:46.000000000 -0700
+++ linux-2.6.9-rc1/kernel/timer.c      2004-09-13 15:20:36.000000000 -0700
@@ -1492,7 +1492,10 @@

 unsigned long time_interpolator_resolution(void)
 {
-       return NSEC_PER_SEC / time_interpolator->frequency;
+       if (time_interpolator->frequency < NSEC_PER_SEC)
+               return NSEC_PER_SEC / time_interpolator->frequency;
+       else
+               return 1;
 }

 #define GET_TI_NSECS(count,i) ((((count) - i->last_counter) *
i->nsec_per_cyc) >> i->shift)

Re: [PATCH] 2.6.9-rc1 Ia64 build broken due to HPET

[Alex Williamson]

On Mon, 2004-09-13 at 15:22 -0700, Christoph Lameter wrote:
> On Mon, 13 Sep 2004, Alex Williamson wrote:
> 
> > So, first and foremost, the ITC interpolator is broken for any ITC
> > greater than 1GHz.  Secondary, the HPET interpolator comes in too late,
> > and the code doesn't really seem to be able to support that.
> 
> Well. This means that the resolution is broken for any clock > 1GHZ.
> 

   Yep, and the patch below at least fixes that much:

          Gettimeofday() = 1095114506.777214000
           CLOCK_REALTIME= 1095114506.777283285 resolution= 0.000000001
          CLOCK_MONOTONIC=        188.577379016 resolution= 0.000000001
 CLOCK_PROCESS_CPUTIME_ID=          0.000864731 resolution= 0.000000001
  CLOCK_THREAD_CPUTIME_ID=          0.000883450 resolution= 0.000000001


> The hpet interpolator is in a module? or is it compiled into the kernel?

   Compiled in.  Here's what it looks like in dmesg (with some debug
printks added):

PAL Information Facility v0.5
perfmon: added sampling format default_format
perfmon_default_smpl: default_format v2.0 registered
time_interpolator_resolution() time_interpolator->frequency = 1200000000
register_posix_clock(0,...) res = 1
register_posix_clock(1,...) res = 1
Total HugeTLB memory allocated, 0
SGI XFS with large block/inode numbers, no debug enabled
Initializing Cryptographic API
GSI 18 (level, high) -> CPU 1 (0x0801) vector 71
GSI 19 (level, high) -> CPU 2 (0x0000) vector 72
GSI 20 (level, high) -> CPU 3 (0x0800) vector 73
hpet0: at MMIO 0xc0000ffffc082000, IRQs 71, 72, 73
hpet0: 4ns tick, 3 64-bit timers
GSI 122 (level, high) -> CPU 0 (0x0001) vector 74
GSI 123 (level, high) -> CPU 1 (0x0801) vector 75
GSI 124 (level, high) -> CPU 2 (0x0000) vector 76
hpet1: at MMIO 0xc0000ffffc002000, IRQs 74, 75, 76
hpet1: 4ns tick, 3 64-bit timers
register_time_interpolator() using this timer
EFI Time Services Driver v0.4

> register_time_interpolator allows a later registration of a time
> interpolator. The question though is: Will it win against the one
> already installed. It may win if you specify a smaller drift than the ITC
> interpolator. Gotta look at that.

   Yes, it does win, so apparently the drift is smaller (I only put in
enough printks so see that it won).  But the HPET interpolator is still
coming in after init_posix_timers(), so the monotonic and realtime
resolution isn't going to be updated to match the new timer without an
additional hook in there.  Thanks,

	Alex

-- 
Alex Williamson                             HP Linux & Open Source Lab

Re: [PATCH] 2.6.9-rc1 Ia64 build broken due to HPET

[Christoph Lameter]

On Mon, 13 Sep 2004, Alex Williamson wrote:

> > Well. This means that the resolution is broken for any clock > 1GHZ.
>    Yep, and the patch below at least fixes that much:

I checked that only the resolution is broken (unless the interpolator
shift is zero!). So the interpolators should now work fine at >1Ghz.

> > register_time_interpolator allows a later registration of a time
> > interpolator. The question though is: Will it win against the one
> > already installed. It may win if you specify a smaller drift than the ITC
> > interpolator. Gotta look at that.
>
>    Yes, it does win, so apparently the drift is smaller (I only put in
> enough printks so see that it won).  But the HPET interpolator is still
> coming in after init_posix_timers(), so the monotonic and realtime
> resolution isn't going to be updated to match the new timer without an
> additional hook in there.  Thanks,

Hmm... So if it wins and the posix time system is already up when
register time_interpolator is called then we need to redo
the posix timer setup or meddle with CLOCK_REALTIME and CLOCK_MONOTONIC
accuracy? Uggh....