Re: Posix compliant process clock patch for the linux arch in glibc

[Andreas Jaeger <aj at suse dot de>]

Christoph Lameter <clameter@sgi.com> writes:

> Patches were recently accepted into Linus tree for 2.6.10 to implement
> posix compliant process clocks. The current glibc code does not provide
> access to process and thread clocks of other processes and contains an
> implementation of CLOCK_PROCESS_CPUTIME_ID and CLOCK_THREAD_CPUTIME_ID
> that returns the time that the process/thread has been running instead of
> the cputime spend on the process. This patch makes glibc to use the new
> linux kernel abilities instead of improvising these clocks. In addition
> clockids unknown to glibc are forwarded the kernel instead of glibc
> returning an error immediately. This allow the use of new clocks provided
> by the kernel such as CLOCK_SGI_CYCLE.

Looking at your code, I do not understand how a glibc with your patch
will work on a kernel that does not have the 2.6.10 patches in it?

> [...]

> Index: libc/nptl/sysdeps/pthread/pthread_getcpuclockid.c
> ===================================================================
> --- libc.orig/nptl/sysdeps/pthread/pthread_getcpuclockid.c	2003-06-24 16:53:20.000000000 -0700
> +++ libc/nptl/sysdeps/pthread/pthread_getcpuclockid.c	2004-10-21 11:05:11.295178056 -0700
> @@ -20,7 +20,7 @@
>  #include <pthreadP.h>
>  #include <sys/time.h>
>  #include <tls.h>
> -
> +#include <linux/threads.h>
>
>  int
>  pthread_getcpuclockid (threadid, clockid)
> @@ -35,19 +35,8 @@
>      return ESRCH;
>
>  #ifdef CLOCK_THREAD_CPUTIME_ID
> -  /* We need to store the thread ID in the CLOCKID variable together
> -     with a number identifying the clock.  We reserve the low 3 bits
> -     for the clock ID and the rest for the thread ID.  This is
> -     problematic if the thread ID is too large.  But 29 bits should be
> -     fine.
> -
> -     If some day more clock IDs are needed the ID part can be
> -     enlarged.  The IDs are entirely internal.  */
> -  if (pd->tid >= 1 << (8 * sizeof (*clockid) - CLOCK_IDFIELD_SIZE))
> -    return ERANGE;
> -
>    /* Store the number.  */
> -  *clockid = CLOCK_THREAD_CPUTIME_ID | (pd->tid << CLOCK_IDFIELD_SIZE);
> +  *clockid = - (pd->tid + PID_MAX_LIMIT);

The above will only work if the running kernel has your patch in it.
For older kernels, it will break.  This is not acceptable and the code
has to be written in such a way that it works in both cases.

So, how can we figure out that we're running a fixed kernel?

You should do the following:
- Have a look at sysdeps/unix/sysv/linux/kernel-features.h
- add an __ASSUME_POSIX_CLOCK to kernel-features.h
- Write your code so that you do the following (check other files in
  the linux subdirectoy how this is done:

#if __ASSUME_POSIX_CLOCK
  your new code
#else
  static int have_posix_clock = -1;
  if (have__support_posix_clock < 0)
    check whether kernel contains your code, save it in the var
  if (have__support_posix_clock)
    ...
  else
    ...

  Depending on your check, there might be other ways to write the code
  (check e.g. ftruncate64.c).

Without these changes, your patch is not usable to add to glibc.
People want to have one glibc that runs on any kernel...

Andreas
-- 
 Andreas Jaeger, aj@suse.de, http://www.suse.de/~aj
  SUSE Linux AG, Maxfeldstr. 5, 90409 NÃrnberg, Germany
   GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126

Attachment: pgp00000.pgp
Description: PGP signature