[Bug libc/220] New: IA64: return EINVAL if ITC is an unreliable time source on clock_gettime(CLOCK_PROCESS_TIMEID)

[Bug libc/220] New: IA64: return EINVAL if ITC is an unreliable time source on clock_gettime(CLOCK_PROCESS_TIMEID)

[clameter at sgi dot com]

ITC on an IA64 may not be a reliable source of timing information if the CPUs in
an SMP system do not have synchronized ITC values. 

The clock_getcputimeid() function call returns ENOENT in cases where the ITC is
not reliable. However, the ITC is also the source for the CLOCK_PROCESS_TIMEID
and CLOCK_THREAD_TIMEID clocks. Applications may use this clock and fail because
the timers are not reliable when a process is migrated to a different cpu. The
patch below makes clock_gettime() also return NOENT in cases where ITC is
unreliable.

Index: glibc-2.3.2-200309260658/elf/rtld-Rules
===================================================================
--- glibc-2.3.2-200309260658.orig/elf/rtld-Rules	2003-07-24 20:31:13.000000000 -0700
+++ glibc-2.3.2-200309260658/elf/rtld-Rules	2004-06-03 18:46:55.000000000 -0700
@@ -31,7 +31,7 @@
 ifeq ($(subdir),elf)
 
 ifndef rtld-subdirs
-error This makefile is a subroutine of elf/Makefile not to be used directly
+$(error This makefile is a subroutine of elf/Makefile not to be used directly)
 endif
 
 include ../Makeconfig
Index: glibc-2.3.2-200309260658/sysdeps/ia64/Makefile
===================================================================
--- glibc-2.3.2-200309260658.orig/sysdeps/ia64/Makefile	2004-06-03
18:44:10.000000000 -0700
+++ glibc-2.3.2-200309260658/sysdeps/ia64/Makefile	2004-06-03 18:46:55.000000000
-0700
@@ -12,7 +12,7 @@
 
 ifeq (yes,$(build-shared))
 # Compatibility
-sysdep_routines += libgcc-compat
+sysdep_routines += libgcc-compat 
 shared-only-routines += libgcc-compat
 endif
 endif
@@ -21,4 +21,15 @@
 sysdep-dl-routines += dl-symaddr dl-fptr
 sysdep_routines += $(sysdep-dl-routines)
 sysdep-rtld-routines += $(sysdep-dl-routines)
+sysdep-dl-routines += hp-timing
 endif
+
+ifeq ($(subdir),posix)
+sysdep_routines += hp-timing
+endif
+
+ifeq ($(subdir),malloc)
+libmemusage-sysdep_routines += hp-timing
+endif
+
+
Index: glibc-2.3.2-200309260658/sysdeps/ia64/hp-timing.c
===================================================================
--- glibc-2.3.2-200309260658.orig/sysdeps/ia64/hp-timing.c	2002-01-31
23:48:41.000000000 -0800
+++ glibc-2.3.2-200309260658/sysdeps/ia64/hp-timing.c	2004-06-04
08:29:06.000000000 -0700
@@ -18,7 +18,61 @@
    Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
    02111-1307 USA.  */
 
+#include <errno.h>
+#include <time.h>
+#include <unistd.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <sys/time.h>
+#include <fcntl.h>
+
 #include <hp-timing.h>
 
 /* We have to define the variable for the overhead.  */
 hp_timing_t _dl_hp_timing_overhead;
+
+static int freq;
+extern hp_timing_t __get_clockfreq (void);
+
+/* Values
+   itc_drifting ==-1 using ITC
+   itc_drifting == 1 using CLOCK_REALTIME
+   itc_drifting == 0 ITC drift status undetermined
+*/
+int itc_drifting=0;
+
+unsigned long int hp_gettimestamp(void)
+{
+	if (itc_drifting>0)
+get_realtimestamp:
+	{
+		struct timeval t;
+		unsigned long usecs;
+		
+		/* Make sure that CPU clock frequency has already been determined */
+		if (freq==0) 
+		{
+			freq=__get_clockfreq()/1000000UL;
+			if (freq==0) freq=10;
+		}
+		/* clock_gettime is not working in the elf loader... use gettimeofday */
+		__gettimeofday(&t,NULL);
+		usecs=t.tv_usec+t.tv_sec*1000000UL;
+		/* properly scale the microseconds according to the CPU clock rate */
+		return usecs*freq;
+	}
+	else
+	{
+		unsigned long x;
+
+		if (itc_is_drifting())
+		{
+			itc_drifting=1;
+			goto get_realtimestamp;
+		}
+		itc_drifting=-1;
+		HP_TIMING_NOW(x);
+		return x;
+	}
+}
+
Index: glibc-2.3.2-200309260658/sysdeps/ia64/hp-timing.h
===================================================================
--- glibc-2.3.2-200309260658.orig/sysdeps/ia64/hp-timing.h	2003-07-20
14:51:49.000000000 -0700
+++ glibc-2.3.2-200309260658/sysdeps/ia64/hp-timing.h	2004-06-03
18:46:55.000000000 -0700
@@ -72,11 +72,13 @@
 /* We always assume having the timestamp register.  */
 #define HP_TIMING_AVAIL		(1)
 
-/* We indeed have inlined functions.  */
-#define HP_TIMING_INLINE	(1)
+/* We indeed have inlined functions but sometimes we need to make a call.  */
+#define HP_TIMING_INLINE	(0)
 
 /* We use 64bit values for the times.  */
 typedef unsigned long int hp_timing_t;
+int itc_drifting;
+unsigned long hp_gettimestamp(void);
 
 /* Set timestamp value to zero.  */
 #define HP_TIMING_ZERO(Var)	(Var) = (0)
@@ -91,10 +93,13 @@
 /* That's quite simple.  Use the `ar.itc' instruction.  */
 #define HP_TIMING_NOW(Var) \
   ({ unsigned long int __itc;						      \
+    if (itc_drifting>=0) Var=hp_gettimestamp();				\
+    else {								\
      do									      \
        asm volatile ("mov %0=ar.itc" : "=r" (__itc) : : "memory");	      \
-     while (REPEAT_READ (__itc));					      \
-     Var = __itc; })
+      while (REPEAT_READ (__itc));					      \
+      Var = __itc; }							\
+   })
 
 /* Use two 'ar.itc' instructions in a row to find out how long it takes.  */
 #define HP_TIMING_DIFF_INIT() \
Index: glibc-2.3.2-200309260658/sysdeps/unix/sysv/linux/ia64/Makefile
===================================================================
--- glibc-2.3.2-200309260658.orig/sysdeps/unix/sysv/linux/ia64/Makefile
2003-06-02 12:56:22.000000000 -0700
+++ glibc-2.3.2-200309260658/sysdeps/unix/sysv/linux/ia64/Makefile	2004-06-03
18:46:55.000000000 -0700
@@ -7,6 +7,11 @@
 gen-as-const-headers += sigcontext-offsets.sym
 endif
 
+ifeq ($(subdir), csu)
+sysdep_routines += get_clockfreq
+static-only-routines += get_clockfreq
+endif
+
 ifeq ($(subdir),misc)
 sysdep_headers += sys/io.h
 sysdep_routines += ioperm clone2
@@ -16,6 +21,15 @@
 sysdep-dl-routines += dl-static
 sysdep_routines += $(sysdep-dl-routines)
 sysdep-rtld-routines += $(sysdep-dl-routines)
+sysdep-dl-routines += get_clockfreq
+endif
+
+ifeq ($(subdir),malloc)
+libmemusage-sysdep_routines += get_clockfreq
+endif
+
+ifeq ($(subdir),posix)
+sysdep_routines += get_clockfreq
 endif
 
 # This is a crude attempt to silence the compiler which complains about
Index: glibc-2.3.2-200309260658/sysdeps/unix/sysv/linux/ia64/hp-timing.c
===================================================================
--- glibc-2.3.2-200309260658.orig/sysdeps/unix/sysv/linux/ia64/hp-timing.c
1969-12-31 16:00:00.000000000 -0800
+++ glibc-2.3.2-200309260658/sysdeps/unix/sysv/linux/ia64/hp-timing.c	2004-06-03
18:46:55.000000000 -0700
@@ -0,0 +1,49 @@
+/* Support for high precision, low overhead timing functions.  IA-64 version.
+   Copyright (C) 2001, 2002 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+   Contributed by Ulrich Drepper <drepper@cygnus.com>, 2001.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, write to the Free
+   Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
+   02111-1307 USA.  */
+
+#include <errno.h>
+#include <time.h>
+#include <unistd.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <sys/time.h>
+#include <fcntl.h>
+
+int itc_is_drifting(void)
+{
+	int itc_drifting=0;
+
+	int fd = open ("/proc/sal/itc_drift", O_RDONLY);
+
+	if (__builtin_expect (fd != -1, 1))
+	{
+		char buf[16];
+		/* We expect the file to contain a single digit followed by
+		   a newline.  If the format changes we better not rely on
+		   the file content.  */
+		if (read (fd, buf, sizeof buf) != 2 || buf[0] != '0' || buf[1] != '\n')
+			itc_drifting=1;
+		close (fd);
+	}
+	return itc_drifting;
+}
+
+#include <sysdeps/ia64/hp-timing.c>
+

-- 
           Summary: IA64: return EINVAL if ITC is an unreliable time source
                    on clock_gettime(CLOCK_PROCESS_TIMEID)
           Product: glibc
           Version: 2.3.3
            Status: NEW
          Severity: normal
          Priority: P2
         Component: libc
        AssignedTo: gotom at debian dot or dot jp
        ReportedBy: clameter at sgi dot com
                CC: glibc-bugs at sources dot redhat dot com
 GCC build triplet: ia64-redhat-linux
  GCC host triplet: ia64-redhat-linux
GCC target triplet: ia64-redhat-linux


http://sources.redhat.com/bugzilla/show_bug.cgi?id=220

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/220] IA64: return EINVAL if ITC is an unreliable time source on clock_gettime(CLOCK_PROCESS_TIMEID)

[clameter at sgi dot com]

------- Additional Comments From clameter at sgi dot com  2004-06-14 20:50 -------
Sorry this was the wrong patch. This did far too much. Here is the right one:

Index: libc/sysdeps/unix/sysv/linux/ia64/clock_getcpuclockid.c
===================================================================
--- libc.orig/sysdeps/unix/sysv/linux/ia64/clock_getcpuclockid.c	2004-05-27
18:26:58.170594467 -0700
+++ libc/sysdeps/unix/sysv/linux/ia64/clock_getcpuclockid.c	2004-06-09
11:27:09.270731016 -0700
@@ -23,6 +23,7 @@
 #include <sys/types.h>
 #include <fcntl.h>
 
+extern int itc_usable;
 
 int
 clock_getcpuclockid (pid_t pid, clockid_t *clock_id)
@@ -31,7 +32,6 @@
   if (pid != 0 && pid != getpid ())
     return EPERM;
 
-  static int itc_usable;
   int retval = ENOENT;
 
   if (__builtin_expect (itc_usable == 0, 0))
Index: libc/sysdeps/unix/clock_gettime.c
===================================================================
--- libc.orig/sysdeps/unix/clock_gettime.c	2004-06-09 11:30:47.381079907 -0700
+++ libc/sysdeps/unix/clock_gettime.c	2004-06-09 11:37:09.235567416 -0700
@@ -29,7 +29,7 @@
    because some jokers are already playing with processors with more
    than 4GHz.  */
 static hp_timing_t freq;
-
+int itc_usable;
 
 /* This function is defined in the thread library.  */
 extern int __pthread_clock_gettime (clockid_t clock_id, hp_timing_t freq,
@@ -78,7 +78,9 @@
 #if HP_TIMING_AVAIL
       /* FALLTHROUGH.  */
     case CLOCK_PROCESS_CPUTIME_ID:
-      {
+      /* If itc usability has not been determined then do so now */
+      if (itc_usable==0) clock_getcpuclockid();
+      if (itc_usable>0) {
 	hp_timing_t tsc;
 
 	if (__builtin_expect (freq == 0, 0))
@@ -115,6 +117,9 @@
 
 	retval = 0;
       }
+    else
+	_set_errno(ENOENT);
+    
     break;
 #endif
     }

-- 


http://sources.redhat.com/bugzilla/show_bug.cgi?id=220

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/220] IA64: return EINVAL if ITC is an unreliable time source on clock_gettime(CLOCK_PROCESS_TIMEID)

[schwab at suse dot de]

------- Additional Comments From schwab at suse dot de  2004-06-14 21:50 -------
Still you should follow the GNU coding conventions, and put patches in 
attachments so that they can be retrieved unmangled. 

-- 


http://sources.redhat.com/bugzilla/show_bug.cgi?id=220

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/220] IA64: return EINVAL if ITC is an unreliable time source on clock_gettime(CLOCK_PROCESS_TIMEID)

[clameter at sgi dot com]

------- Additional Comments From clameter at sgi dot com  2004-06-14 22:24 -------
Created an attachment (id=114)
 --> (http://sources.redhat.com/bugzilla/attachment.cgi?id=114&action=view)
Patch as an attachment


-- 


http://sources.redhat.com/bugzilla/show_bug.cgi?id=220

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/220] IA64: return EINVAL if ITC is an unreliable time source on clock_gettime(CLOCK_PROCESS_TIMEID)

[clameter at sgi dot com]

------- Additional Comments From clameter at sgi dot com  2004-06-14 22:25 -------
Subject: Re:  IA64: return EINVAL if ITC is an unreliable time
 source on clock_gettime(CLOCK_PROCESS_TIMEID)

I must somehow missed that options. I was looking for it. When I followed
your link I found the function to post an attachment.

On Mon, 14 Jun 2004, schwab at suse dot de wrote:

>
> ------- Additional Comments From schwab at suse dot de  2004-06-14 21:50 -------
> Still you should follow the GNU coding conventions, and put patches in
> attachments so that they can be retrieved unmangled.
>
> --
>
>
> http://sources.redhat.com/bugzilla/show_bug.cgi?id=220
>
> ------- You are receiving this mail because: -------
> You reported the bug, or are watching the reporter.
>


-- 


http://sources.redhat.com/bugzilla/show_bug.cgi?id=220

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/220] IA64: return EINVAL if ITC is an unreliable time source on clock_gettime(CLOCK_PROCESS_TIMEID)

[jakub at redhat dot com]

------- Additional Comments From jakub at redhat dot com  2004-06-15 07:57 -------
FYI this patch breaks all other targets.

-- 


http://sources.redhat.com/bugzilla/show_bug.cgi?id=220

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/220] IA64: return EINVAL if ITC is an unreliable time source on clock_gettime(CLOCK_PROCESS_TIMEID)

[clameter at sgi dot com]

------- Additional Comments From clameter at sgi dot com  2004-06-15 21:56 -------
Created an attachment (id=115)
 --> (http://sources.redhat.com/bugzilla/attachment.cgi?id=115&action=view)
Revised Patch. Make arch independent pieces generic.

Here is a revision of the patch to address the concerns:

1. Rename the variable to signal the unreliability of HP_TIMING to
hp_unreliable.

2. Make the changes to clock_gettime.c independent of what is done for IA64 so
that other arches can also tie into this scheme.


-- 


http://sources.redhat.com/bugzilla/show_bug.cgi?id=220

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/220] IA64: return EINVAL if ITC is an unreliable time source on clock_gettime(CLOCK_PROCESS_TIMEID)

[drepper at redhat dot com]

------- Additional Comments From drepper at redhat dot com  2004-08-10 02:14 -------
I have said several times already that there is no reason for this change.  The
clock_getcpuclockid provides the information.  There is no reason to duplicate
all this work over and over again in the gettime function.  If somebody
disregards the information that the clock is not available they don't deserve
better.  And maybe somebody wants to continue using the information, e.g., if a
process is tied to one specific processor.  Then the itc value is usable.

Beside, despite being told you have to follow the coding standard you chose to
ignore this.  That's certainly a good way to get any of your changes accepted.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |WONTFIX


http://sources.redhat.com/bugzilla/show_bug.cgi?id=220

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/220] IA64: return EINVAL if ITC is an unreliable time source on clock_gettime(CLOCK_PROCESS_TIMEID)

[clameter at sgi dot com]

------- Additional Comments From clameter at sgi dot com  2004-08-10 17:17 -------
Subject: Re:  IA64: return EINVAL if ITC is an unreliable time
 source on clock_gettime(CLOCK_PROCESS_TIMEID)

I thought this was the reason behind it but it would have been good to
have this clarificatione earlier. I have submitted a change to the
manpage for clock_gettime to explain the need to call clock_getcpuclockid
and the difficulties with the use of CPU timer registers in SMP settings
last Friday.

On Mon, 10 Aug 2004, drepper at redhat dot com wrote:

>
> ------- Additional Comments From drepper at redhat dot com  2004-08-10 02:14 -------
> I have said several times already that there is no reason for this change.  The
> clock_getcpuclockid provides the information.  There is no reason to duplicate
> all this work over and over again in the gettime function.  If somebody
> disregards the information that the clock is not available they don't deserve
> better.  And maybe somebody wants to continue using the information, e.g., if a
> process is tied to one specific processor.  Then the itc value is usable.
>
> Beside, despite being told you have to follow the coding standard you chose to
> ignore this.  That's certainly a good way to get any of your changes accepted.
>
> --
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>              Status|NEW                         |RESOLVED
>          Resolution|                            |WONTFIX
>
>
> http://sources.redhat.com/bugzilla/show_bug.cgi?id=220
>
> ------- You are receiving this mail because: -------
> You reported the bug, or are watching the reporter.
>


-- 


http://sources.redhat.com/bugzilla/show_bug.cgi?id=220

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.