Re: [PATCH] clock_nanosleep(2)
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 - 1.19
+++ libc/include/time.h 15 May 2011 19:22:48 -
@@ -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
Re: [PATCH] clock_nanosleep(2)
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
Re: [PATCH] clock_nanosleep(2)
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
Re: [PATCH] clock_nanosleep(2)
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
Re: [PATCH] clock_nanosleep(2)
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
Re: [PATCH] clock_nanosleep(2), pthread_condattr_[gs]etclock(3)
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
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 - 1.60
+++ sysconf.cc 20 Jul 2011 21:33:56 -
@@ -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 */
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 - 1.244
+++ cygwin.din 20 Jul 2011 21:33:53 -
@@ -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 - 1.68
+++ posix.sgml 20 Jul 2011 21:33:56 -
@@ -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[...]
-
Re: [PATCH] clock_nanosleep(2), pthread_condattr_[gs]etclock(3)
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
