public inbox for cygwin-patches@cygwin.com
 help / color / mirror / Atom feed
* [PATCH] clock_nanosleep(2)
@ 2011-07-20  1:54 Yaakov (Cygwin/X)
  2011-07-20  7:57 ` Corinna Vinschen
  2011-07-21 10:39 ` Corinna Vinschen
  0 siblings, 2 replies; 28+ messages in thread
From: Yaakov (Cygwin/X) @ 2011-07-20  1:54 UTC (permalink / raw)
  To: cygwin-patches

[-- Attachment #1: Type: text/plain, Size: 621 bytes --]

This patchset implements the POSIX clock_nanosleep(2) function:

http://pubs.opengroup.org/onlinepubs/9699919799/functions/clock_nanosleep.html
http://www.kernel.org/doc/man-pages/online/pages/man2/clock_nanosleep.2.html

In summary, clock_nanosleep(2) replaces nanosleep(2) as the primary
sleeping function, with all others rewritten in terms of the former.  It
also restores maximum precision to hires_ms::resolution(), saving the
<5000 100ns check for the one place where resolution is rounded off.

Patches for newlib, winsup/cygwin, and winsup/doc attached.  I would
appreciate a careful look at this one.


Yaakov


[-- Attachment #2: newlib-clock_nanosleep.patch --]
[-- Type: text/x-patch, Size: 600 bytes --]

Index: libc/include/time.h
===================================================================
RCS file: /cvs/src/src/newlib/libc/include/time.h,v
retrieving revision 1.19
diff -u -r1.19 time.h
--- libc/include/time.h	16 Oct 2008 21:53:58 -0000	1.19
+++ libc/include/time.h	15 May 2011 19:22:48 -0000
@@ -168,6 +168,9 @@
 
 /* High Resolution Sleep, P1003.1b-1993, p. 269 */
 
+int _EXFUN(clock_nanosleep,
+  (clockid_t clock_id, int flags, const struct timespec *rqtp,
+   struct timespec *rmtp));
 int _EXFUN(nanosleep, (const struct timespec  *rqtp, struct timespec *rmtp));
 
 #ifdef __cplusplus

[-- Attachment #3: winsup-cygwin-clock_nanosleep.patch --]
[-- Type: text/x-patch, Size: 11583 bytes --]

2011-07-19  Yaakov Selkowitz  <yselkowitz@...>

	* cygwin.din (clock_nanosleep): Export.
	* hires.h (hires_ns::msecs): New function.
	(ntod): Declare.
	* posix.sgml (std-notimpl): Move clock_nanosleep from here...
	(std-susv4): ... to here.
	(std-notes): Note limitations of clock_nanosleep.
	* signal.cc (now_msecs): New static function.
	(clock_nanosleep): Renamed from nanosleep with additional arguments.
	Add support for CLOCK_MONOTONIC and TIMER_ABSTIME.
	(nanosleep): Rewrite in terms of clock_nanosleep.
	(sleep): Ditto.
	(usleep): Ditto.
	* thread.cc: Mark clock_nanosleep in list of cancellation points.
	* times.cc (hires_ms::resolution): Provide best possible resolution.
	(clock_getres): Adjust accordingly.
	(clock_setres): Ditto.
	* include/cygwin/version.h (CYGWIN_VERSION_API_MINOR): Bump.

Index: cygwin.din
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/cygwin.din,v
retrieving revision 1.244
diff -u -p -r1.244 cygwin.din
--- cygwin.din	19 May 2011 07:23:28 -0000	1.244
+++ cygwin.din	19 Jul 2011 01:28:56 -0000
@@ -223,6 +223,7 @@ _clock = clock SIGFE
 clock_getcpuclockid SIGFE
 clock_getres SIGFE
 clock_gettime SIGFE
+clock_nanosleep SIGFE
 clock_setres SIGFE
 clock_settime SIGFE
 clog NOSIGFE
Index: hires.h
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/hires.h,v
retrieving revision 1.19
diff -u -p -r1.19 hires.h
--- hires.h	17 May 2011 17:08:09 -0000	1.19
+++ hires.h	19 Jul 2011 01:28:56 -0000
@@ -45,6 +45,7 @@ class hires_ns : public hires_base
  public:
   LONGLONG nsecs ();
   LONGLONG usecs () {return nsecs () / 1000LL;}
+  LONGLONG msecs () { return nsecs () / 1000000LL; }
   LONGLONG resolution();
 };
 
@@ -63,4 +64,5 @@ class hires_ms : public hires_base
 };
 
 extern hires_ms gtod;
+extern hires_ns ntod;
 #endif /*__HIRES_H__*/
Index: posix.sgml
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/posix.sgml,v
retrieving revision 1.68
diff -u -p -r1.68 posix.sgml
--- posix.sgml	25 May 2011 06:10:24 -0000	1.68
+++ posix.sgml	19 Jul 2011 01:28:56 -0000
@@ -92,6 +92,7 @@ also IEEE Std 1003.1-2008 (POSIX.1-2008)
     clock_getcpuclockid
     clock_getres
     clock_gettime
+    clock_nanosleep		(see chapter "Implementation Notes")
     clock_settime		(see chapter "Implementation Notes")
     clog
     clogf
@@ -1297,7 +1298,6 @@ also IEEE Std 1003.1-2008 (POSIX.1-2008)
     ceill
     cexpl
     cimagl
-    clock_nanosleep
     clogl
     conjl
     copysignl
@@ -1446,8 +1446,10 @@ by keeping track of the current root and
 related function calls.  A real chroot functionality is not supported by
 Windows however.</para>
 
-<para><function>clock_setres</function>, <function>clock_settime</function>,
-and <function>timer_create</function> only support CLOCK_REALTIME.</para>
+<para><function>clock_nanosleep</function> currently supports only
+CLOCK_REALTIME and CLOCK_MONOTONIC.  <function>clock_setres</function>,
+<function>clock_settime</function>, and <function>timer_create</function>
+currently support only CLOCK_REALTIME.</para>
 
 <para>BSD file locks created via <function>flock</function> are not
 propagated to the parent process and sibling processes.  The locks are
Index: signal.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/signal.cc,v
retrieving revision 1.98
diff -u -p -r1.98 signal.cc
--- signal.cc	10 Jul 2011 00:01:33 -0000	1.98
+++ signal.cc	19 Jul 2011 01:28:56 -0000
@@ -80,62 +80,113 @@ signal (int sig, _sig_func_ptr func)
   return prev;
 }
 
+static long long
+now_msecs (clockid_t clk_id, bool abstime)
+{
+  long long res = 0LL;
+  switch (clk_id)
+    {
+    case CLOCK_REALTIME:
+      if (abstime)
+	res = gtod.msecs ();
+      else
+	res = gtod.dmsecs ();
+      break;
+    case CLOCK_MONOTONIC:
+      res = ntod.msecs ();
+      break;
+    }
+  return res;
+}
+
 extern "C" int
-nanosleep (const struct timespec *rqtp, struct timespec *rmtp)
+clock_nanosleep (clockid_t clk_id, int flags, const struct timespec *rqtp, struct timespec *rmtp)
 {
+  const bool abstime = (flags & TIMER_ABSTIME) ? true : false;
   int res = 0;
   sig_dispatch_pending ();
   pthread_testcancel ();
 
-  if ((unsigned int) rqtp->tv_nsec > 999999999)
-    {
-      set_errno (EINVAL);
-      return -1;
-    }
-  unsigned int sec = rqtp->tv_sec;
-  DWORD resolution = gtod.resolution ();
+  if (rqtp->tv_nsec > 999999999L || rqtp->tv_nsec < 0L)
+    return EINVAL;
+
+  /* disallowed by POSIX */
+  if (clk_id == CLOCK_THREAD_CPUTIME_ID)
+    return EINVAL;
+
+  if (CLOCKID_IS_PROCESS (clk_id) || CLOCKID_IS_THREAD (clk_id))
+    return ENOTSUP;
+
+  long long msec = (rqtp->tv_sec * 1000LL) + (rqtp->tv_nsec + 999999LL) / 1000000LL;
+  long long end_time;
+  DWORD resolution;
   bool done = false;
   DWORD req;
-  DWORD rem;
+  long long rem;
+
+  switch (clk_id)
+    {
+    case CLOCK_REALTIME:
+      resolution = gtod.resolution () / 10000L;
+      break;
+    case CLOCK_MONOTONIC:
+      resolution = ntod.resolution () / 1000000L;
+      break;
+    default:
+      return EINVAL;
+    }
+
+  /* The resolution can be as low as 5000 100ns intervals on recent OSes.
+     We have to make sure that the resolution in ms is never 0. */
+  if (!resolution)
+    resolution = 1L;
+
+  if (abstime)
+    {
+      end_time = msec;
+      msec -= now_msecs (clk_id, abstime);
+    }
+  else
+    end_time = now_msecs (clk_id, abstime) + msec;
+
+  if (msec < 0)
+    return 0;
 
   while (!done)
     {
       /* Divide user's input into transactions no larger than 49.7
 	 days at a time.  */
-      if (sec > HIRES_DELAY_MAX / 1000)
+      if (msec > HIRES_DELAY_MAX)
 	{
 	  req = ((HIRES_DELAY_MAX + resolution - 1)
 		 / resolution * resolution);
-	  sec -= HIRES_DELAY_MAX / 1000;
+	  msec -= HIRES_DELAY_MAX;
 	}
       else
 	{
-	  req = ((sec * 1000 + (rqtp->tv_nsec + 999999) / 1000000
-		  + resolution - 1) / resolution) * resolution;
-	  sec = 0;
+	  req = (((DWORD) msec + resolution - 1) / resolution) * resolution;
+	  msec = 0;
 	  done = true;
 	}
 
-      DWORD end_time = gtod.dmsecs () + req;
-      syscall_printf ("nanosleep (%ld)", req);
+      syscall_printf ("clock_nanosleep (%ld)", req);
 
       int rc = cancelable_wait (signal_arrived, req);
-      if ((rem = end_time - gtod.dmsecs ()) > HIRES_DELAY_MAX)
+      if ((rem = end_time - now_msecs (clk_id, abstime)) > HIRES_DELAY_MAX)
 	rem = 0;
       if (rc == WAIT_OBJECT_0)
 	{
 	  _my_tls.call_signal_handler ();
-	  set_errno (EINTR);
-	  res = -1;
+	  res = EINTR;
 	  break;
 	}
     }
 
-  if (rmtp)
+  if (rmtp && !abstime)
     {
-      rmtp->tv_sec = sec + rem / 1000;
-      rmtp->tv_nsec = (rem % 1000) * 1000000;
-      if (sec)
+      rmtp->tv_sec = (time_t) ((msec + rem) / 1000);
+      rmtp->tv_nsec = (long) ((rem % 1000L) * 1000000L);
+      if (msec)
 	{
 	  rmtp->tv_nsec += rqtp->tv_nsec;
 	  if (rmtp->tv_nsec >= 1000000000)
@@ -146,17 +197,30 @@ nanosleep (const struct timespec *rqtp, 
 	}
     }
 
-  syscall_printf ("%d = nanosleep (%ld, %ld)", res, req, rem);
+  syscall_printf ("%d = clock_nanosleep (%lu, %d, %ld, %ld)",
+                  res, clk_id, flags, req, rem);
   return res;
 }
 
+extern "C" int
+nanosleep (const struct timespec *rqtp, struct timespec *rmtp)
+{
+  int res = clock_nanosleep (CLOCK_REALTIME, 0, rqtp, rmtp);
+  if (res != 0)
+    {
+      set_errno (res);
+      return -1;
+    }
+  return 0;
+}
+
 extern "C" unsigned int
 sleep (unsigned int seconds)
 {
   struct timespec req, rem;
   req.tv_sec = seconds;
   req.tv_nsec = 0;
-  if (nanosleep (&req, &rem))
+  if (clock_nanosleep (CLOCK_REALTIME, 0, &req, &rem))
     return rem.tv_sec + (rem.tv_nsec > 0);
   return 0;
 }
@@ -167,8 +231,13 @@ usleep (useconds_t useconds)
   struct timespec req;
   req.tv_sec = useconds / 1000000;
   req.tv_nsec = (useconds % 1000000) * 1000;
-  int res = nanosleep (&req, NULL);
-  return res;
+  int res = clock_nanosleep (CLOCK_REALTIME, 0, &req, NULL);
+  if (res != 0)
+    {
+      set_errno (res);
+      return -1;
+    }
+  return 0;
 }
 
 extern "C" int
Index: thread.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/thread.cc,v
retrieving revision 1.243
diff -u -p -r1.243 thread.cc
--- thread.cc	6 Jun 2011 05:02:13 -0000	1.243
+++ thread.cc	19 Jul 2011 01:28:57 -0000
@@ -577,7 +577,7 @@ pthread::cancel ()
 
     * accept ()
     o aio_suspend ()
-    o clock_nanosleep ()
+    * clock_nanosleep ()
     * close ()
     * connect ()
     * creat ()
Index: times.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/times.cc,v
retrieving revision 1.110
diff -u -p -r1.110 times.cc
--- times.cc	6 Jun 2011 05:02:13 -0000	1.110
+++ times.cc	19 Jul 2011 01:28:57 -0000
@@ -737,7 +737,7 @@ hires_ms::resolution ()
 
       status = NtQueryTimerResolution (&coarsest, &finest, &actual);
       if (NT_SUCCESS (status))
-	minperiod = (DWORD) actual / 10000L;
+	minperiod = (DWORD) actual;
       else
 	{
 	  /* Try to empirically determine current timer resolution */
@@ -757,13 +757,9 @@ hires_ms::resolution ()
 	      period += now - then;
 	    }
 	  SetThreadPriority (GetCurrentThread (), priority);
-	  period /= 40000L;
+	  period /= 4L;
 	  minperiod = (DWORD) period;
 	}
-      /* The resolution can be as low as 5000 100ns intervals on recent OSes.
-	 We have to make sure that the resolution in ms is never 0. */
-      if (!minperiod)
-	minperiod = 1L;
     }
   return minperiod;
 }
@@ -786,8 +782,8 @@ clock_getres (clockid_t clk_id, struct t
       case CLOCK_REALTIME:
 	{
 	  DWORD period = gtod.resolution ();
-	  tp->tv_sec = period / 1000;
-	  tp->tv_nsec = (period % 1000) * 1000000;
+	  tp->tv_sec = period / NSPERSEC;
+	  tp->tv_nsec = (period % NSPERSEC) * 100;
 	  break;
 	}
 
@@ -838,7 +834,7 @@ clock_setres (clockid_t clk_id, struct t
     }
 
   if (period_set
-      && NT_SUCCESS (NtSetTimerResolution (minperiod * 10000L, FALSE, &actual)))
+      && NT_SUCCESS (NtSetTimerResolution (minperiod, FALSE, &actual)))
     period_set = false;
 
   status = NtSetTimerResolution (period, TRUE, &actual);
@@ -847,11 +843,7 @@ clock_setres (clockid_t clk_id, struct t
       __seterrno_from_nt_status (status);
       return -1;
     }
-  minperiod = actual / 10000L;
-  /* The resolution can be as low as 5000 100ns intervals on recent OSes.
-     We have to make sure that the resolution in ms is never 0. */
-  if (!minperiod)
-    minperiod = 1L;
+  minperiod = actual;
   period_set = true;
   return 0;
 }
Index: include/cygwin/version.h
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/include/cygwin/version.h,v
retrieving revision 1.349
diff -u -p -r1.349 version.h
--- include/cygwin/version.h	19 May 2011 07:23:29 -0000	1.349
+++ include/cygwin/version.h	19 Jul 2011 01:28:57 -0000
@@ -417,12 +417,13 @@ details. */
       247: Export error, error_at_line, error_message_count, error_one_per_line,
 	   error_print_progname.
       248: Export __fpurge.
+      249: Export clock_nanosleep.
      */
 
      /* Note that we forgot to bump the api for ualarm, strtoll, strtoull */
 
 #define CYGWIN_VERSION_API_MAJOR 0
-#define CYGWIN_VERSION_API_MINOR 248
+#define CYGWIN_VERSION_API_MINOR 249
 
      /* There is also a compatibity version number associated with the
 	shared memory regions.  It is incremented when incompatible

[-- Attachment #4: winsup-doc-clock_nanosleep.patch --]
[-- Type: text/x-patch, Size: 1400 bytes --]

2011-07-19  Yaakov Selkowitz  <yselkowitz@...>

	* new-features.sgml (ov-new1.7.10): Document clock_nanosleep(2).

Index: new-features.sgml
===================================================================
RCS file: /cvs/src/src/winsup/doc/new-features.sgml,v
retrieving revision 1.87
diff -u -p -r1.87 new-features.sgml
--- new-features.sgml	20 Jul 2011 01:19:53 -0000	1.87
+++ new-features.sgml	20 Jul 2011 01:27:23 -0000
@@ -41,9 +41,11 @@ pthread_attr_setstackaddr, pthread_attr_
 </para></listitem>
 
 <listitem><para>
-clock_gettime(3) and clock_getres(3) accept per-process and per-thread CPU-time
-clocks, including CLOCK_PROCESS_CPUTIME_ID and CLOCK_THREAD_CPUTIME_ID.
-New APIs: clock_getcpuclockid, pthread_getcpuclockid.
+clock_gettime(2) and clock_getres(2) accept per-process and per-thread
+CPU-time clocks, including CLOCK_PROCESS_CPUTIME_ID and CLOCK_THREAD_CPUTIME_ID.
+New POSIX clock APIs: clock_nanosleep (supports CLOCK_REALTIME and
+CLOCK_MONOTONIC), clock_settime (supports CLOCK_REALTIME),
+clock_getcpuclockid, pthread_getcpuclockid.
 </para></listitem>
 
 <listitem><para>
@@ -73,7 +75,7 @@ as well as the version of GCC used when 
 </para></listitem>
 
 <listitem><para>
-Other new API: clock_settime, __fpurge, ppoll, psiginfo, psignal, sys_siglist,
+Other new API: __fpurge, ppoll, psiginfo, psignal, sys_siglist,
 pthread_setschedprio, sysinfo.
 </para></listitem>
 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] clock_nanosleep(2)
  2011-07-20  1:54 [PATCH] clock_nanosleep(2) Yaakov (Cygwin/X)
@ 2011-07-20  7:57 ` Corinna Vinschen
  2011-07-20  9:16   ` Yaakov (Cygwin/X)
  2011-07-20 17:37   ` [PATCH] clock_nanosleep(2) Christopher Faylor
  2011-07-21 10:39 ` Corinna Vinschen
  1 sibling, 2 replies; 28+ messages in thread
From: Corinna Vinschen @ 2011-07-20  7:57 UTC (permalink / raw)
  To: cygwin-patches

Hi Yaakov,

On Jul 19 20:54, Yaakov (Cygwin/X) wrote:
> This patchset implements the POSIX clock_nanosleep(2) function:
> 
> http://pubs.opengroup.org/onlinepubs/9699919799/functions/clock_nanosleep.html
> http://www.kernel.org/doc/man-pages/online/pages/man2/clock_nanosleep.2.html
> 
> In summary, clock_nanosleep(2) replaces nanosleep(2) as the primary
> sleeping function, with all others rewritten in terms of the former.  It
> also restores maximum precision to hires_ms::resolution(), saving the
> <5000 100ns check for the one place where resolution is rounded off.

I like this, it's probably not only faster but it makes the code better
readable.  But let's talk about the newlib side first.

> Index: libc/include/time.h
> ===================================================================
> RCS file: /cvs/src/src/newlib/libc/include/time.h,v
> retrieving revision 1.19
> diff -u -r1.19 time.h
> --- libc/include/time.h	16 Oct 2008 21:53:58 -0000	1.19
> +++ libc/include/time.h	15 May 2011 19:22:48 -0000
> @@ -168,6 +168,9 @@
>  
>  /* High Resolution Sleep, P1003.1b-1993, p. 269 */
>  
> +int _EXFUN(clock_nanosleep,
> +  (clockid_t clock_id, int flags, const struct timespec *rqtp,
> +   struct timespec *rmtp));
>  int _EXFUN(nanosleep, (const struct timespec  *rqtp, struct timespec *rmtp));
>  
>  #ifdef __cplusplus

This doesn't look right.  In contrast to nanosleep, clock_nanosleep
is not subsumed under the _POSIX_TIMERS option.  In fact it's the only
function under the _POSIX_CLOCK_SELECTION option.  So clock_nanosleep
should be guarded independently of _POSIX_TIMERS, kind of like this:

 #if defined(_POSIX_CLOCK_SELECTION)
 extern "C" {
   int _EXFUN(clock_nanosleep, ...

Additionally _POSIX_CLOCK_SELECTION has to be activated in features.h.

Would you mind to send this patch to the newlib list then?

I haven't much time right now.  If cgf doesn't beat me to it, I'll
review the function later.


Thanks,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] clock_nanosleep(2)
  2011-07-20  7:57 ` Corinna Vinschen
@ 2011-07-20  9:16   ` Yaakov (Cygwin/X)
  2011-07-20  9:51     ` Yaakov (Cygwin/X)
  2011-07-20 17:37   ` [PATCH] clock_nanosleep(2) Christopher Faylor
  1 sibling, 1 reply; 28+ messages in thread
From: Yaakov (Cygwin/X) @ 2011-07-20  9:16 UTC (permalink / raw)
  To: cygwin-patches

On Wed, 2011-07-20 at 09:56 +0200, Corinna Vinschen wrote:
> This doesn't look right.  In contrast to nanosleep, clock_nanosleep
> is not subsumed under the _POSIX_TIMERS option.  In fact it's the only
> function under the _POSIX_CLOCK_SELECTION option.

I did some searching, and there are actually two more:

http://pubs.opengroup.org/onlinepubs/009695399/functions/pthread_condattr_getclock.html

The behaviour of the following functions are also affected by this
option:

http://pubs.opengroup.org/onlinepubs/009695399/functions/clock_getres.html
http://pubs.opengroup.org/onlinepubs/009695399/functions/pthread_cond_wait.html

(It should be noted that the Clock Selection option was merged into the
Base with POSIX.1-2008.)

How should we proceed now?


Yaakov


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] clock_nanosleep(2)
  2011-07-20  9:16   ` Yaakov (Cygwin/X)
@ 2011-07-20  9:51     ` Yaakov (Cygwin/X)
  2011-07-20 14:12       ` Corinna Vinschen
  0 siblings, 1 reply; 28+ messages in thread
From: Yaakov (Cygwin/X) @ 2011-07-20  9:51 UTC (permalink / raw)
  To: cygwin-patches

On Wed, 2011-07-20 at 04:16 -0500, Yaakov (Cygwin/X) wrote:
> On Wed, 2011-07-20 at 09:56 +0200, Corinna Vinschen wrote:
> > This doesn't look right.  In contrast to nanosleep, clock_nanosleep
> > is not subsumed under the _POSIX_TIMERS option.  In fact it's the only
> > function under the _POSIX_CLOCK_SELECTION option.
> 
> I did some searching, and there are actually two more:
> 
> http://pubs.opengroup.org/onlinepubs/009695399/functions/pthread_condattr_getclock.html
> 
> The behaviour of the following functions are also affected by this
> option:
> 
> http://pubs.opengroup.org/onlinepubs/009695399/functions/clock_getres.html
> http://pubs.opengroup.org/onlinepubs/009695399/functions/pthread_cond_wait.html
> 
> (It should be noted that the Clock Selection option was merged into the
> Base with POSIX.1-2008.)
> 
> How should we proceed now?

Actually, no need to panic, I took a closer look at this, and it's not
all that hard at all, so I'll go ahead and implement
pthread_condattr_[gs]etclock() as well.  Just give me a day or two to
get it done.  In the meantime, I'll proceed with the revised newlib
patch.


Yaakov


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] clock_nanosleep(2)
  2011-07-20  9:51     ` Yaakov (Cygwin/X)
@ 2011-07-20 14:12       ` Corinna Vinschen
  2011-07-20 22:04         ` [PATCH] clock_nanosleep(2), pthread_condattr_[gs]etclock(3) Yaakov (Cygwin/X)
  0 siblings, 1 reply; 28+ messages in thread
From: Corinna Vinschen @ 2011-07-20 14:12 UTC (permalink / raw)
  To: cygwin-patches

On Jul 20 04:50, Yaakov (Cygwin/X) wrote:
> On Wed, 2011-07-20 at 04:16 -0500, Yaakov (Cygwin/X) wrote:
> > On Wed, 2011-07-20 at 09:56 +0200, Corinna Vinschen wrote:
> > > This doesn't look right.  In contrast to nanosleep, clock_nanosleep
> > > is not subsumed under the _POSIX_TIMERS option.  In fact it's the only
> > > function under the _POSIX_CLOCK_SELECTION option.
> > 
> > I did some searching, and there are actually two more:
> > 
> > http://pubs.opengroup.org/onlinepubs/009695399/functions/pthread_condattr_getclock.html

As long as it's not implemented, I don't see a problem.

> > 
> > The behaviour of the following functions are also affected by this
> > option:
> > 
> > http://pubs.opengroup.org/onlinepubs/009695399/functions/clock_getres.html
> > http://pubs.opengroup.org/onlinepubs/009695399/functions/pthread_cond_wait.html
> > 
> > (It should be noted that the Clock Selection option was merged into the
> > Base with POSIX.1-2008.)
> > 
> > How should we proceed now?
> 
> Actually, no need to panic, I took a closer look at this, and it's not
> all that hard at all, so I'll go ahead and implement
> pthread_condattr_[gs]etclock() as well.  Just give me a day or two to
> get it done.  In the meantime, I'll proceed with the revised newlib
> patch.

Thanks.

The only problem I see is the fact that a call to clock_settime
influences calls to clock_nanosleep with absolute timeouts(*).

The problem is that we convert absolute timeouts to relative timeouts
and then use the timeout facility of the WFMO function to handle the
timeout for us.  IMO this is neither very reliable, nor is it elegant.

So, here's the question.  Shouldn't we better use waitable timers
to implement this sort of stuff?  Waitable timers are pretty easy to
use, they support relative and absolute timeouts with an accuracy of 100
ns in the API and a real accuracy which only depends on the underlying
HW, and they are especially not subject to the 49.7 days overflow
problem.


Corinna


(*) Does it also influence pthread_cond_timedwait?  This information seems
    to be missing in SUSv4.

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] clock_nanosleep(2)
  2011-07-20  7:57 ` Corinna Vinschen
  2011-07-20  9:16   ` Yaakov (Cygwin/X)
@ 2011-07-20 17:37   ` Christopher Faylor
  1 sibling, 0 replies; 28+ messages in thread
From: Christopher Faylor @ 2011-07-20 17:37 UTC (permalink / raw)
  To: cygwin-patches

On Wed, Jul 20, 2011 at 09:56:54AM +0200, Corinna Vinschen wrote:
>I haven't much time right now.  If cgf doesn't beat me to it, I'll
>review the function later.

I have a couple of PriA problems to look into at work so I don't have
much time either.

cgf

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] clock_nanosleep(2), pthread_condattr_[gs]etclock(3)
  2011-07-20 14:12       ` Corinna Vinschen
@ 2011-07-20 22:04         ` Yaakov (Cygwin/X)
  2011-07-21  2:22           ` Yaakov (Cygwin/X)
  2011-07-21  7:54           ` Corinna Vinschen
  0 siblings, 2 replies; 28+ messages in thread
From: Yaakov (Cygwin/X) @ 2011-07-20 22:04 UTC (permalink / raw)
  To: cygwin-patches

[-- Attachment #1: Type: text/plain, Size: 1934 bytes --]

On Wed, 2011-07-20 at 16:11 +0200, Corinna Vinschen wrote:
> On Jul 20 04:50, Yaakov (Cygwin/X) wrote:
> > Actually, no need to panic, I took a closer look at this, and it's not
> > all that hard at all, so I'll go ahead and implement
> > pthread_condattr_[gs]etclock() as well.  Just give me a day or two to
> > get it done.  In the meantime, I'll proceed with the revised newlib
> > patch.
> 
> Thanks.

Not taking the following issue into account, my patches to implement
pthread_condwait_[gs]etclock() and update sysconf() are attached.  (The
chunk for include/cygwin/version.h is not included, as that will depend
on which order these patches are applied.)

> The only problem I see is the fact that a call to clock_settime
> influences calls to clock_nanosleep with absolute timeouts(*).
> 
> The problem is that we convert absolute timeouts to relative timeouts
> and then use the timeout facility of the WFMO function to handle the
> timeout for us.  IMO this is neither very reliable, nor is it elegant.
> 
> So, here's the question.  Shouldn't we better use waitable timers
> to implement this sort of stuff?  Waitable timers are pretty easy to
> use, they support relative and absolute timeouts with an accuracy of 100
> ns in the API and a real accuracy which only depends on the underlying
> HW, and they are especially not subject to the 49.7 days overflow
> problem.

I see your point.  The question is how to use waitable timers for
CLOCK_MONOTONIC.

> (*) Does it also influence pthread_cond_timedwait?  This information seems
>     to be missing in SUSv4.

The last paragraph of RATIONALE -> Timed Wait Semantics states:

> For cases when the system clock is advanced discontinuously by an
> operator, it is expected that implementations process any timed wait
> expiring at an intervening time as if that time had actually occurred.

Of course, this would be an old problem with pthread_cond_timedwait().


Yaakov


[-- Attachment #2: cygwin-posix-clock-selection.patch --]
[-- Type: text/x-patch, Size: 896 bytes --]

2011-07-20  Yaakov Selkowitz  <yselkowitz@...>

	* sysconf.cc (sca): Set _SC_CLOCK_SELECTION to _POSIX_CLOCK_SELECTION.

Index: sysconf.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/sysconf.cc,v
retrieving revision 1.60
diff -u -p -r1.60 sysconf.cc
--- sysconf.cc	18 Jul 2011 23:08:09 -0000	1.60
+++ sysconf.cc	20 Jul 2011 21:33:56 -0000
@@ -158,7 +158,7 @@ static struct
   {cons, {c:BC_DIM_MAX}},		/*  58, _SC_BC_DIM_MAX */
   {cons, {c:BC_SCALE_MAX}},		/*  59, _SC_BC_SCALE_MAX */
   {cons, {c:BC_STRING_MAX}},		/*  60, _SC_BC_STRING_MAX */
-  {cons, {c:-1L}},			/*  61, _SC_CLOCK_SELECTION */
+  {cons, {c:_POSIX_CLOCK_SELECTION}},	/*  61, _SC_CLOCK_SELECTION */
   {nsup, {c:0}},			/*  62, _SC_COLL_WEIGHTS_MAX */
   {cons, {c:_POSIX_CPUTIME}},		/*  63, _SC_CPUTIME */
   {cons, {c:EXPR_NEST_MAX}},		/*  64, _SC_EXPR_NEST_MAX */

[-- Attachment #3: cygwin-pthread_condattr_getclock.patch --]
[-- Type: text/x-patch, Size: 7702 bytes --]

2011-07-20  Yaakov Selkowitz  <yselkowitz@...>

	* cygwin.din (pthread_condattr_getclock): Export.
	(pthread_condattr_setclock): Export.
	* posix.sgml (std-notimpl): Move pthread_condattr_getclock and
	pthread_condattr_setclock from here...
	(std-susv4): ... to here.
	* thread.cc: (pthread_condattr::pthread_condattr): Initialize clock_id.
	(pthread_cond::pthread_cond): Initialize clock_id.
	(pthread_cond_timedwait): Use clock_gettime() instead of gettimeofday()
	in order to support all allowed clocks.
	(pthread_condattr_getclock): New function.
	(pthread_condattr_setclock): New function.
	* thread.h (class pthread_condattr): Add clock_id member.
	(class pthread_cond): Ditto.
	* include/pthread.h: Remove obsolete comment.
	(pthread_condattr_getclock): Declare.
	(pthread_condattr_setclock): Declare.

Index: cygwin.din
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/cygwin.din,v
retrieving revision 1.244
diff -u -p -r1.244 cygwin.din
--- cygwin.din	19 May 2011 07:23:28 -0000	1.244
+++ cygwin.din	20 Jul 2011 21:33:53 -0000
@@ -1209,8 +1210,10 @@ pthread_cond_signal SIGFE
 pthread_cond_timedwait SIGFE
 pthread_cond_wait SIGFE
 pthread_condattr_destroy SIGFE
+pthread_condattr_getclock SIGFE
 pthread_condattr_getpshared SIGFE
 pthread_condattr_init SIGFE
+pthread_condattr_setclock SIGFE
 pthread_condattr_setpshared SIGFE
 pthread_continue SIGFE
 pthread_create SIGFE
Index: posix.sgml
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/posix.sgml,v
retrieving revision 1.68
diff -u -p -r1.68 posix.sgml
--- posix.sgml	25 May 2011 06:10:24 -0000	1.68
+++ posix.sgml	20 Jul 2011 21:33:56 -0000
@@ -554,8 +555,10 @@ also IEEE Std 1003.1-2008 (POSIX.1-2008)
     pthread_cond_timedwait
     pthread_cond_wait
     pthread_condattr_destroy
+    pthread_condattr_getclock
     pthread_condattr_getpshared
     pthread_condattr_init
+    pthread_condattr_setclock
     pthread_condattr_setpshared
     pthread_create
     pthread_detach
@@ -1390,8 +1392,6 @@ also IEEE Std 1003.1-2008 (POSIX.1-2008)
     posix_typed_[...]
     powl
     pthread_barrier[...]
-    pthread_condattr_getclock
-    pthread_condattr_setclock
     pthread_mutexattr_getrobust
     pthread_mutexattr_setrobust
     pthread_mutex_consistent
Index: thread.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/thread.cc,v
retrieving revision 1.243
diff -u -p -r1.243 thread.cc
--- thread.cc	6 Jun 2011 05:02:13 -0000	1.243
+++ thread.cc	20 Jul 2011 21:33:56 -0000
@@ -1099,7 +1099,8 @@ pthread_attr::~pthread_attr ()
 }
 
 pthread_condattr::pthread_condattr ():verifyable_object
-  (PTHREAD_CONDATTR_MAGIC), shared (PTHREAD_PROCESS_PRIVATE)
+  (PTHREAD_CONDATTR_MAGIC), shared (PTHREAD_PROCESS_PRIVATE),
+  clock_id (CLOCK_REALTIME)
 {
 }
 
@@ -1124,17 +1125,21 @@ pthread_cond::init_mutex ()
 
 pthread_cond::pthread_cond (pthread_condattr *attr) :
   verifyable_object (PTHREAD_COND_MAGIC),
-  shared (0), waiting (0), pending (0), sem_wait (NULL),
-  mtx_cond(NULL), next (NULL)
+  shared (0), clock_id (CLOCK_REALTIME), waiting (0), pending (0),
+  sem_wait (NULL), mtx_cond(NULL), next (NULL)
 {
   pthread_mutex *verifyable_mutex_obj;
 
   if (attr)
-    if (attr->shared != PTHREAD_PROCESS_PRIVATE)
-      {
-	magic = 0;
-	return;
-      }
+    {
+      clock_id = attr->clock_id;
+
+      if (attr->shared != PTHREAD_PROCESS_PRIVATE)
+	{
+	  magic = 0;
+	  return;
+	}
+    }
 
   verifyable_mutex_obj = &mtx_in;
   if (!pthread_mutex::is_good_object (&verifyable_mutex_obj))
@@ -2716,7 +2721,7 @@ extern "C" int
 pthread_cond_timedwait (pthread_cond_t *cond, pthread_mutex_t *mutex,
 			const struct timespec *abstime)
 {
-  struct timeval tv;
+  struct timespec tp;
   DWORD waitlength;
 
   myfault efault;
@@ -2731,17 +2736,18 @@ pthread_cond_timedwait (pthread_cond_t *
       || abstime->tv_nsec > 999999999)
     return EINVAL;
 
-  gettimeofday (&tv, NULL);
+  clock_gettime ((*cond)->clock_id, &tp);
+
   /* Check for immediate timeout before converting to microseconds, since
      the resulting value can easily overflow long.  This also allows to
      evaluate microseconds directly in DWORD. */
-  if (tv.tv_sec > abstime->tv_sec
-      || (tv.tv_sec == abstime->tv_sec
-	  && tv.tv_usec > abstime->tv_nsec / 1000))
+  if (tp.tv_sec > abstime->tv_sec
+      || (tp.tv_sec == abstime->tv_sec
+	  && tp.tv_nsec > abstime->tv_nsec))
     return ETIMEDOUT;
 
-  waitlength = (abstime->tv_sec - tv.tv_sec) * 1000;
-  waitlength += (abstime->tv_nsec / 1000 - tv.tv_usec) / 1000;
+  waitlength = (abstime->tv_sec - tp.tv_sec) * 1000;
+  waitlength += (abstime->tv_nsec - tp.tv_nsec) / 1000000;
   return __pthread_cond_dowait (cond, mutex, waitlength);
 }
 
@@ -2793,6 +2799,32 @@ pthread_condattr_setpshared (pthread_con
 }
 
 extern "C" int
+pthread_condattr_getclock (const pthread_condattr_t *attr, clockid_t *clock_id)
+{
+  if (!pthread_condattr::is_good_object (attr))
+    return EINVAL;
+  *clock_id = (*attr)->clock_id;
+  return 0;
+}
+
+extern "C" int
+pthread_condattr_setclock (pthread_condattr_t *attr, clockid_t clock_id)
+{
+  if (!pthread_condattr::is_good_object (attr))
+    return EINVAL;
+  switch (clock_id)
+    {
+    case CLOCK_REALTIME:
+    case CLOCK_MONOTONIC:
+      break;
+    default:
+      return EINVAL;
+    }
+  (*attr)->clock_id = clock_id;
+  return 0;
+}
+
+extern "C" int
 pthread_condattr_destroy (pthread_condattr_t *condattr)
 {
   if (!pthread_condattr::is_good_object (condattr))
Index: thread.h
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/thread.h,v
retrieving revision 1.121
diff -u -p -r1.121 thread.h
--- thread.h	15 May 2011 18:49:40 -0000	1.121
+++ thread.h	20 Jul 2011 21:33:56 -0000
@@ -488,6 +488,7 @@ class pthread_condattr: public verifyabl
 public:
   static bool is_good_object(pthread_condattr_t const *);
   int shared;
+  clockid_t clock_id;
 
   pthread_condattr ();
   ~pthread_condattr ();
@@ -504,6 +505,7 @@ public:
   static int init (pthread_cond_t *, const pthread_condattr_t *);
 
   int shared;
+  clockid_t clock_id;
 
   unsigned long waiting;
   unsigned long pending;
Index: include/pthread.h
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/include/pthread.h,v
retrieving revision 1.33
diff -u -p -r1.33 pthread.h
--- include/pthread.h	17 May 2011 17:08:10 -0000	1.33
+++ include/pthread.h	20 Jul 2011 21:33:56 -0000
@@ -27,12 +27,6 @@ extern "C"
 /* Defines. (These are correctly defined here as per
    http://www.opengroup.org/onlinepubs/7908799/xsh/pthread.h.html */
 
-/* FIXME: this should allocate a new cond variable, and return the value  that
- would normally be written to the passed parameter of pthread_cond_init(lvalue, NULL); */
-/* #define PTHREAD_COND_INITIALIZER 0 */
-
-/* the default : joinable */
-
 #define PTHREAD_CANCEL_ASYNCHRONOUS 1
 /* defaults are enable, deferred */
 #define PTHREAD_CANCEL_ENABLE 0
@@ -132,8 +126,10 @@ int pthread_cond_timedwait (pthread_cond
 			    pthread_mutex_t *, const struct timespec *);
 int pthread_cond_wait (pthread_cond_t *, pthread_mutex_t *);
 int pthread_condattr_destroy (pthread_condattr_t *);
+int pthread_condattr_getclock (const pthread_condattr_t *, clockid_t *);
 int pthread_condattr_getpshared (const pthread_condattr_t *, int *);
 int pthread_condattr_init (pthread_condattr_t *);
+int pthread_condattr_setclock (pthread_condattr_t *, clockid_t);
 int pthread_condattr_setpshared (pthread_condattr_t *, int);
 
 int pthread_create (pthread_t *, const pthread_attr_t *,

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] clock_nanosleep(2), pthread_condattr_[gs]etclock(3)
  2011-07-20 22:04         ` [PATCH] clock_nanosleep(2), pthread_condattr_[gs]etclock(3) Yaakov (Cygwin/X)
@ 2011-07-21  2:22           ` Yaakov (Cygwin/X)
  2011-07-21  9:21             ` Corinna Vinschen
  2011-07-21  7:54           ` Corinna Vinschen
  1 sibling, 1 reply; 28+ messages in thread
From: Yaakov (Cygwin/X) @ 2011-07-21  2:22 UTC (permalink / raw)
  To: cygwin-patches

On Wed, 2011-07-20 at 17:03 -0500, Yaakov (Cygwin/X) wrote:
> On Wed, 2011-07-20 at 16:11 +0200, Corinna Vinschen wrote:
> > The only problem I see is the fact that a call to clock_settime
> > influences calls to clock_nanosleep with absolute timeouts(*).

However, clock_settime() can set only CLOCK_REALTIME, not
CLOCK_MONOTONIC, so...

> > The problem is that we convert absolute timeouts to relative timeouts
> > and then use the timeout facility of the WFMO function to handle the
> > timeout for us.  IMO this is neither very reliable, nor is it elegant.
> > 
> > So, here's the question.  Shouldn't we better use waitable timers
> > to implement this sort of stuff?  Waitable timers are pretty easy to
> > use, they support relative and absolute timeouts with an accuracy of 100
> > ns in the API and a real accuracy which only depends on the underlying
> > HW, and they are especially not subject to the 49.7 days overflow
> > problem.
> 
> I see your point.  The question is how to use waitable timers for
> CLOCK_MONOTONIC.

...therefore we could still handle CLOCK_MONOTONIC timedwait as a
relative timeout.  So pthread_condattr_[gs]etclock should be correct
even without this (although it would still gain accuracy), but that does
leave a problem with clock_nanosleep(TIMER_ABSTIME).

Looking at the other uses of cancelable_wait(), would the following make
sense:

* change the timeout argument to struct timespec *;
* cancelable_wait (object, INFINITE) calls change to (object, NULL);
* cancelable_wait (object, DWORD) calls change to (object, &timespec);
* then in cancelable_wait:

HANDLE hTimer;
HANDLE wait_objects[4];
....
wait_objects[num++] = object;

if (timeout)
  {
    LARGE_INTEGER li;
    li.QuadPart = (timeout->tv_sec * NSPERSEC) + (timeout->tv_nsec /
100); /* rounding? */
    hTimer = CreateWaitableTimer (NULL, FALSE, NULL);
    SetWaitableTimer (hTimer, &li, 0, NULL, NULL, FALSE); /* handle
possible error?  what would cause one? */
    wait_objects[num++] = hTimer;
  }
...
while (1)
  {
    res = WaitForMultipleObjects (num, wait_objects, FALSE, INFINITE);
....

Or am I completely off-base here?


Yaakov


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] clock_nanosleep(2), pthread_condattr_[gs]etclock(3)
  2011-07-20 22:04         ` [PATCH] clock_nanosleep(2), pthread_condattr_[gs]etclock(3) Yaakov (Cygwin/X)
  2011-07-21  2:22           ` Yaakov (Cygwin/X)
@ 2011-07-21  7:54           ` Corinna Vinschen
  1 sibling, 0 replies; 28+ messages in thread
From: Corinna Vinschen @ 2011-07-21  7:54 UTC (permalink / raw)
  To: cygwin-patches

On Jul 20 17:03, Yaakov (Cygwin/X) wrote:
> On Wed, 2011-07-20 at 16:11 +0200, Corinna Vinschen wrote:
> > (*) Does it also influence pthread_cond_timedwait?  This information seems
> >     to be missing in SUSv4.
> 
> The last paragraph of RATIONALE -> Timed Wait Semantics states:
> 
> > For cases when the system clock is advanced discontinuously by an
> > operator, it is expected that implementations process any timed wait
> > expiring at an intervening time as if that time had actually occurred.
> 
> Of course, this would be an old problem with pthread_cond_timedwait().

Thanks, I missed that.

> 2011-07-20  Yaakov Selkowitz  <yselkowitz@...>
> 
> 	* sysconf.cc (sca): Set _SC_CLOCK_SELECTION to _POSIX_CLOCK_SELECTION.
> 
> 2011-07-20  Yaakov Selkowitz  <yselkowitz@...>
> 
> 	* cygwin.din (pthread_condattr_getclock): Export.
> 	(pthread_condattr_setclock): Export.
> 	* posix.sgml (std-notimpl): Move pthread_condattr_getclock and
> 	pthread_condattr_setclock from here...
> 	(std-susv4): ... to here.
> 	* thread.cc: (pthread_condattr::pthread_condattr): Initialize clock_id.
> 	(pthread_cond::pthread_cond): Initialize clock_id.
> 	(pthread_cond_timedwait): Use clock_gettime() instead of gettimeofday()
> 	in order to support all allowed clocks.
> 	(pthread_condattr_getclock): New function.
> 	(pthread_condattr_setclock): New function.
> 	* thread.h (class pthread_condattr): Add clock_id member.
> 	(class pthread_cond): Ditto.
> 	* include/pthread.h: Remove obsolete comment.
> 	(pthread_condattr_getclock): Declare.
> 	(pthread_condattr_setclock): Declare.

This patch looks good, please apply.


Thanks,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] clock_nanosleep(2), pthread_condattr_[gs]etclock(3)
  2011-07-21  2:22           ` Yaakov (Cygwin/X)
@ 2011-07-21  9:21             ` Corinna Vinschen
  2011-07-21  9:36               ` Corinna Vinschen
  0 siblings, 1 reply; 28+ messages in thread
From: Corinna Vinschen @ 2011-07-21  9:21 UTC (permalink / raw)
  To: cygwin-patches

On Jul 20 21:22, Yaakov (Cygwin/X) wrote:
> On Wed, 2011-07-20 at 17:03 -0500, Yaakov (Cygwin/X) wrote:
> > On Wed, 2011-07-20 at 16:11 +0200, Corinna Vinschen wrote:
> > > The only problem I see is the fact that a call to clock_settime
> > > influences calls to clock_nanosleep with absolute timeouts(*).
> 
> However, clock_settime() can set only CLOCK_REALTIME, not
> CLOCK_MONOTONIC, so...
> [...]
> ...therefore we could still handle CLOCK_MONOTONIC timedwait as a
> relative timeout.  So pthread_condattr_[gs]etclock should be correct
> even without this (although it would still gain accuracy), but that does
> leave a problem with clock_nanosleep(TIMER_ABSTIME).
> 
> Looking at the other uses of cancelable_wait(), would the following make
> sense:
> 
> * change the timeout argument to struct timespec *;
> * cancelable_wait (object, INFINITE) calls change to (object, NULL);
> * cancelable_wait (object, DWORD) calls change to (object, &timespec);
> * then in cancelable_wait:
> 
> HANDLE hTimer;
> HANDLE wait_objects[4];
> ....
> wait_objects[num++] = object;
> 
> if (timeout)
>   {
>     LARGE_INTEGER li;
>     li.QuadPart = (timeout->tv_sec * NSPERSEC) + (timeout->tv_nsec /
> 100); /* rounding? */
>     hTimer = CreateWaitableTimer (NULL, FALSE, NULL);
>     SetWaitableTimer (hTimer, &li, 0, NULL, NULL, FALSE); /* handle
> possible error?  what would cause one? */
>     wait_objects[num++] = hTimer;
>   }
> ...
> while (1)
>   {
>     res = WaitForMultipleObjects (num, wait_objects, FALSE, INFINITE);
> ....
> 
> Or am I completely off-base here?

No, you're not at all off-base.  Personally I'd prefer to use the native
NT timer functions, but that's not important.  What I'm missing is a way
to specify relative vs. absolute timeouts in your above sketch.  I guess
we need a flag argument as well.

Other than that, I think we should make sure to create the waitable
timer only once on a per-thread base.  Object creation and deletion is
usually a time consuming process.  So what we could do is to add a
HANDLE "cw_timer" to struct _local_storage in cygtls.h, which gets
inited to NULL in _cygtls::init_thread as well as in
_cygtls::fixup_after_fork.

Then cancelable_wait with a non-NULL timespec would check for the handle
being NULL and create a non-inheritable timer, if so.  All subsequent
calls only set (and cancel) the timer.

Does that sound reasonable?


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] clock_nanosleep(2), pthread_condattr_[gs]etclock(3)
  2011-07-21  9:21             ` Corinna Vinschen
@ 2011-07-21  9:36               ` Corinna Vinschen
  2011-07-21 18:59                 ` Yaakov (Cygwin/X)
  0 siblings, 1 reply; 28+ messages in thread
From: Corinna Vinschen @ 2011-07-21  9:36 UTC (permalink / raw)
  To: cygwin-patches

[-- Attachment #1: Type: text/plain, Size: 2387 bytes --]

On Jul 21 11:21, Corinna Vinschen wrote:
> On Jul 20 21:22, Yaakov (Cygwin/X) wrote:
> > Looking at the other uses of cancelable_wait(), would the following make
> > sense:
> > 
> > * change the timeout argument to struct timespec *;
> > * cancelable_wait (object, INFINITE) calls change to (object, NULL);
> > * cancelable_wait (object, DWORD) calls change to (object, &timespec);
> > * then in cancelable_wait:
> > 
> > HANDLE hTimer;
> > HANDLE wait_objects[4];
> > ....
> > wait_objects[num++] = object;
> > 
> > if (timeout)
> >   {
> >     LARGE_INTEGER li;
> >     li.QuadPart = (timeout->tv_sec * NSPERSEC) + (timeout->tv_nsec /
> > 100); /* rounding? */
> >     hTimer = CreateWaitableTimer (NULL, FALSE, NULL);
> >     SetWaitableTimer (hTimer, &li, 0, NULL, NULL, FALSE); /* handle
> > possible error?  what would cause one? */
> >     wait_objects[num++] = hTimer;
> >   }
> > ...
> > while (1)
> >   {
> >     res = WaitForMultipleObjects (num, wait_objects, FALSE, INFINITE);
> > ....
> > 
> > Or am I completely off-base here?
> 
> No, you're not at all off-base.  Personally I'd prefer to use the native
> NT timer functions, but that's not important.  What I'm missing is a way
> to specify relative vs. absolute timeouts in your above sketch.  I guess
> we need a flag argument as well.
> 
> Other than that, I think we should make sure to create the waitable
> timer only once on a per-thread base.  Object creation and deletion is
> usually a time consuming process.  So what we could do is to add a
> HANDLE "cw_timer" to struct _local_storage in cygtls.h, which gets
> inited to NULL in _cygtls::init_thread as well as in
> _cygtls::fixup_after_fork.
> 
> Then cancelable_wait with a non-NULL timespec would check for the handle
> being NULL and create a non-inheritable timer, if so.  All subsequent
> calls only set (and cancel) the timer.
> 
> Does that sound reasonable?

Btw., if you call NtQueryTimer right before NtCancelTimer, then you get
the remaining time for free to return to clock_nanosleep.  It would
be nice if NtQueryTimer would return the remaining time after calling
NtCancelTimer, but my experiments show that some weird value gets
returned.  See my attached testcase.  Build with -lntdll.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: tick.c --]
[-- Type: text/plain, Size: 1514 bytes --]

#include <stdio.h>
#include <stdlib.h>
#include <windows.h>
#include <ddk/ntddk.h>

int
main (int argc, char* argv[])
{
  NTSTATUS status;
  HANDLE timer;
  LARGE_INTEGER duetime;
  PTIMER_BASIC_INFORMATION tbi;
  int cnt = 10, i;
  long long lastTimer = 0;

  duetime.QuadPart = -10000000000LL;
  tbi = (PTIMER_BASIC_INFORMATION)
	calloc (cnt + 1, sizeof (TIMER_BASIC_INFORMATION));
  if (!tbi)
    {
      printf ("malloc failed\n:");
      return 1;
    }
  status = NtCreateTimer (&timer, TIMER_ALL_ACCESS, NULL, NotificationTimer);
  if (!NT_SUCCESS (status))
    {
      printf ("NtCreateTimer: %p\n", status);
      return 1;
    }
  status = NtSetTimer (timer, &duetime, NULL, NULL, FALSE, 1000000L, NULL);
  if (!NT_SUCCESS (status))
    {
      printf ("NtSetTimer: %p\n", status);
      return 1;
    }
  for (i = 0; i < cnt; ++i)
    {
      do
	{
	  NtQueryTimer (timer, TimerBasicInformation, tbi + i, sizeof *tbi,
			NULL);
	  //Sleep(1);
	}
      while (tbi[i].TimeRemaining.QuadPart
	     == tbi[i - 1].TimeRemaining.QuadPart);
    }
  NtCancelTimer (timer, NULL);
  status = NtQueryTimer (timer, TimerBasicInformation, tbi + i, sizeof *tbi,
			 NULL);
  if (!NT_SUCCESS (status))
    printf ("NtQueryTimer: %p\n", status);
  NtClose (timer);
  for (i = 1; i < cnt + 1; ++i)
    printf("Timer: %lld (%llx), dTimer: %lld usec\n",
	   tbi[i].TimeRemaining.QuadPart,
	   tbi[i].TimeRemaining.QuadPart,
	   (tbi[i - 1].TimeRemaining.QuadPart
	    - tbi[i].TimeRemaining.QuadPart) / 10);
  return 0;
}

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] clock_nanosleep(2)
  2011-07-20  1:54 [PATCH] clock_nanosleep(2) Yaakov (Cygwin/X)
  2011-07-20  7:57 ` Corinna Vinschen
@ 2011-07-21 10:39 ` Corinna Vinschen
  2011-07-21 18:51   ` Yaakov (Cygwin/X)
  1 sibling, 1 reply; 28+ messages in thread
From: Corinna Vinschen @ 2011-07-21 10:39 UTC (permalink / raw)
  To: cygwin-patches

On Jul 19 20:54, Yaakov (Cygwin/X) wrote:
> This patchset implements the POSIX clock_nanosleep(2) function:
> 
> http://pubs.opengroup.org/onlinepubs/9699919799/functions/clock_nanosleep.html
> http://www.kernel.org/doc/man-pages/online/pages/man2/clock_nanosleep.2.html
> 
> In summary, clock_nanosleep(2) replaces nanosleep(2) as the primary
> sleeping function, with all others rewritten in terms of the former.  It
> also restores maximum precision to hires_ms::resolution(), saving the
> <5000 100ns check for the one place where resolution is rounded off.
> 
> Patches for newlib, winsup/cygwin, and winsup/doc attached.  I would
> appreciate a careful look at this one.

Given our current discussion to change cancelable_wait, does it make
sense to review this patch?  AFAICs the clock_nanosleep function will
have to be changes quite a bit, right?

Something else occured to me, but I think we should do this in an extra
step, if at all.  IMO the family of sleep functions should be moved out
of signal.cc into times.cc.  It just seems to belong there.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] clock_nanosleep(2)
  2011-07-21 10:39 ` Corinna Vinschen
@ 2011-07-21 18:51   ` Yaakov (Cygwin/X)
  2011-07-21 19:04     ` Corinna Vinschen
  2011-07-31  8:25     ` Corinna Vinschen
  0 siblings, 2 replies; 28+ messages in thread
From: Yaakov (Cygwin/X) @ 2011-07-21 18:51 UTC (permalink / raw)
  To: cygwin-patches

On Thu, 2011-07-21 at 12:37 +0200, Corinna Vinschen wrote:
> Given our current discussion to change cancelable_wait, does it make
> sense to review this patch?  

No, the cancelable_wait changes need to go first.

> AFAICs the clock_nanosleep function will have to be changes quite a bit, right?

Definitely.

> Something else occured to me, but I think we should do this in an extra
> step, if at all.  IMO the family of sleep functions should be moved out
> of signal.cc into times.cc.  It just seems to belong there.

I'm not sure what the gain would be.


Yaakov


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] clock_nanosleep(2), pthread_condattr_[gs]etclock(3)
  2011-07-21  9:36               ` Corinna Vinschen
@ 2011-07-21 18:59                 ` Yaakov (Cygwin/X)
  2011-07-21 19:09                   ` Corinna Vinschen
  0 siblings, 1 reply; 28+ messages in thread
From: Yaakov (Cygwin/X) @ 2011-07-21 18:59 UTC (permalink / raw)
  To: cygwin-patches

On Thu, 2011-07-21 at 11:35 +0200, Corinna Vinschen wrote:
> On Jul 21 11:21, Corinna Vinschen wrote:
> > No, you're not at all off-base.  Personally I'd prefer to use the native
> > NT timer functions, but that's not important.

No problem, that's something I keep forgetting about.

> > What I'm missing is a way to specify relative vs. absolute timeouts in
> > your above sketch.  I guess we need a flag argument as well.

Working on this last night, I decided to make the timeout a LONGLONG of
100ns units instead, positive for absolute and negative for relative.

> > Other than that, I think we should make sure to create the waitable
> > timer only once on a per-thread base.  Object creation and deletion is
> > usually a time consuming process.  So what we could do is to add a
> > HANDLE "cw_timer" to struct _local_storage in cygtls.h, which gets
> > inited to NULL in _cygtls::init_thread as well as in
> > _cygtls::fixup_after_fork.
> > 
> > Then cancelable_wait with a non-NULL timespec would check for the handle
> > being NULL and create a non-inheritable timer, if so.  All subsequent
> > calls only set (and cancel) the timer.
> > 
> > Does that sound reasonable?
> 
> Btw., if you call NtQueryTimer right before NtCancelTimer, then you get
> the remaining time for free to return to clock_nanosleep.  It would
> be nice if NtQueryTimer would return the remaining time after calling
> NtCancelTimer, but my experiments show that some weird value gets
> returned.  See my attached testcase.  Build with -lntdll.

Thanks, that was the piece I was missing last night.


Yaakov


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] clock_nanosleep(2)
  2011-07-21 18:51   ` Yaakov (Cygwin/X)
@ 2011-07-21 19:04     ` Corinna Vinschen
  2011-07-31  8:25     ` Corinna Vinschen
  1 sibling, 0 replies; 28+ messages in thread
From: Corinna Vinschen @ 2011-07-21 19:04 UTC (permalink / raw)
  To: cygwin-patches

On Jul 21 13:51, Yaakov (Cygwin/X) wrote:
> On Thu, 2011-07-21 at 12:37 +0200, Corinna Vinschen wrote:
> > Something else occured to me, but I think we should do this in an extra
> > step, if at all.  IMO the family of sleep functions should be moved out
> > of signal.cc into times.cc.  It just seems to belong there.
> 
> I'm not sure what the gain would be.

Only a bit of code cleanup.  The other clock functions are in times.cc
as well.  But, never mind, just ignore these musings.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] clock_nanosleep(2), pthread_condattr_[gs]etclock(3)
  2011-07-21 18:59                 ` Yaakov (Cygwin/X)
@ 2011-07-21 19:09                   ` Corinna Vinschen
  2011-07-21 19:15                     ` Corinna Vinschen
  0 siblings, 1 reply; 28+ messages in thread
From: Corinna Vinschen @ 2011-07-21 19:09 UTC (permalink / raw)
  To: cygwin-patches

On Jul 21 13:59, Yaakov (Cygwin/X) wrote:
> On Thu, 2011-07-21 at 11:35 +0200, Corinna Vinschen wrote:
> > On Jul 21 11:21, Corinna Vinschen wrote:
> > > No, you're not at all off-base.  Personally I'd prefer to use the native
> > > NT timer functions, but that's not important.
> 
> No problem, that's something I keep forgetting about.
> 
> > > What I'm missing is a way to specify relative vs. absolute timeouts in
> > > your above sketch.  I guess we need a flag argument as well.
> 
> Working on this last night, I decided to make the timeout a LONGLONG of
> 100ns units instead, positive for absolute and negative for relative.

Good idea.  The value can be immediately used in NtSetTimer and it
can be used for testing.

> > Btw., if you call NtQueryTimer right before NtCancelTimer, then you get
> > the remaining time for free to return to clock_nanosleep.  It would
> > be nice if NtQueryTimer would return the remaining time after calling
> > NtCancelTimer, but my experiments show that some weird value gets
> > returned.  See my attached testcase.  Build with -lntdll.
> 
> Thanks, that was the piece I was missing last night.

Just ignore the bug in the while expression, please...


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] clock_nanosleep(2), pthread_condattr_[gs]etclock(3)
  2011-07-21 19:09                   ` Corinna Vinschen
@ 2011-07-21 19:15                     ` Corinna Vinschen
  2011-07-22  6:34                       ` Yaakov (Cygwin/X)
  0 siblings, 1 reply; 28+ messages in thread
From: Corinna Vinschen @ 2011-07-21 19:15 UTC (permalink / raw)
  To: cygwin-patches

On Jul 21 21:09, Corinna Vinschen wrote:
> On Jul 21 13:59, Yaakov (Cygwin/X) wrote:
> > On Thu, 2011-07-21 at 11:35 +0200, Corinna Vinschen wrote:
> > > On Jul 21 11:21, Corinna Vinschen wrote:
> > > > No, you're not at all off-base.  Personally I'd prefer to use the native
> > > > NT timer functions, but that's not important.
> > 
> > No problem, that's something I keep forgetting about.
> > 
> > > > What I'm missing is a way to specify relative vs. absolute timeouts in
> > > > your above sketch.  I guess we need a flag argument as well.
> > 
> > Working on this last night, I decided to make the timeout a LONGLONG of
> > 100ns units instead, positive for absolute and negative for relative.
> 
> Good idea.  The value can be immediately used in NtSetTimer and it
> can be used for testing.

Erm... maybe PLARGE_INTEGER would be the right type for this?


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] clock_nanosleep(2), pthread_condattr_[gs]etclock(3)
  2011-07-21 19:15                     ` Corinna Vinschen
@ 2011-07-22  6:34                       ` Yaakov (Cygwin/X)
  2011-07-22  8:05                         ` Corinna Vinschen
  0 siblings, 1 reply; 28+ messages in thread
From: Yaakov (Cygwin/X) @ 2011-07-22  6:34 UTC (permalink / raw)
  To: cygwin-patches

On Thu, 2011-07-21 at 21:15 +0200, Corinna Vinschen wrote:
> On Jul 21 21:09, Corinna Vinschen wrote:
> > On Jul 21 13:59, Yaakov (Cygwin/X) wrote:
> > Good idea.  The value can be immediately used in NtSetTimer and it
> > can be used for testing.
> 
> Erm... maybe PLARGE_INTEGER would be the right type for this?

You're right, that would also allow it to be used as an in/out variable
to get the remaining time back in nanosleep().

I'm most of the way there now, but I've got a busy weekend ahead, so I
probably won't be finished with this until at least Monday.


Yaakov


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] clock_nanosleep(2), pthread_condattr_[gs]etclock(3)
  2011-07-22  6:34                       ` Yaakov (Cygwin/X)
@ 2011-07-22  8:05                         ` Corinna Vinschen
  0 siblings, 0 replies; 28+ messages in thread
From: Corinna Vinschen @ 2011-07-22  8:05 UTC (permalink / raw)
  To: cygwin-patches

On Jul 22 01:34, Yaakov (Cygwin/X) wrote:
> On Thu, 2011-07-21 at 21:15 +0200, Corinna Vinschen wrote:
> > On Jul 21 21:09, Corinna Vinschen wrote:
> > > On Jul 21 13:59, Yaakov (Cygwin/X) wrote:
> > > Good idea.  The value can be immediately used in NtSetTimer and it
> > > can be used for testing.
> > 
> > Erm... maybe PLARGE_INTEGER would be the right type for this?
> 
> You're right, that would also allow it to be used as an in/out variable
> to get the remaining time back in nanosleep().
> 
> I'm most of the way there now, but I've got a busy weekend ahead, so I
> probably won't be finished with this until at least Monday.

No worries.  This is worth waiting for :)


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] clock_nanosleep(2)
  2011-07-21 18:51   ` Yaakov (Cygwin/X)
  2011-07-21 19:04     ` Corinna Vinschen
@ 2011-07-31  8:25     ` Corinna Vinschen
  2011-08-02  4:09       ` Yaakov (Cygwin/X)
  1 sibling, 1 reply; 28+ messages in thread
From: Corinna Vinschen @ 2011-07-31  8:25 UTC (permalink / raw)
  To: cygwin-patches

Hi Yaakov,

anything new from the clock_nanosleep frontier?

On Jul 21 13:51, Yaakov (Cygwin/X) wrote:
> On Thu, 2011-07-21 at 12:37 +0200, Corinna Vinschen wrote:
> > Given our current discussion to change cancelable_wait, does it make
> > sense to review this patch?  
> 
> No, the cancelable_wait changes need to go first.
> 
> > AFAICs the clock_nanosleep function will have to be changes quite a bit, right?
> 
> Definitely.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] clock_nanosleep(2)
  2011-07-31  8:25     ` Corinna Vinschen
@ 2011-08-02  4:09       ` Yaakov (Cygwin/X)
  2011-08-02 15:43         ` Corinna Vinschen
  0 siblings, 1 reply; 28+ messages in thread
From: Yaakov (Cygwin/X) @ 2011-08-02  4:09 UTC (permalink / raw)
  To: cygwin-patches

[-- Attachment #1: Type: text/plain, Size: 666 bytes --]

On Sun, 2011-07-31 at 10:24 +0200, Corinna Vinschen wrote:
> anything new from the clock_nanosleep frontier?

Sorry, I've been having elusive problems with CVS HEAD that have been
making it hard to test my patch.

Here's what I have so far, FWIW.  So far I've found two problems with
it: the remaining time returned is incorrect, based on testing of
nanosleep(), and the pthread_spin chunk doesn't look right (previously
the timeout would repeat in the while loop, but that won't happen the
way the waitable timer is set up).

I'll try to get back to this as soon as I am able to test this properly.
In the meantime, is there anything obvious I'm missing?


Yaakov


[-- Attachment #2: cygwin-waitable-timer.patch --]
[-- Type: text/x-patch, Size: 22070 bytes --]

FIRST DRAFT!  DO NOT COMMIT!!!

	* cygtls.h (struct _local_storage): Add cw_timer member.
	* cygtls.cc (_cygtls::init_thread): Initialize locals.cw_timer.
	(_cygtls::fixup_after_fork): Ditto.
	* tlsoffsets.h: Regenerate.
	* ntdll.h (enum _TIMER_INFORMATION_CLASS): Define.
	(struct _TIMER_BASIC_INFORMATION): Define.
	(NtQueryTimer): Declare function.
	* thread.h (cancelable_wait): Change timeout argument to
	PLARGE_INTEGER and provide NULL default.
	(fast_mutex::lock): Adjust accordingly.
	(pthread_cond::wait): Change timeout argument to PLARGE_INTEGER
	and default to NULL.
	* thread.cc (cancelable_wait): Change timeout argument to
	PLARGE_INTEGER.  Initialize _cygtls.locals.cw_timer if needed.
	Use NT waitable timers for handling timeout.  Return remaining time
	to timeout argument if timeout was relative.
	(pthread_cond::wait): Change timeout argument to PLARGE_INTEGER.
	Adjust to change in cancelable_wait.
	(pthread_mutex::lock): Adjust to change in cancelable_wait.
	(pthread_spinlock::lock): Ditto.
	(pthread::join): Ditto.
	(__pthread_cond_dowait): Change waitlength argument to PLARGE_INTEGER.
	Adjust to changes in cancelable_wait and pthread_cond::wait.
	(pthread_cond_timedwait): Adjust to change in __pthread_cond_dowait.
	(pthread_cond_wait): Ditto.
	(semaphore::_timedwait): Adjust to change in cancelable_wait.
	(semaphore::_wait): Ditto.
	* exceptions.cc (handle_sigsuspend): Ditto.
	* signal.cc (nanosleep): Ditto.
	* wait.cc (wait4): Ditto.
	* times.cc (FACTOR, NSPERSEC): Move from here...
	* hires.h (FACTOR, NSPERSEC): ...to here.

Index: cygtls.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/cygtls.cc,v
retrieving revision 1.76
diff -u -p -r1.76 cygtls.cc
--- cygtls.cc	21 Apr 2011 08:10:28 -0000	1.76
+++ cygtls.cc	29 Jul 2011 05:56:31 -0000
@@ -98,6 +98,7 @@ _cygtls::init_thread (void *x, DWORD (*f
   thread_id = GetCurrentThreadId ();
   initialized = CYGTLS_INITIALIZED;
   errno_addr = &(local_clib._errno);
+  locals.cw_timer = NULL;
 
   if ((void *) func == (void *) cygthread::stub
       || (void *) func == (void *) cygthread::simplestub)
@@ -127,6 +128,7 @@ _cygtls::fixup_after_fork ()
     }
   stacklock = spinning = 0;
   locals.select.sockevt = NULL;
+  locals.cw_timer = NULL;
   wq.thread_ev = NULL;
 }
 
Index: cygtls.h
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/cygtls.h,v
retrieving revision 1.72
diff -u -p -r1.72 cygtls.h
--- cygtls.h	28 May 2011 18:17:08 -0000	1.72
+++ cygtls.h	29 Jul 2011 05:56:31 -0000
@@ -131,6 +131,9 @@ struct _local_storage
   int setmode_file;
   int setmode_mode;
 
+  /* thread.cc */
+  HANDLE cw_timer;
+
   /* All functions requiring temporary path buffers. */
   tls_pathbuf pathbufs;
   char ttybuf[32];
Index: exceptions.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/exceptions.cc,v
retrieving revision 1.359
diff -u -p -r1.359 exceptions.cc
--- exceptions.cc	13 Jul 2011 17:53:21 -0000	1.359
+++ exceptions.cc	29 Jul 2011 05:56:32 -0000
@@ -719,7 +719,7 @@ handle_sigsuspend (sigset_t tempmask)
   sigproc_printf ("oldmask %p, newmask %p", oldmask, tempmask);
 
   pthread_testcancel ();
-  cancelable_wait (signal_arrived, INFINITE);
+  cancelable_wait (signal_arrived);
 
   set_sig_errno (EINTR);	// Per POSIX
 
Index: hires.h
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/hires.h,v
retrieving revision 1.19
diff -u -p -r1.19 hires.h
--- hires.h	17 May 2011 17:08:09 -0000	1.19
+++ hires.h	29 Jul 2011 05:56:32 -0000
@@ -29,6 +29,11 @@ details. */
    and rounding won't exceed HIRES_DELAY_MAX */
 #define HIRES_DELAY_MAX ((((UINT_MAX - 10000) / 1000) * 1000) + 10)
 
+/* 100ns difference between Windows and UNIX timebase. */
+#define FACTOR (0x19db1ded53e8000LL)
+/* # of 100ns intervals per second. */
+#define NSPERSEC 10000000LL
+
 class hires_base
 {
  protected:
Index: ntdll.h
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/ntdll.h,v
retrieving revision 1.125
diff -u -p -r1.125 ntdll.h
--- ntdll.h	26 Jul 2011 09:54:11 -0000	1.125
+++ ntdll.h	29 Jul 2011 05:56:32 -0000
@@ -986,6 +986,15 @@ typedef struct _THREAD_BASIC_INFORMATION
     KPRIORITY  BasePriority;
 } THREAD_BASIC_INFORMATION, *PTHREAD_BASIC_INFORMATION;
 
+typedef enum _TIMER_INFORMATION_CLASS {
+  TimerBasicInformation = 0
+} TIMER_INFORMATION_CLASS, *PTIMER_INFORMATION_CLASS;
+
+typedef struct _TIMER_BASIC_INFORMATION {
+  LARGE_INTEGER TimeRemaining;
+  BOOLEAN SignalState;
+} TIMER_BASIC_INFORMATION, *PTIMER_BASIC_INFORMATION;
+
 #define RTL_QUERY_REGISTRY_SUBKEY 0x01
 #define RTL_QUERY_REGISTRY_TOPKEY 0x02
 #define RTL_QUERY_REGISTRY_REQUIRED 0x04
@@ -1155,6 +1164,8 @@ extern "C"
   NTSTATUS NTAPI NtQuerySecurityObject (HANDLE, SECURITY_INFORMATION,
 					PSECURITY_DESCRIPTOR, ULONG, PULONG);
   NTSTATUS NTAPI NtQuerySymbolicLinkObject (HANDLE, PUNICODE_STRING, PULONG);
+  NTSTATUS NTAPI NtQueryTimer (HANDLE, TIMER_INFORMATION_CLASS, PVOID,
+			       ULONG, PULONG);
   NTSTATUS NTAPI NtQueryTimerResolution (PULONG, PULONG, PULONG);
   NTSTATUS NTAPI NtQueryValueKey (HANDLE, PUNICODE_STRING,
 				  KEY_VALUE_INFORMATION_CLASS, PVOID, ULONG,
Index: signal.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/signal.cc,v
retrieving revision 1.98
diff -u -p -r1.98 signal.cc
--- signal.cc	10 Jul 2011 00:01:33 -0000	1.98
+++ signal.cc	29 Jul 2011 05:56:32 -0000
@@ -92,61 +92,31 @@ nanosleep (const struct timespec *rqtp, 
       set_errno (EINVAL);
       return -1;
     }
-  unsigned int sec = rqtp->tv_sec;
-  DWORD resolution = gtod.resolution ();
-  bool done = false;
-  DWORD req;
-  DWORD rem;
-
-  while (!done)
-    {
-      /* Divide user's input into transactions no larger than 49.7
-	 days at a time.  */
-      if (sec > HIRES_DELAY_MAX / 1000)
-	{
-	  req = ((HIRES_DELAY_MAX + resolution - 1)
-		 / resolution * resolution);
-	  sec -= HIRES_DELAY_MAX / 1000;
-	}
-      else
-	{
-	  req = ((sec * 1000 + (rqtp->tv_nsec + 999999) / 1000000
-		  + resolution - 1) / resolution) * resolution;
-	  sec = 0;
-	  done = true;
-	}
+  LARGE_INTEGER timeout;
 
-      DWORD end_time = gtod.dmsecs () + req;
-      syscall_printf ("nanosleep (%ld)", req);
+  timeout.QuadPart = (LONGLONG) rqtp->tv_sec * NSPERSEC
+		     + ((LONGLONG) rqtp->tv_nsec + 99LL) / 100LL;
+  timeout.QuadPart *= -1LL;
 
-      int rc = cancelable_wait (signal_arrived, req);
-      if ((rem = end_time - gtod.dmsecs ()) > HIRES_DELAY_MAX)
-	rem = 0;
-      if (rc == WAIT_OBJECT_0)
-	{
-	  _my_tls.call_signal_handler ();
-	  set_errno (EINTR);
-	  res = -1;
-	  break;
-	}
+  syscall_printf ("nanosleep (%ld.%09ld)", rqtp->tv_sec, rqtp->tv_nsec);
+
+  int rc = cancelable_wait (signal_arrived, &timeout);
+  if (rc == WAIT_OBJECT_0)
+    {
+      _my_tls.call_signal_handler ();
+      set_errno (EINTR);
+      res = -1;
     }
 
   if (rmtp)
     {
-      rmtp->tv_sec = sec + rem / 1000;
-      rmtp->tv_nsec = (rem % 1000) * 1000000;
-      if (sec)
-	{
-	  rmtp->tv_nsec += rqtp->tv_nsec;
-	  if (rmtp->tv_nsec >= 1000000000)
-	    {
-	      rmtp->tv_nsec -= 1000000000;
-	      rmtp->tv_sec++;
-	    }
-	}
+      rmtp->tv_sec = (time_t) (timeout.QuadPart / NSPERSEC);
+      rmtp->tv_nsec = (long) ((timeout.QuadPart % NSPERSEC) * 100LL);
     }
 
-  syscall_printf ("%d = nanosleep (%ld, %ld)", res, req, rem);
+  syscall_printf ("%d = nanosleep (%ld.%09ld, %ld.%09.ld)", res, rqtp->tv_sec,
+		  rqtp->tv_nsec, rmtp ? rmtp->tv_sec : 0,
+		  rmtp ? rmtp->tv_nsec : 0);
   return res;
 }
 
Index: thread.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/thread.cc,v
retrieving revision 1.244
diff -u -p -r1.244 thread.cc
--- thread.cc	21 Jul 2011 09:39:21 -0000	1.244
+++ thread.cc	29 Jul 2011 05:56:32 -0000
@@ -906,13 +906,13 @@ pthread::static_cancel_self ()
 }
 
 DWORD
-cancelable_wait (HANDLE object, DWORD timeout,
+cancelable_wait (HANDLE object, PLARGE_INTEGER timeout,
 		 const cw_cancel_action cancel_action,
 		 const enum cw_sig_wait sig_wait)
 {
   DWORD res;
   DWORD num = 0;
-  HANDLE wait_objects[3];
+  HANDLE wait_objects[4];
   pthread_t thread = pthread::self ();
 
   /* Do not change the wait order.
@@ -939,15 +939,30 @@ cancelable_wait (HANDLE object, DWORD ti
       wait_objects[sig_n] = signal_arrived;
     }
 
+  DWORD timeout_n;
+  if (!timeout)
+    timeout_n = WAIT_TIMEOUT + 1;
+  else
+    {
+      timeout_n = WAIT_OBJECT_0 + num++;
+      if (!_my_tls.locals.cw_timer)
+	NtCreateTimer (&_my_tls.locals.cw_timer, TIMER_ALL_ACCESS, NULL,
+		       NotificationTimer);
+      NtSetTimer (_my_tls.locals.cw_timer, timeout, NULL, NULL, FALSE, 0, NULL);
+      wait_objects[timeout_n] = _my_tls.locals.cw_timer;
+    }
+
   while (1)
     {
-      res = WaitForMultipleObjects (num, wait_objects, FALSE, timeout);
+      res = WaitForMultipleObjects (num, wait_objects, FALSE, INFINITE);
       if (res == cancel_n)
 	{
 	  if (cancel_action == cw_cancel_self)
 	    pthread::static_cancel_self ();
 	  res = WAIT_CANCELED;
 	}
+      else if (res == timeout_n)
+	res = WAIT_TIMEOUT;
       else if (res != sig_n)
 	/* all set */;
       else if (sig_wait == cw_sig_eintr)
@@ -959,6 +974,20 @@ cancelable_wait (HANDLE object, DWORD ti
 	}
       break;
     }
+
+  if (timeout)
+    {
+      PTIMER_BASIC_INFORMATION tbi;
+      const size_t sizeof_tbi = sizeof (TIMER_BASIC_INFORMATION);
+
+      tbi = (PTIMER_BASIC_INFORMATION) malloc (sizeof_tbi);
+      NtQueryTimer (_my_tls.locals.cw_timer, TimerBasicInformation, tbi,
+		    sizeof_tbi, NULL);
+      if (timeout->QuadPart < 0LL)
+	*timeout = tbi->TimeRemaining;
+      NtCancelTimer (_my_tls.locals.cw_timer, NULL);
+    }
+
   return res;
 }
 
@@ -1228,7 +1257,7 @@ pthread_cond::unblock (const bool all)
 }
 
 int
-pthread_cond::wait (pthread_mutex_t mutex, DWORD dwMilliseconds)
+pthread_cond::wait (pthread_mutex_t mutex, PLARGE_INTEGER timeout)
 {
   DWORD rv;
 
@@ -1249,7 +1278,7 @@ pthread_cond::wait (pthread_mutex_t mute
   ++mutex->condwaits;
   mutex->unlock ();
 
-  rv = cancelable_wait (sem_wait, dwMilliseconds, cw_no_cancel_self, cw_sig_eintr);
+  rv = cancelable_wait (sem_wait, timeout, cw_no_cancel_self, cw_sig_eintr);
 
   mtx_out.lock ();
 
@@ -1764,7 +1793,7 @@ pthread_mutex::lock ()
   else if (type == PTHREAD_MUTEX_NORMAL /* potentially causes deadlock */
 	   || !pthread::equal (owner, self))
     {
-      cancelable_wait (win32_obj_id, INFINITE, cw_no_cancel, cw_sig_resume);
+      cancelable_wait (win32_obj_id, NULL, cw_no_cancel, cw_sig_resume);
       set_owner (self);
     }
   else
@@ -1899,8 +1928,13 @@ pthread_spinlock::lock ()
 	}
       else if (pthread::equal (owner, self))
 	result = EDEADLK;
-      else /* Minimal timeout to minimize CPU usage while still spinning. */
-	cancelable_wait (win32_obj_id, 1L, cw_no_cancel, cw_sig_resume);
+      else
+	{
+	  /* Minimal timeout to minimize CPU usage while still spinning. */
+	  LARGE_INTEGER timeout;
+	  timeout.QuadPart = -10000LL;
+	  cancelable_wait (win32_obj_id, &timeout, cw_no_cancel, cw_sig_resume);
+	}
     }
   while (result == -1);
   pthread_printf ("spinlock %p, self %p, owner %p", this, self, owner);
@@ -2377,7 +2411,7 @@ pthread::join (pthread_t *thread, void *
       (*thread)->attr.joinable = PTHREAD_CREATE_DETACHED;
       (*thread)->mutex.unlock ();
 
-      switch (cancelable_wait ((*thread)->win32_obj_id, INFINITE, cw_no_cancel_self, cw_sig_resume))
+      switch (cancelable_wait ((*thread)->win32_obj_id, NULL, cw_no_cancel_self, cw_sig_resume))
 	{
 	case WAIT_OBJECT_0:
 	  if (return_val)
@@ -2702,7 +2736,7 @@ pthread_cond_signal (pthread_cond_t *con
 
 static int
 __pthread_cond_dowait (pthread_cond_t *cond, pthread_mutex_t *mutex,
-		       DWORD waitlength)
+		       PLARGE_INTEGER waitlength)
 {
   if (!pthread_mutex::is_good_object (mutex))
     return EINVAL;
@@ -2722,7 +2756,7 @@ pthread_cond_timedwait (pthread_cond_t *
 			const struct timespec *abstime)
 {
   struct timespec tp;
-  DWORD waitlength;
+  LARGE_INTEGER timeout;
 
   myfault efault;
   if (efault.faulted ())
@@ -2738,17 +2772,26 @@ pthread_cond_timedwait (pthread_cond_t *
 
   clock_gettime ((*cond)->clock_id, &tp);
 
-  /* Check for immediate timeout before converting to microseconds, since
-     the resulting value can easily overflow long.  This also allows to
-     evaluate microseconds directly in DWORD. */
+  /* Check for immediate timeout before converting */
   if (tp.tv_sec > abstime->tv_sec
       || (tp.tv_sec == abstime->tv_sec
 	  && tp.tv_nsec > abstime->tv_nsec))
     return ETIMEDOUT;
 
-  waitlength = (abstime->tv_sec - tp.tv_sec) * 1000;
-  waitlength += (abstime->tv_nsec - tp.tv_nsec) / 1000000;
-  return __pthread_cond_dowait (cond, mutex, waitlength);
+  switch ((*cond)->clock_id)
+    {
+    case CLOCK_REALTIME:
+      timeout.QuadPart = abstime->tv_sec * NSPERSEC
+			 + (abstime->tv_nsec + 99LL) / 100LL + FACTOR;
+      break;
+    default:
+      /* other clocks must be handled as relative timeout */
+      timeout.QuadPart = (abstime->tv_sec - tp.tv_sec) * NSPERSEC;
+      timeout.QuadPart += (abstime->tv_nsec - tp.tv_nsec + 99LL) / 100LL;
+      timeout.QuadPart *= -1LL;
+      break;
+    }
+  return __pthread_cond_dowait (cond, mutex, &timeout);
 }
 
 extern "C" int
@@ -2756,7 +2799,7 @@ pthread_cond_wait (pthread_cond_t *cond,
 {
   pthread_testcancel ();
 
-  return __pthread_cond_dowait (cond, mutex, INFINITE);
+  return __pthread_cond_dowait (cond, mutex, NULL);
 }
 
 extern "C" int
@@ -3439,8 +3482,7 @@ semaphore::_trywait ()
 int
 semaphore::_timedwait (const struct timespec *abstime)
 {
-  struct timeval tv;
-  long waitlength;
+  LARGE_INTEGER timeout;
 
   myfault efault;
   if (efault.faulted ())
@@ -3453,12 +3495,10 @@ semaphore::_timedwait (const struct time
       return -1;
     }
 
-  gettimeofday (&tv, NULL);
-  waitlength = abstime->tv_sec * 1000 + abstime->tv_nsec / (1000 * 1000);
-  waitlength -= tv.tv_sec * 1000 + tv.tv_usec / 1000;
-  if (waitlength < 0)
-    waitlength = 0;
-  switch (cancelable_wait (win32_obj_id, waitlength, cw_cancel_self, cw_sig_eintr))
+  timeout.QuadPart = abstime->tv_sec * NSPERSEC
+		     + (abstime->tv_nsec + 99) / 100 + FACTOR;
+
+  switch (cancelable_wait (win32_obj_id, &timeout, cw_cancel_self, cw_sig_eintr))
     {
     case WAIT_OBJECT_0:
       currentvalue--;
@@ -3480,7 +3520,7 @@ semaphore::_timedwait (const struct time
 int
 semaphore::_wait ()
 {
-  switch (cancelable_wait (win32_obj_id, INFINITE, cw_cancel_self, cw_sig_eintr))
+  switch (cancelable_wait (win32_obj_id, NULL, cw_cancel_self, cw_sig_eintr))
     {
     case WAIT_OBJECT_0:
       currentvalue--;
Index: thread.h
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/thread.h,v
retrieving revision 1.122
diff -u -p -r1.122 thread.h
--- thread.h	21 Jul 2011 09:39:22 -0000	1.122
+++ thread.h	29 Jul 2011 05:56:32 -0000
@@ -37,7 +37,8 @@ enum cw_cancel_action
   cw_no_cancel
 };
 
-DWORD cancelable_wait (HANDLE, DWORD, const cw_cancel_action = cw_cancel_self,
+DWORD cancelable_wait (HANDLE, PLARGE_INTEGER timeout = NULL,
+		       const cw_cancel_action = cw_cancel_self,
 		       const enum cw_sig_wait = cw_sig_nosig)
   __attribute__ ((regparm (3)));
 
@@ -70,7 +71,7 @@ public:
   void lock ()
   {
     if (InterlockedIncrement ((long *) &lock_counter) != 1)
-      cancelable_wait (win32_obj_id, INFINITE, cw_no_cancel, cw_sig_resume);
+      cancelable_wait (win32_obj_id, NULL, cw_no_cancel, cw_sig_resume);
   }
 
   void unlock ()
@@ -517,7 +518,7 @@ public:
   pthread_mutex_t mtx_cond;
 
   void unblock (const bool all);
-  int wait (pthread_mutex_t mutex, DWORD dwMilliseconds = INFINITE);
+  int wait (pthread_mutex_t mutex, PLARGE_INTEGER timeout = NULL);
 
   pthread_cond (pthread_condattr *);
   ~pthread_cond ();
Index: times.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/times.cc,v
retrieving revision 1.110
diff -u -p -r1.110 times.cc
--- times.cc	6 Jun 2011 05:02:13 -0000	1.110
+++ times.cc	29 Jul 2011 05:56:32 -0000
@@ -27,10 +27,6 @@ details. */
 #include "cygtls.h"
 #include "ntdll.h"
 
-/* 100ns difference between WIndows and UNIX timebase. */
-#define FACTOR (0x19db1ded53e8000LL)
-/* # of 100ns intervals per second. */
-#define NSPERSEC 10000000LL
 /* Max allowed diversion in 100ns of internal timer from system time.  If
    this difference is exceeded, the internal timer gets re-primed. */
 #define JITTER (40 * 10000LL)
@@ -737,7 +733,7 @@ hires_ms::resolution ()
 
       status = NtQueryTimerResolution (&coarsest, &finest, &actual);
       if (NT_SUCCESS (status))
-	minperiod = (DWORD) actual / 10000L;
+	minperiod = (DWORD) actual;
       else
 	{
 	  /* Try to empirically determine current timer resolution */
@@ -757,13 +753,9 @@ hires_ms::resolution ()
 	      period += now - then;
 	    }
 	  SetThreadPriority (GetCurrentThread (), priority);
-	  period /= 40000L;
+	  period /= 4L;
 	  minperiod = (DWORD) period;
 	}
-      /* The resolution can be as low as 5000 100ns intervals on recent OSes.
-	 We have to make sure that the resolution in ms is never 0. */
-      if (!minperiod)
-	minperiod = 1L;
     }
   return minperiod;
 }
@@ -786,8 +778,8 @@ clock_getres (clockid_t clk_id, struct t
       case CLOCK_REALTIME:
 	{
 	  DWORD period = gtod.resolution ();
-	  tp->tv_sec = period / 1000;
-	  tp->tv_nsec = (period % 1000) * 1000000;
+	  tp->tv_sec = period / NSPERSEC;
+	  tp->tv_nsec = (period % NSPERSEC) * 100;
 	  break;
 	}
 
@@ -838,7 +830,7 @@ clock_setres (clockid_t clk_id, struct t
     }
 
   if (period_set
-      && NT_SUCCESS (NtSetTimerResolution (minperiod * 10000L, FALSE, &actual)))
+      && NT_SUCCESS (NtSetTimerResolution (minperiod, FALSE, &actual)))
     period_set = false;
 
   status = NtSetTimerResolution (period, TRUE, &actual);
@@ -847,11 +839,7 @@ clock_setres (clockid_t clk_id, struct t
       __seterrno_from_nt_status (status);
       return -1;
     }
-  minperiod = actual / 10000L;
-  /* The resolution can be as low as 5000 100ns intervals on recent OSes.
-     We have to make sure that the resolution in ms is never 0. */
-  if (!minperiod)
-    minperiod = 1L;
+  minperiod = actual;
   period_set = true;
   return 0;
 }
Index: tlsoffsets.h
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/tlsoffsets.h,v
retrieving revision 1.47
diff -u -p -r1.47 tlsoffsets.h
--- tlsoffsets.h	28 May 2011 18:17:09 -0000	1.47
+++ tlsoffsets.h	29 Jul 2011 05:56:32 -0000
@@ -1,6 +1,6 @@
 //;# autogenerated:  Do not edit.
 
-//; $tls::sizeof__cygtls = 4044;
+//; $tls::sizeof__cygtls = 4048;
 //; $tls::func = -12700;
 //; $tls::pfunc = 0;
 //; $tls::saved_errno = -12696;
@@ -37,26 +37,26 @@
 //; $tls::p__dontuse = 412;
 //; $tls::locals = -11200;
 //; $tls::plocals = 1500;
-//; $tls::_ctinfo = -9740;
-//; $tls::p_ctinfo = 2960;
-//; $tls::andreas = -9736;
-//; $tls::pandreas = 2964;
-//; $tls::wq = -9732;
-//; $tls::pwq = 2968;
-//; $tls::sig = -9704;
-//; $tls::psig = 2996;
-//; $tls::incyg = -9700;
-//; $tls::pincyg = 3000;
-//; $tls::spinning = -9696;
-//; $tls::pspinning = 3004;
-//; $tls::stacklock = -9692;
-//; $tls::pstacklock = 3008;
-//; $tls::stackptr = -9688;
-//; $tls::pstackptr = 3012;
-//; $tls::stack = -9684;
-//; $tls::pstack = 3016;
-//; $tls::initialized = -8660;
-//; $tls::pinitialized = 4040;
+//; $tls::_ctinfo = -9736;
+//; $tls::p_ctinfo = 2964;
+//; $tls::andreas = -9732;
+//; $tls::pandreas = 2968;
+//; $tls::wq = -9728;
+//; $tls::pwq = 2972;
+//; $tls::sig = -9700;
+//; $tls::psig = 3000;
+//; $tls::incyg = -9696;
+//; $tls::pincyg = 3004;
+//; $tls::spinning = -9692;
+//; $tls::pspinning = 3008;
+//; $tls::stacklock = -9688;
+//; $tls::pstacklock = 3012;
+//; $tls::stackptr = -9684;
+//; $tls::pstackptr = 3016;
+//; $tls::stack = -9680;
+//; $tls::pstack = 3020;
+//; $tls::initialized = -8656;
+//; $tls::pinitialized = 4044;
 //; __DATA__
 
 #define tls_func (-12700)
@@ -95,23 +95,23 @@
 #define tls_p__dontuse (412)
 #define tls_locals (-11200)
 #define tls_plocals (1500)
-#define tls__ctinfo (-9740)
-#define tls_p_ctinfo (2960)
-#define tls_andreas (-9736)
-#define tls_pandreas (2964)
-#define tls_wq (-9732)
-#define tls_pwq (2968)
-#define tls_sig (-9704)
-#define tls_psig (2996)
-#define tls_incyg (-9700)
-#define tls_pincyg (3000)
-#define tls_spinning (-9696)
-#define tls_pspinning (3004)
-#define tls_stacklock (-9692)
-#define tls_pstacklock (3008)
-#define tls_stackptr (-9688)
-#define tls_pstackptr (3012)
-#define tls_stack (-9684)
-#define tls_pstack (3016)
-#define tls_initialized (-8660)
-#define tls_pinitialized (4040)
+#define tls__ctinfo (-9736)
+#define tls_p_ctinfo (2964)
+#define tls_andreas (-9732)
+#define tls_pandreas (2968)
+#define tls_wq (-9728)
+#define tls_pwq (2972)
+#define tls_sig (-9700)
+#define tls_psig (3000)
+#define tls_incyg (-9696)
+#define tls_pincyg (3004)
+#define tls_spinning (-9692)
+#define tls_pspinning (3008)
+#define tls_stacklock (-9688)
+#define tls_pstacklock (3012)
+#define tls_stackptr (-9684)
+#define tls_pstackptr (3016)
+#define tls_stack (-9680)
+#define tls_pstack (3020)
+#define tls_initialized (-8656)
+#define tls_pinitialized (4044)
Index: wait.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/wait.cc,v
retrieving revision 1.37
diff -u -p -r1.37 wait.cc
--- wait.cc	18 Jul 2009 20:25:07 -0000	1.37
+++ wait.cc	29 Jul 2011 05:56:32 -0000
@@ -78,7 +78,7 @@ wait4 (int intpid, int *status, int opti
       if ((waitfor = w->ev) == NULL)
 	goto nochildren;
 
-      res = cancelable_wait (waitfor, INFINITE);
+      res = cancelable_wait (waitfor);
 
       sigproc_printf ("%d = WaitForSingleObject (...)", res);
 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] clock_nanosleep(2)
  2011-08-02  4:09       ` Yaakov (Cygwin/X)
@ 2011-08-02 15:43         ` Corinna Vinschen
  2011-08-03  6:20           ` Yaakov (Cygwin/X)
  0 siblings, 1 reply; 28+ messages in thread
From: Corinna Vinschen @ 2011-08-02 15:43 UTC (permalink / raw)
  To: cygwin-patches

On Aug  1 23:09, Yaakov (Cygwin/X) wrote:
> On Sun, 2011-07-31 at 10:24 +0200, Corinna Vinschen wrote:
> > anything new from the clock_nanosleep frontier?
> 
> Sorry, I've been having elusive problems with CVS HEAD that have been
> making it hard to test my patch.
> 
> Here's what I have so far, FWIW.  So far I've found two problems with
> it: the remaining time returned is incorrect, based on testing of
> nanosleep(),

Does that mean the return value from NtQueryTimer is unreliable?  In
what way is it wrong?  Does nanosleep wait too long or not long enough?
If NtQueryTimer is unusable, maybe we should just skip the idea to return
the remaining time from cancelabel_wait and simply use the return
value from hires_ms::timeGetTime_ns() to return the remaining time
from {clock_}nanosleep, kind of like

  LONGLONG remaining = hires_ms::timeGetTime_ns ();
  cancelable_wait();
  LONGLONG remaining = hires_ms::timeGetTime_ns () - start;
  rem->tv_sec = remaining / NSPERSEC;
  rem->tv_nsec = remaining - (rem->tv_sec * NSPERSEC);

> and the pthread_spin chunk doesn't look right (previously
> the timeout would repeat in the while loop, but that won't happen the
> way the waitable timer is set up).

It doesn't look wrong to me, but then again, I didn't test it...
> 
> I'll try to get back to this as soon as I am able to test this properly.
> In the meantime, is there anything obvious I'm missing?

Nothing I can think of.  This looks good.


Thanks,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] clock_nanosleep(2)
  2011-08-02 15:43         ` Corinna Vinschen
@ 2011-08-03  6:20           ` Yaakov (Cygwin/X)
  2011-08-03  7:45             ` Corinna Vinschen
  0 siblings, 1 reply; 28+ messages in thread
From: Yaakov (Cygwin/X) @ 2011-08-03  6:20 UTC (permalink / raw)
  To: cygwin-patches

[-- Attachment #1: Type: text/plain, Size: 1133 bytes --]

On Tue, 2011-08-02 at 17:42 +0200, Corinna Vinschen wrote:
> Does that mean the return value from NtQueryTimer is unreliable?
> In what way is it wrong?  

I'm not sure.  When I run an STC (attached), it works as expected.  In
cancelable_wait(), however, it returns the negative system uptime.  Is
Cygwin doing something to make this occur?

> Does nanosleep wait too long or not long enough?

That doesn't seem to be an issue.

> If NtQueryTimer is unusable, maybe we should just skip the idea to return
> the remaining time from cancelabel_wait and simply use the return
> value from hires_ms::timeGetTime_ns() to return the remaining time
> from {clock_}nanosleep

I'd rather avoid this type of workaround, particularly with
clock_nanosleep having to deal with CLOCK_MONOTONIC as well.

> > and the pthread_spin chunk doesn't look right (previously
> > the timeout would repeat in the while loop, but that won't happen the
> > way the waitable timer is set up).
> 
> It doesn't look wrong to me, but then again, I didn't test it...

You're right, since we pay no attention to the return value of
cancelable_wait() here.


Yaakov


[-- Attachment #2: ntquerytimer-test.c --]
[-- Type: text/x-csrc, Size: 888 bytes --]

#pragma CCOD:script no
#pragma CCOD:options -lntdll

#include <stdlib.h>
#include <stdio.h>
#include <windows.h>
#include <ddk/ntapi.h>

int main(void) {
  HANDLE timer;
  LARGE_INTEGER li;
  PTIMER_BASIC_INFORMATION tbi;
  const size_t sizeof_tbi = sizeof (TIMER_BASIC_INFORMATION);
  int i;

  li.QuadPart = -20000000LL;

  NtCreateTimer (&timer, TIMER_ALL_ACCESS, NULL, NotificationTimer);

  for (i = 0; i < 10; i++)
    {
      NtSetTimer (timer, &li, NULL, NULL, FALSE, 0, NULL);
      WaitForSingleObject (timer, 1000);

      tbi = (PTIMER_BASIC_INFORMATION) malloc (sizeof_tbi);
      NtQueryTimer (timer, TimerBasicInformation, tbi, sizeof_tbi, NULL);
      NtCancelTimer (timer, NULL);

      printf ("%ld.%09ld\n", (long) (tbi->TimeRemaining.QuadPart / 10000000LL),
	      (long) ((tbi->TimeRemaining.QuadPart % 10000000LL) * 100LL));
    }

  NtClose (timer);

  return 0;
}

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] clock_nanosleep(2)
  2011-08-03  6:20           ` Yaakov (Cygwin/X)
@ 2011-08-03  7:45             ` Corinna Vinschen
  2011-08-03  9:20               ` Yaakov (Cygwin/X)
  0 siblings, 1 reply; 28+ messages in thread
From: Corinna Vinschen @ 2011-08-03  7:45 UTC (permalink / raw)
  To: cygwin-patches

On Aug  3 01:20, Yaakov (Cygwin/X) wrote:
> On Tue, 2011-08-02 at 17:42 +0200, Corinna Vinschen wrote:
> > Does that mean the return value from NtQueryTimer is unreliable?
> > In what way is it wrong?  
> 
> I'm not sure.  When I run an STC (attached), it works as expected.  In
> cancelable_wait(), however, it returns the negative system uptime.  Is
> Cygwin doing something to make this occur?

That sounds weird.  How should Cygwin influence what an independent OS
function returns?  And you sure it's the system uptime?  Wow.

I'll have a look into that, but I doubt I see more than you.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] clock_nanosleep(2)
  2011-08-03  7:45             ` Corinna Vinschen
@ 2011-08-03  9:20               ` Yaakov (Cygwin/X)
  2011-08-03  9:28                 ` Corinna Vinschen
  0 siblings, 1 reply; 28+ messages in thread
From: Yaakov (Cygwin/X) @ 2011-08-03  9:20 UTC (permalink / raw)
  To: cygwin-patches

[-- Attachment #1: Type: text/plain, Size: 1175 bytes --]

On Wed, 2011-08-03 at 09:45 +0200, Corinna Vinschen wrote:
> On Aug  3 01:20, Yaakov (Cygwin/X) wrote:
> > On Tue, 2011-08-02 at 17:42 +0200, Corinna Vinschen wrote:
> > > Does that mean the return value from NtQueryTimer is unreliable?
> > > In what way is it wrong?  
> > 
> > I'm not sure.  When I run an STC (attached), it works as expected.  In
> > cancelable_wait(), however, it returns the negative system uptime.  Is
> > Cygwin doing something to make this occur?
> 
> That sounds weird.  How should Cygwin influence what an independent OS
> function returns?  And you sure it's the system uptime?  Wow.

Never mind, I figured it out.  The difference is the timeout to
WaitFor*Object*(); my STC doesn't allow the timer to finish, but
cancelable_wait() does with the INFINITE timeout.  If there is time
remaining, as in the STC, then TIMER_BASIC_INFORMATION.TimeRemaining
contains just that (as a positive).  If the timer has signalled, then
instead of zero, it appears to provide when it was signalled (system
uptime, as a negative).

With that figured out, here's a revised patch.  Once this is in, then
adding clock_nanosleep() should be relatively easy.


Yaakov


[-- Attachment #2: cygwin-waitable-timer.patch --]
[-- Type: text/x-patch, Size: 22181 bytes --]

2011-08-03  Yaakov Selkowitz  <yselkowitz@...>

	* cygtls.h (struct _local_storage): Add cw_timer member.
	* cygtls.cc (_cygtls::init_thread): Initialize locals.cw_timer.
	(_cygtls::fixup_after_fork): Ditto.
	* tlsoffsets.h: Regenerate.
	* ntdll.h (enum _TIMER_INFORMATION_CLASS): Define.
	(struct _TIMER_BASIC_INFORMATION): Define.
	(NtQueryTimer): Declare function.
	* thread.h (cancelable_wait): Change timeout argument to
	PLARGE_INTEGER and provide NULL default.
	(fast_mutex::lock): Adjust accordingly.
	(pthread_cond::wait): Change timeout argument to PLARGE_INTEGER
	and default to NULL.
	* thread.cc (cancelable_wait): Change timeout argument to
	PLARGE_INTEGER.  Initialize _cygtls.locals.cw_timer if needed.
	Use NT waitable timers for handling timeout.  Return remaining time
	to timeout argument if timeout was relative.
	(pthread_cond::wait): Change timeout argument to PLARGE_INTEGER.
	Adjust to change in cancelable_wait.
	(pthread_mutex::lock): Adjust to change in cancelable_wait.
	(pthread_spinlock::lock): Ditto.
	(pthread::join): Ditto.
	(__pthread_cond_dowait): Change waitlength argument to PLARGE_INTEGER.
	Adjust to changes in cancelable_wait and pthread_cond::wait.
	(pthread_cond_timedwait): Adjust to change in __pthread_cond_dowait.
	(pthread_cond_wait): Ditto.
	(semaphore::_timedwait): Adjust to change in cancelable_wait.
	(semaphore::_wait): Ditto.
	* exceptions.cc (handle_sigsuspend): Ditto.
	* signal.cc (nanosleep): Ditto.
	* wait.cc (wait4): Ditto.
	* times.cc (FACTOR, NSPERSEC): Move from here...
	* hires.h (FACTOR, NSPERSEC): ...to here.

Index: cygtls.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/cygtls.cc,v
retrieving revision 1.76
diff -u -p -r1.76 cygtls.cc
--- cygtls.cc	21 Apr 2011 08:10:28 -0000	1.76
+++ cygtls.cc	29 Jul 2011 05:56:31 -0000
@@ -98,6 +98,7 @@ _cygtls::init_thread (void *x, DWORD (*f
   thread_id = GetCurrentThreadId ();
   initialized = CYGTLS_INITIALIZED;
   errno_addr = &(local_clib._errno);
+  locals.cw_timer = NULL;
 
   if ((void *) func == (void *) cygthread::stub
       || (void *) func == (void *) cygthread::simplestub)
@@ -127,6 +128,7 @@ _cygtls::fixup_after_fork ()
     }
   stacklock = spinning = 0;
   locals.select.sockevt = NULL;
+  locals.cw_timer = NULL;
   wq.thread_ev = NULL;
 }
 
Index: cygtls.h
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/cygtls.h,v
retrieving revision 1.72
diff -u -p -r1.72 cygtls.h
--- cygtls.h	28 May 2011 18:17:08 -0000	1.72
+++ cygtls.h	29 Jul 2011 05:56:31 -0000
@@ -131,6 +131,9 @@ struct _local_storage
   int setmode_file;
   int setmode_mode;
 
+  /* thread.cc */
+  HANDLE cw_timer;
+
   /* All functions requiring temporary path buffers. */
   tls_pathbuf pathbufs;
   char ttybuf[32];
Index: exceptions.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/exceptions.cc,v
retrieving revision 1.359
diff -u -p -r1.359 exceptions.cc
--- exceptions.cc	13 Jul 2011 17:53:21 -0000	1.359
+++ exceptions.cc	29 Jul 2011 05:56:32 -0000
@@ -719,7 +719,7 @@ handle_sigsuspend (sigset_t tempmask)
   sigproc_printf ("oldmask %p, newmask %p", oldmask, tempmask);
 
   pthread_testcancel ();
-  cancelable_wait (signal_arrived, INFINITE);
+  cancelable_wait (signal_arrived);
 
   set_sig_errno (EINTR);	// Per POSIX
 
Index: hires.h
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/hires.h,v
retrieving revision 1.19
diff -u -p -r1.19 hires.h
--- hires.h	17 May 2011 17:08:09 -0000	1.19
+++ hires.h	29 Jul 2011 05:56:32 -0000
@@ -29,6 +29,11 @@ details. */
    and rounding won't exceed HIRES_DELAY_MAX */
 #define HIRES_DELAY_MAX ((((UINT_MAX - 10000) / 1000) * 1000) + 10)
 
+/* 100ns difference between Windows and UNIX timebase. */
+#define FACTOR (0x19db1ded53e8000LL)
+/* # of 100ns intervals per second. */
+#define NSPERSEC 10000000LL
+
 class hires_base
 {
  protected:
Index: ntdll.h
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/ntdll.h,v
retrieving revision 1.125
diff -u -p -r1.125 ntdll.h
--- ntdll.h	26 Jul 2011 09:54:11 -0000	1.125
+++ ntdll.h	29 Jul 2011 05:56:32 -0000
@@ -986,6 +986,15 @@ typedef struct _THREAD_BASIC_INFORMATION
     KPRIORITY  BasePriority;
 } THREAD_BASIC_INFORMATION, *PTHREAD_BASIC_INFORMATION;
 
+typedef enum _TIMER_INFORMATION_CLASS {
+  TimerBasicInformation = 0
+} TIMER_INFORMATION_CLASS, *PTIMER_INFORMATION_CLASS;
+
+typedef struct _TIMER_BASIC_INFORMATION {
+  LARGE_INTEGER TimeRemaining;
+  BOOLEAN SignalState;
+} TIMER_BASIC_INFORMATION, *PTIMER_BASIC_INFORMATION;
+
 #define RTL_QUERY_REGISTRY_SUBKEY 0x01
 #define RTL_QUERY_REGISTRY_TOPKEY 0x02
 #define RTL_QUERY_REGISTRY_REQUIRED 0x04
@@ -1155,6 +1164,8 @@ extern "C"
   NTSTATUS NTAPI NtQuerySecurityObject (HANDLE, SECURITY_INFORMATION,
 					PSECURITY_DESCRIPTOR, ULONG, PULONG);
   NTSTATUS NTAPI NtQuerySymbolicLinkObject (HANDLE, PUNICODE_STRING, PULONG);
+  NTSTATUS NTAPI NtQueryTimer (HANDLE, TIMER_INFORMATION_CLASS, PVOID,
+			       ULONG, PULONG);
   NTSTATUS NTAPI NtQueryTimerResolution (PULONG, PULONG, PULONG);
   NTSTATUS NTAPI NtQueryValueKey (HANDLE, PUNICODE_STRING,
 				  KEY_VALUE_INFORMATION_CLASS, PVOID, ULONG,
Index: signal.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/signal.cc,v
retrieving revision 1.98
diff -u -p -r1.98 signal.cc
--- signal.cc	10 Jul 2011 00:01:33 -0000	1.98
+++ signal.cc	29 Jul 2011 05:56:32 -0000
@@ -92,61 +92,31 @@ nanosleep (const struct timespec *rqtp, 
       set_errno (EINVAL);
       return -1;
     }
-  unsigned int sec = rqtp->tv_sec;
-  DWORD resolution = gtod.resolution ();
-  bool done = false;
-  DWORD req;
-  DWORD rem;
-
-  while (!done)
-    {
-      /* Divide user's input into transactions no larger than 49.7
-	 days at a time.  */
-      if (sec > HIRES_DELAY_MAX / 1000)
-	{
-	  req = ((HIRES_DELAY_MAX + resolution - 1)
-		 / resolution * resolution);
-	  sec -= HIRES_DELAY_MAX / 1000;
-	}
-      else
-	{
-	  req = ((sec * 1000 + (rqtp->tv_nsec + 999999) / 1000000
-		  + resolution - 1) / resolution) * resolution;
-	  sec = 0;
-	  done = true;
-	}
+  LARGE_INTEGER timeout;
 
-      DWORD end_time = gtod.dmsecs () + req;
-      syscall_printf ("nanosleep (%ld)", req);
+  timeout.QuadPart = (LONGLONG) rqtp->tv_sec * NSPERSEC
+		     + ((LONGLONG) rqtp->tv_nsec + 99LL) / 100LL;
+  timeout.QuadPart *= -1LL;
 
-      int rc = cancelable_wait (signal_arrived, req);
-      if ((rem = end_time - gtod.dmsecs ()) > HIRES_DELAY_MAX)
-	rem = 0;
-      if (rc == WAIT_OBJECT_0)
-	{
-	  _my_tls.call_signal_handler ();
-	  set_errno (EINTR);
-	  res = -1;
-	  break;
-	}
+  syscall_printf ("nanosleep (%ld.%09ld)", rqtp->tv_sec, rqtp->tv_nsec);
+
+  int rc = cancelable_wait (signal_arrived, &timeout);
+  if (rc == WAIT_OBJECT_0)
+    {
+      _my_tls.call_signal_handler ();
+      set_errno (EINTR);
+      res = -1;
     }
 
   if (rmtp)
     {
-      rmtp->tv_sec = sec + rem / 1000;
-      rmtp->tv_nsec = (rem % 1000) * 1000000;
-      if (sec)
-	{
-	  rmtp->tv_nsec += rqtp->tv_nsec;
-	  if (rmtp->tv_nsec >= 1000000000)
-	    {
-	      rmtp->tv_nsec -= 1000000000;
-	      rmtp->tv_sec++;
-	    }
-	}
+      rmtp->tv_sec = (time_t) (timeout.QuadPart / NSPERSEC);
+      rmtp->tv_nsec = (long) ((timeout.QuadPart % NSPERSEC) * 100LL);
     }
 
-  syscall_printf ("%d = nanosleep (%ld, %ld)", res, req, rem);
+  syscall_printf ("%d = nanosleep (%ld.%09ld, %ld.%09.ld)", res, rqtp->tv_sec,
+		  rqtp->tv_nsec, rmtp ? rmtp->tv_sec : 0,
+		  rmtp ? rmtp->tv_nsec : 0);
   return res;
 }
 
Index: thread.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/thread.cc,v
retrieving revision 1.244
diff -u -p -r1.244 thread.cc
--- thread.cc	21 Jul 2011 09:39:21 -0000	1.244
+++ thread.cc	29 Jul 2011 05:56:32 -0000
@@ -906,13 +906,13 @@ pthread::static_cancel_self ()
 }
 
 DWORD
-cancelable_wait (HANDLE object, DWORD timeout,
+cancelable_wait (HANDLE object, PLARGE_INTEGER timeout,
 		 const cw_cancel_action cancel_action,
 		 const enum cw_sig_wait sig_wait)
 {
   DWORD res;
   DWORD num = 0;
-  HANDLE wait_objects[3];
+  HANDLE wait_objects[4];
   pthread_t thread = pthread::self ();
 
   /* Do not change the wait order.
@@ -939,15 +939,30 @@ cancelable_wait (HANDLE object, DWORD ti
       wait_objects[sig_n] = signal_arrived;
     }
 
+  DWORD timeout_n;
+  if (!timeout)
+    timeout_n = WAIT_TIMEOUT + 1;
+  else
+    {
+      timeout_n = WAIT_OBJECT_0 + num++;
+      if (!_my_tls.locals.cw_timer)
+	NtCreateTimer (&_my_tls.locals.cw_timer, TIMER_ALL_ACCESS, NULL,
+		       NotificationTimer);
+      NtSetTimer (_my_tls.locals.cw_timer, timeout, NULL, NULL, FALSE, 0, NULL);
+      wait_objects[timeout_n] = _my_tls.locals.cw_timer;
+    }
+
   while (1)
     {
-      res = WaitForMultipleObjects (num, wait_objects, FALSE, timeout);
+      res = WaitForMultipleObjects (num, wait_objects, FALSE, INFINITE);
       if (res == cancel_n)
 	{
 	  if (cancel_action == cw_cancel_self)
 	    pthread::static_cancel_self ();
 	  res = WAIT_CANCELED;
 	}
+      else if (res == timeout_n)
+	res = WAIT_TIMEOUT;
       else if (res != sig_n)
 	/* all set */;
       else if (sig_wait == cw_sig_eintr)
@@ -959,6 +974,21 @@ cancelable_wait (HANDLE object, DWORD ti
 	}
       break;
     }
+
+  if (timeout)
+    {
+      const size_t sizeof_tbi = sizeof (TIMER_BASIC_INFORMATION);
+      PTIMER_BASIC_INFORMATION tbi = (PTIMER_BASIC_INFORMATION) malloc (sizeof_tbi);
+
+      NtQueryTimer (_my_tls.locals.cw_timer, TimerBasicInformation, tbi,
+		    sizeof_tbi, NULL);
+      /* if timer expired, TimeRemaining is negative and represents the
+	  system uptime when signalled */
+      if (timeout->QuadPart < 0LL)
+	timeout->QuadPart = tbi->SignalState ? 0LL : tbi->TimeRemaining.QuadPart;
+      NtCancelTimer (_my_tls.locals.cw_timer, NULL);
+    }
+
   return res;
 }
 
@@ -1228,7 +1257,7 @@ pthread_cond::unblock (const bool all)
 }
 
 int
-pthread_cond::wait (pthread_mutex_t mutex, DWORD dwMilliseconds)
+pthread_cond::wait (pthread_mutex_t mutex, PLARGE_INTEGER timeout)
 {
   DWORD rv;
 
@@ -1249,7 +1278,7 @@ pthread_cond::wait (pthread_mutex_t mute
   ++mutex->condwaits;
   mutex->unlock ();
 
-  rv = cancelable_wait (sem_wait, dwMilliseconds, cw_no_cancel_self, cw_sig_eintr);
+  rv = cancelable_wait (sem_wait, timeout, cw_no_cancel_self, cw_sig_eintr);
 
   mtx_out.lock ();
 
@@ -1764,7 +1793,7 @@ pthread_mutex::lock ()
   else if (type == PTHREAD_MUTEX_NORMAL /* potentially causes deadlock */
 	   || !pthread::equal (owner, self))
     {
-      cancelable_wait (win32_obj_id, INFINITE, cw_no_cancel, cw_sig_resume);
+      cancelable_wait (win32_obj_id, NULL, cw_no_cancel, cw_sig_resume);
       set_owner (self);
     }
   else
@@ -1899,8 +1928,13 @@ pthread_spinlock::lock ()
 	}
       else if (pthread::equal (owner, self))
 	result = EDEADLK;
-      else /* Minimal timeout to minimize CPU usage while still spinning. */
-	cancelable_wait (win32_obj_id, 1L, cw_no_cancel, cw_sig_resume);
+      else
+	{
+	  /* Minimal timeout to minimize CPU usage while still spinning. */
+	  LARGE_INTEGER timeout;
+	  timeout.QuadPart = -10000LL;
+	  cancelable_wait (win32_obj_id, &timeout, cw_no_cancel, cw_sig_resume);
+	}
     }
   while (result == -1);
   pthread_printf ("spinlock %p, self %p, owner %p", this, self, owner);
@@ -2377,7 +2411,7 @@ pthread::join (pthread_t *thread, void *
       (*thread)->attr.joinable = PTHREAD_CREATE_DETACHED;
       (*thread)->mutex.unlock ();
 
-      switch (cancelable_wait ((*thread)->win32_obj_id, INFINITE, cw_no_cancel_self, cw_sig_resume))
+      switch (cancelable_wait ((*thread)->win32_obj_id, NULL, cw_no_cancel_self, cw_sig_resume))
 	{
 	case WAIT_OBJECT_0:
 	  if (return_val)
@@ -2702,7 +2736,7 @@ pthread_cond_signal (pthread_cond_t *con
 
 static int
 __pthread_cond_dowait (pthread_cond_t *cond, pthread_mutex_t *mutex,
-		       DWORD waitlength)
+		       PLARGE_INTEGER waitlength)
 {
   if (!pthread_mutex::is_good_object (mutex))
     return EINVAL;
@@ -2722,7 +2756,7 @@ pthread_cond_timedwait (pthread_cond_t *
 			const struct timespec *abstime)
 {
   struct timespec tp;
-  DWORD waitlength;
+  LARGE_INTEGER timeout;
 
   myfault efault;
   if (efault.faulted ())
@@ -2738,17 +2772,27 @@ pthread_cond_timedwait (pthread_cond_t *
 
   clock_gettime ((*cond)->clock_id, &tp);
 
-  /* Check for immediate timeout before converting to microseconds, since
-     the resulting value can easily overflow long.  This also allows to
-     evaluate microseconds directly in DWORD. */
+  /* Check for immediate timeout before converting */
   if (tp.tv_sec > abstime->tv_sec
       || (tp.tv_sec == abstime->tv_sec
 	  && tp.tv_nsec > abstime->tv_nsec))
     return ETIMEDOUT;
 
-  waitlength = (abstime->tv_sec - tp.tv_sec) * 1000;
-  waitlength += (abstime->tv_nsec - tp.tv_nsec) / 1000000;
-  return __pthread_cond_dowait (cond, mutex, waitlength);
+  timeout.QuadPart = abstime->tv_sec * NSPERSEC
+		      + (abstime->tv_nsec + 99LL) / 100LL;
+
+  switch ((*cond)->clock_id)
+    {
+    case CLOCK_REALTIME:
+      timeout.QuadPart += FACTOR;
+      break;
+    default:
+      /* other clocks must be handled as relative timeout */
+      timeout.QuadPart -= tp.tv_sec * NSPERSEC + tp.tv_nsec / 100LL;
+      timeout.QuadPart *= -1LL;
+      break;
+    }
+  return __pthread_cond_dowait (cond, mutex, &timeout);
 }
 
 extern "C" int
@@ -2756,7 +2799,7 @@ pthread_cond_wait (pthread_cond_t *cond,
 {
   pthread_testcancel ();
 
-  return __pthread_cond_dowait (cond, mutex, INFINITE);
+  return __pthread_cond_dowait (cond, mutex, NULL);
 }
 
 extern "C" int
@@ -3439,8 +3482,7 @@ semaphore::_trywait ()
 int
 semaphore::_timedwait (const struct timespec *abstime)
 {
-  struct timeval tv;
-  long waitlength;
+  LARGE_INTEGER timeout;
 
   myfault efault;
   if (efault.faulted ())
@@ -3453,12 +3495,10 @@ semaphore::_timedwait (const struct time
       return -1;
     }
 
-  gettimeofday (&tv, NULL);
-  waitlength = abstime->tv_sec * 1000 + abstime->tv_nsec / (1000 * 1000);
-  waitlength -= tv.tv_sec * 1000 + tv.tv_usec / 1000;
-  if (waitlength < 0)
-    waitlength = 0;
-  switch (cancelable_wait (win32_obj_id, waitlength, cw_cancel_self, cw_sig_eintr))
+  timeout.QuadPart = abstime->tv_sec * NSPERSEC
+		     + (abstime->tv_nsec + 99) / 100 + FACTOR;
+
+  switch (cancelable_wait (win32_obj_id, &timeout, cw_cancel_self, cw_sig_eintr))
     {
     case WAIT_OBJECT_0:
       currentvalue--;
@@ -3480,7 +3520,7 @@ semaphore::_timedwait (const struct time
 int
 semaphore::_wait ()
 {
-  switch (cancelable_wait (win32_obj_id, INFINITE, cw_cancel_self, cw_sig_eintr))
+  switch (cancelable_wait (win32_obj_id, NULL, cw_cancel_self, cw_sig_eintr))
     {
     case WAIT_OBJECT_0:
       currentvalue--;
Index: thread.h
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/thread.h,v
retrieving revision 1.122
diff -u -p -r1.122 thread.h
--- thread.h	21 Jul 2011 09:39:22 -0000	1.122
+++ thread.h	29 Jul 2011 05:56:32 -0000
@@ -37,7 +37,8 @@ enum cw_cancel_action
   cw_no_cancel
 };
 
-DWORD cancelable_wait (HANDLE, DWORD, const cw_cancel_action = cw_cancel_self,
+DWORD cancelable_wait (HANDLE, PLARGE_INTEGER timeout = NULL,
+		       const cw_cancel_action = cw_cancel_self,
 		       const enum cw_sig_wait = cw_sig_nosig)
   __attribute__ ((regparm (3)));
 
@@ -70,7 +71,7 @@ public:
   void lock ()
   {
     if (InterlockedIncrement ((long *) &lock_counter) != 1)
-      cancelable_wait (win32_obj_id, INFINITE, cw_no_cancel, cw_sig_resume);
+      cancelable_wait (win32_obj_id, NULL, cw_no_cancel, cw_sig_resume);
   }
 
   void unlock ()
@@ -517,7 +518,7 @@ public:
   pthread_mutex_t mtx_cond;
 
   void unblock (const bool all);
-  int wait (pthread_mutex_t mutex, DWORD dwMilliseconds = INFINITE);
+  int wait (pthread_mutex_t mutex, PLARGE_INTEGER timeout = NULL);
 
   pthread_cond (pthread_condattr *);
   ~pthread_cond ();
Index: times.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/times.cc,v
retrieving revision 1.110
diff -u -p -r1.110 times.cc
--- times.cc	6 Jun 2011 05:02:13 -0000	1.110
+++ times.cc	29 Jul 2011 05:56:32 -0000
@@ -27,10 +27,6 @@ details. */
 #include "cygtls.h"
 #include "ntdll.h"
 
-/* 100ns difference between WIndows and UNIX timebase. */
-#define FACTOR (0x19db1ded53e8000LL)
-/* # of 100ns intervals per second. */
-#define NSPERSEC 10000000LL
 /* Max allowed diversion in 100ns of internal timer from system time.  If
    this difference is exceeded, the internal timer gets re-primed. */
 #define JITTER (40 * 10000LL)
@@ -737,7 +733,7 @@ hires_ms::resolution ()
 
       status = NtQueryTimerResolution (&coarsest, &finest, &actual);
       if (NT_SUCCESS (status))
-	minperiod = (DWORD) actual / 10000L;
+	minperiod = (DWORD) actual;
       else
 	{
 	  /* Try to empirically determine current timer resolution */
@@ -757,13 +753,9 @@ hires_ms::resolution ()
 	      period += now - then;
 	    }
 	  SetThreadPriority (GetCurrentThread (), priority);
-	  period /= 40000L;
+	  period /= 4L;
 	  minperiod = (DWORD) period;
 	}
-      /* The resolution can be as low as 5000 100ns intervals on recent OSes.
-	 We have to make sure that the resolution in ms is never 0. */
-      if (!minperiod)
-	minperiod = 1L;
     }
   return minperiod;
 }
@@ -786,8 +778,8 @@ clock_getres (clockid_t clk_id, struct t
       case CLOCK_REALTIME:
 	{
 	  DWORD period = gtod.resolution ();
-	  tp->tv_sec = period / 1000;
-	  tp->tv_nsec = (period % 1000) * 1000000;
+	  tp->tv_sec = period / NSPERSEC;
+	  tp->tv_nsec = (period % NSPERSEC) * 100;
 	  break;
 	}
 
@@ -838,7 +830,7 @@ clock_setres (clockid_t clk_id, struct t
     }
 
   if (period_set
-      && NT_SUCCESS (NtSetTimerResolution (minperiod * 10000L, FALSE, &actual)))
+      && NT_SUCCESS (NtSetTimerResolution (minperiod, FALSE, &actual)))
     period_set = false;
 
   status = NtSetTimerResolution (period, TRUE, &actual);
@@ -847,11 +839,7 @@ clock_setres (clockid_t clk_id, struct t
       __seterrno_from_nt_status (status);
       return -1;
     }
-  minperiod = actual / 10000L;
-  /* The resolution can be as low as 5000 100ns intervals on recent OSes.
-     We have to make sure that the resolution in ms is never 0. */
-  if (!minperiod)
-    minperiod = 1L;
+  minperiod = actual;
   period_set = true;
   return 0;
 }
Index: tlsoffsets.h
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/tlsoffsets.h,v
retrieving revision 1.47
diff -u -p -r1.47 tlsoffsets.h
--- tlsoffsets.h	28 May 2011 18:17:09 -0000	1.47
+++ tlsoffsets.h	29 Jul 2011 05:56:32 -0000
@@ -1,6 +1,6 @@
 //;# autogenerated:  Do not edit.
 
-//; $tls::sizeof__cygtls = 4044;
+//; $tls::sizeof__cygtls = 4048;
 //; $tls::func = -12700;
 //; $tls::pfunc = 0;
 //; $tls::saved_errno = -12696;
@@ -37,26 +37,26 @@
 //; $tls::p__dontuse = 412;
 //; $tls::locals = -11200;
 //; $tls::plocals = 1500;
-//; $tls::_ctinfo = -9740;
-//; $tls::p_ctinfo = 2960;
-//; $tls::andreas = -9736;
-//; $tls::pandreas = 2964;
-//; $tls::wq = -9732;
-//; $tls::pwq = 2968;
-//; $tls::sig = -9704;
-//; $tls::psig = 2996;
-//; $tls::incyg = -9700;
-//; $tls::pincyg = 3000;
-//; $tls::spinning = -9696;
-//; $tls::pspinning = 3004;
-//; $tls::stacklock = -9692;
-//; $tls::pstacklock = 3008;
-//; $tls::stackptr = -9688;
-//; $tls::pstackptr = 3012;
-//; $tls::stack = -9684;
-//; $tls::pstack = 3016;
-//; $tls::initialized = -8660;
-//; $tls::pinitialized = 4040;
+//; $tls::_ctinfo = -9736;
+//; $tls::p_ctinfo = 2964;
+//; $tls::andreas = -9732;
+//; $tls::pandreas = 2968;
+//; $tls::wq = -9728;
+//; $tls::pwq = 2972;
+//; $tls::sig = -9700;
+//; $tls::psig = 3000;
+//; $tls::incyg = -9696;
+//; $tls::pincyg = 3004;
+//; $tls::spinning = -9692;
+//; $tls::pspinning = 3008;
+//; $tls::stacklock = -9688;
+//; $tls::pstacklock = 3012;
+//; $tls::stackptr = -9684;
+//; $tls::pstackptr = 3016;
+//; $tls::stack = -9680;
+//; $tls::pstack = 3020;
+//; $tls::initialized = -8656;
+//; $tls::pinitialized = 4044;
 //; __DATA__
 
 #define tls_func (-12700)
@@ -95,23 +95,23 @@
 #define tls_p__dontuse (412)
 #define tls_locals (-11200)
 #define tls_plocals (1500)
-#define tls__ctinfo (-9740)
-#define tls_p_ctinfo (2960)
-#define tls_andreas (-9736)
-#define tls_pandreas (2964)
-#define tls_wq (-9732)
-#define tls_pwq (2968)
-#define tls_sig (-9704)
-#define tls_psig (2996)
-#define tls_incyg (-9700)
-#define tls_pincyg (3000)
-#define tls_spinning (-9696)
-#define tls_pspinning (3004)
-#define tls_stacklock (-9692)
-#define tls_pstacklock (3008)
-#define tls_stackptr (-9688)
-#define tls_pstackptr (3012)
-#define tls_stack (-9684)
-#define tls_pstack (3016)
-#define tls_initialized (-8660)
-#define tls_pinitialized (4040)
+#define tls__ctinfo (-9736)
+#define tls_p_ctinfo (2964)
+#define tls_andreas (-9732)
+#define tls_pandreas (2968)
+#define tls_wq (-9728)
+#define tls_pwq (2972)
+#define tls_sig (-9700)
+#define tls_psig (3000)
+#define tls_incyg (-9696)
+#define tls_pincyg (3004)
+#define tls_spinning (-9692)
+#define tls_pspinning (3008)
+#define tls_stacklock (-9688)
+#define tls_pstacklock (3012)
+#define tls_stackptr (-9684)
+#define tls_pstackptr (3016)
+#define tls_stack (-9680)
+#define tls_pstack (3020)
+#define tls_initialized (-8656)
+#define tls_pinitialized (4044)
Index: wait.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/wait.cc,v
retrieving revision 1.37
diff -u -p -r1.37 wait.cc
--- wait.cc	18 Jul 2009 20:25:07 -0000	1.37
+++ wait.cc	29 Jul 2011 05:56:32 -0000
@@ -78,7 +78,7 @@ wait4 (int intpid, int *status, int opti
       if ((waitfor = w->ev) == NULL)
 	goto nochildren;
 
-      res = cancelable_wait (waitfor, INFINITE);
+      res = cancelable_wait (waitfor);
 
       sigproc_printf ("%d = WaitForSingleObject (...)", res);
 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] clock_nanosleep(2)
  2011-08-03  9:20               ` Yaakov (Cygwin/X)
@ 2011-08-03  9:28                 ` Corinna Vinschen
  2011-08-03  9:35                   ` Yaakov (Cygwin/X)
  0 siblings, 1 reply; 28+ messages in thread
From: Corinna Vinschen @ 2011-08-03  9:28 UTC (permalink / raw)
  To: cygwin-patches

On Aug  3 04:19, Yaakov (Cygwin/X) wrote:
> On Wed, 2011-08-03 at 09:45 +0200, Corinna Vinschen wrote:
> > On Aug  3 01:20, Yaakov (Cygwin/X) wrote:
> > > On Tue, 2011-08-02 at 17:42 +0200, Corinna Vinschen wrote:
> > > > Does that mean the return value from NtQueryTimer is unreliable?
> > > > In what way is it wrong?  
> > > 
> > > I'm not sure.  When I run an STC (attached), it works as expected.  In
> > > cancelable_wait(), however, it returns the negative system uptime.  Is
> > > Cygwin doing something to make this occur?
> > 
> > That sounds weird.  How should Cygwin influence what an independent OS
> > function returns?  And you sure it's the system uptime?  Wow.
> 
> Never mind, I figured it out.  The difference is the timeout to
> WaitFor*Object*(); my STC doesn't allow the timer to finish, but
> cancelable_wait() does with the INFINITE timeout.  If there is time
> remaining, as in the STC, then TIMER_BASIC_INFORMATION.TimeRemaining
> contains just that (as a positive).  If the timer has signalled, then
> instead of zero, it appears to provide when it was signalled (system
> uptime, as a negative).

This is cool.  Does it match the tickcount as returned by
hires_ms::timeGetTime_ns()?  If so, it sounds like the return value from
NtQueryTimer *after* the NtCancelTimer call would be usable and probably
more reliable than calling NtQueryTimer first, then NtCancelTimer.

What do you think?


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] clock_nanosleep(2)
  2011-08-03  9:28                 ` Corinna Vinschen
@ 2011-08-03  9:35                   ` Yaakov (Cygwin/X)
  2011-08-03 10:20                     ` Corinna Vinschen
  0 siblings, 1 reply; 28+ messages in thread
From: Yaakov (Cygwin/X) @ 2011-08-03  9:35 UTC (permalink / raw)
  To: cygwin-patches

On Wed, 2011-08-03 at 11:27 +0200, Corinna Vinschen wrote:
> On Aug  3 04:19, Yaakov (Cygwin/X) wrote:
> > Never mind, I figured it out.  The difference is the timeout to
> > WaitFor*Object*(); my STC doesn't allow the timer to finish, but
> > cancelable_wait() does with the INFINITE timeout.  If there is time
> > remaining, as in the STC, then TIMER_BASIC_INFORMATION.TimeRemaining
> > contains just that (as a positive).  If the timer has signalled, then
> > instead of zero, it appears to provide when it was signalled (system
> > uptime, as a negative).
> 
> This is cool.  Does it match the tickcount as returned by
> hires_ms::timeGetTime_ns()?  If so, it sounds like the return value from
> NtQueryTimer *after* the NtCancelTimer call would be usable and probably
> more reliable than calling NtQueryTimer first, then NtCancelTimer.
> 
> What do you think?

The only thing that uses the remaining time is nanosleep(), which uses a
relative timeout.  Same thing will go for clock_nanosleep(): per POSIX,
rmtp is only returned if TIMER_ABSTIME is not set.  If we only care
about relative remainders, then calling NtQueryTimer first is the
simplest way to go, as in my patch.


Yaakov


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] clock_nanosleep(2)
  2011-08-03  9:35                   ` Yaakov (Cygwin/X)
@ 2011-08-03 10:20                     ` Corinna Vinschen
  0 siblings, 0 replies; 28+ messages in thread
From: Corinna Vinschen @ 2011-08-03 10:20 UTC (permalink / raw)
  To: cygwin-patches

On Aug  3 04:35, Yaakov (Cygwin/X) wrote:
> On Wed, 2011-08-03 at 11:27 +0200, Corinna Vinschen wrote:
> > On Aug  3 04:19, Yaakov (Cygwin/X) wrote:
> > > Never mind, I figured it out.  The difference is the timeout to
> > > WaitFor*Object*(); my STC doesn't allow the timer to finish, but
> > > cancelable_wait() does with the INFINITE timeout.  If there is time
> > > remaining, as in the STC, then TIMER_BASIC_INFORMATION.TimeRemaining
> > > contains just that (as a positive).  If the timer has signalled, then
> > > instead of zero, it appears to provide when it was signalled (system
> > > uptime, as a negative).
> > 
> > This is cool.  Does it match the tickcount as returned by
> > hires_ms::timeGetTime_ns()?  If so, it sounds like the return value from
> > NtQueryTimer *after* the NtCancelTimer call would be usable and probably
> > more reliable than calling NtQueryTimer first, then NtCancelTimer.
> > 
> > What do you think?
> 
> The only thing that uses the remaining time is nanosleep(), which uses a
> relative timeout.  Same thing will go for clock_nanosleep(): per POSIX,
> rmtp is only returned if TIMER_ABSTIME is not set.  If we only care
> about relative remainders, then calling NtQueryTimer first is the
> simplest way to go, as in my patch.

Yes, I know.  I was just wondering about the reliability factor of
the value returned by NtQueryTimer.  Using the absolute value after
the call to NtCancelTimer and subtracting the start time may be more
reliable.

But, never mind.  Your patch looks good to me.  Please apply.


Thanks,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2011-08-03 10:20 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-07-20  1:54 [PATCH] clock_nanosleep(2) Yaakov (Cygwin/X)
2011-07-20  7:57 ` Corinna Vinschen
2011-07-20  9:16   ` Yaakov (Cygwin/X)
2011-07-20  9:51     ` Yaakov (Cygwin/X)
2011-07-20 14:12       ` Corinna Vinschen
2011-07-20 22:04         ` [PATCH] clock_nanosleep(2), pthread_condattr_[gs]etclock(3) Yaakov (Cygwin/X)
2011-07-21  2:22           ` Yaakov (Cygwin/X)
2011-07-21  9:21             ` Corinna Vinschen
2011-07-21  9:36               ` Corinna Vinschen
2011-07-21 18:59                 ` Yaakov (Cygwin/X)
2011-07-21 19:09                   ` Corinna Vinschen
2011-07-21 19:15                     ` Corinna Vinschen
2011-07-22  6:34                       ` Yaakov (Cygwin/X)
2011-07-22  8:05                         ` Corinna Vinschen
2011-07-21  7:54           ` Corinna Vinschen
2011-07-20 17:37   ` [PATCH] clock_nanosleep(2) Christopher Faylor
2011-07-21 10:39 ` Corinna Vinschen
2011-07-21 18:51   ` Yaakov (Cygwin/X)
2011-07-21 19:04     ` Corinna Vinschen
2011-07-31  8:25     ` Corinna Vinschen
2011-08-02  4:09       ` Yaakov (Cygwin/X)
2011-08-02 15:43         ` Corinna Vinschen
2011-08-03  6:20           ` Yaakov (Cygwin/X)
2011-08-03  7:45             ` Corinna Vinschen
2011-08-03  9:20               ` Yaakov (Cygwin/X)
2011-08-03  9:28                 ` Corinna Vinschen
2011-08-03  9:35                   ` Yaakov (Cygwin/X)
2011-08-03 10:20                     ` Corinna Vinschen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).