Yaakov (Cygwin/X | 2 Aug 22:49 2010
Picon
Picon

[PATCH] POSIX monotonic clock

Here is an attempt to implement POSIX.1-2004+ Monotonic Clock:

http://www.opengroup.org/onlinepubs/9699919799/functions/clock_getres.html

In summary, I took hires_us and changed the resolution to nanoseconds. I
dropped systime() because the only place hires_us was being used is in
strace.cc which ignored it, and WRT POSIX monotonic clocks the absolute
value of the clock is meaningless.  Since systime() has only 100ns
precision, using it would either force a loss in resolution or (if
multiplied by 100 to get ns) an early overflow.  I also switched from
ENOSYS to EINVAL, as POSIX.1-2004 and 2008 dropped references to the
former (as noted in Change History).

Patches for newlib, winsup/cygwin and winsup/doc attached.

I have also attached an STC for the new functionality.  FWIW, on my
machine, QueryPerformanceFrequency() returns just over 2.9 million,
resulting in a clock_getres(CLOCK_MONOTONIC) of 340ns.

I would appreciate a careful review of this patch, both from the Cygwin
API and POSIX POVs.

Yaakov

Attachment (clock-test.c): text/x-csrc, 776 bytes
Christopher Faylor | 3 Aug 02:37 2010

Re: [PATCH] POSIX monotonic clock

On Mon, Aug 02, 2010 at 03:49:08PM -0500, Yaakov (Cygwin/X) wrote:
>Here is an attempt to implement POSIX.1-2004+ Monotonic Clock:
>
>http://www.opengroup.org/onlinepubs/9699919799/functions/clock_getres.html
>
>In summary, I took hires_us and changed the resolution to nanoseconds. I
>dropped systime() because the only place hires_us was being used is in
>strace.cc which ignored it, and WRT POSIX monotonic clocks the absolute
>value of the clock is meaningless.  Since systime() has only 100ns
>precision, using it would either force a loss in resolution or (if
>multiplied by 100 to get ns) an early overflow.  I also switched from
>ENOSYS to EINVAL, as POSIX.1-2004 and 2008 dropped references to the
>former (as noted in Change History).

But that changes the sense of the errno.  EINVAL would indicate an unsupported
argument.  At least some of the cases fail because of an unimplemented NT
function call, e.g., ENOSYS.  Regardless of what POSIX says, I think that ENOSYS
is ok, in at least some cases, for Cygwin since I believe QueryPerformanceCounter
doesn't work on NT3.5 and NT4.

Other than that this looks ok.

cgf

Yaakov (Cygwin/X | 3 Aug 07:52 2010
Picon
Picon

Re: [PATCH] POSIX monotonic clock

On Mon, 2010-08-02 at 20:37 -0400, Christopher Faylor wrote:
> But that changes the sense of the errno.  EINVAL would indicate an unsupported
> argument.  At least some of the cases fail because of an unimplemented NT
> function call, e.g., ENOSYS.  Regardless of what POSIX says, I think that ENOSYS
> is ok, in at least some cases, for Cygwin since I believe QueryPerformanceCounter
> doesn't work on NT3.5 and NT4.

Thanks for the review.  This patch uses ENOSYS for QueryPerformance*
failures and EINVAL for unsupported/illegal CLOCK_* arguments.  If this
is OK, then I'll proceed with the newlib patch, which needs to be
applied first.

Yaakov

Václav Haisman | 3 Aug 09:32 2010
Picon
Picon

Re: [PATCH] POSIX monotonic clock

On Mon, 02 Aug 2010 15:49:08 -0500, "Yaakov (Cygwin/X)" wrote:
> Here is an attempt to implement POSIX.1-2004+ Monotonic Clock:
> 
>
http://www.opengroup.org/onlinepubs/9699919799/functions/clock_getres.html
> 
> In summary, I took hires_us and changed the resolution to nanoseconds. I
> dropped systime() because the only place hires_us was being used is in
> strace.cc which ignored it, and WRT POSIX monotonic clocks the absolute
> value of the clock is meaningless.  Since systime() has only 100ns
> precision, using it would either force a loss in resolution or (if
> multiplied by 100 to get ns) an early overflow.  I also switched from
> ENOSYS to EINVAL, as POSIX.1-2004 and 2008 dropped references to the
> former (as noted in Change History).
> 
> Patches for newlib, winsup/cygwin and winsup/doc attached.
> 
> I have also attached an STC for the new functionality.  FWIW, on my
> machine, QueryPerformanceFrequency() returns just over 2.9 million,
> resulting in a clock_getres(CLOCK_MONOTONIC) of 340ns.
> 
> I would appreciate a careful review of this patch, both from the Cygwin
> API and POSIX POVs.
Is it really ok to use QueryPerformanceCounter() to implement this? Quote
from <http://msdn.microsoft.com/en-us/library/ms644904%28VS.85%29.aspx>:

"On a multiprocessor computer, it should not matter which processor is
called. However, you can get different results on different processors due
to bugs in the basic input/output system (BIOS) or the hardware abstraction
layer (HAL). To specify processor affinity for a thread, use the
(Continue reading)

Václav Haisman | 3 Aug 10:10 2010
Picon
Picon

Re: [PATCH] POSIX monotonic clock

On Tue, 03 Aug 2010 09:32:47 +0200, Václav Haisman
wrote:
> On Mon, 02 Aug 2010 15:49:08 -0500, "Yaakov (Cygwin/X)" wrote:
>> Here is an attempt to implement POSIX.1-2004+ Monotonic Clock:
>> 
>>
>
http://www.opengroup.org/onlinepubs/9699919799/functions/clock_getres.html
>> 
>> In summary, I took hires_us and changed the resolution to nanoseconds.
I
>> dropped systime() because the only place hires_us was being used is in
>> strace.cc which ignored it, and WRT POSIX monotonic clocks the absolute
>> value of the clock is meaningless.  Since systime() has only 100ns
>> precision, using it would either force a loss in resolution or (if
>> multiplied by 100 to get ns) an early overflow.  I also switched from
>> ENOSYS to EINVAL, as POSIX.1-2004 and 2008 dropped references to the
>> former (as noted in Change History).
>> 
>> Patches for newlib, winsup/cygwin and winsup/doc attached.
>> 
>> I have also attached an STC for the new functionality.  FWIW, on my
>> machine, QueryPerformanceFrequency() returns just over 2.9 million,
>> resulting in a clock_getres(CLOCK_MONOTONIC) of 340ns.
>> 
>> I would appreciate a careful review of this patch, both from the Cygwin
>> API and POSIX POVs.
> Is it really ok to use QueryPerformanceCounter() to implement this?
Quote
> from <http://msdn.microsoft.com/en-us/library/ms644904%28VS.85%29.aspx>:
(Continue reading)

Christopher Faylor | 3 Aug 16:04 2010

Re: [PATCH] POSIX monotonic clock

On Tue, Aug 03, 2010 at 09:32:47AM +0200, V??clav Haisman wrote:
>This looks like you could get monotonic clock going backwards.

That's a good point.  We have that very problem here where I work.
However, Yaakov isn't adding anything new here so, if this is a problem,
it would be a long-standing one.

It sounds like it would be trivially solvable by setting the processor
affinity mask but I'm not sure what that would mean for performance.

cgf

Yaakov (Cygwin/X | 8 Aug 07:06 2010
Picon
Picon

Re: [PATCH] POSIX monotonic clock

On Tue, 2010-08-03 at 10:04 -0400, Christopher Faylor wrote:
> On Tue, Aug 03, 2010 at 09:32:47AM +0200, V??clav Haisman wrote:
> >This looks like you could get monotonic clock going backwards.
> 
> That's a good point.  We have that very problem here where I work.
> However, Yaakov isn't adding anything new here so, if this is a problem,
> it would be a long-standing one.
> 
> It sounds like it would be trivially solvable by setting the processor
> affinity mask but I'm not sure what that would mean for performance.

So should I hold off on my patch until this can be fixed?

Yaakov

Yaakov (Cygwin/X | 8 Aug 07:49 2010
Picon
Picon

[PATCH] define RTLD_LOCAL

POSIX requires RTLD_LOCAL to be defined in <dlfcn.h>[1].  While our
dlopen() does nothing with its second argument, portable software can
rightfully expect the definition to exist alongside the other RTLD_*
macros.

So why 0?  POSIX states wrt dlopen()[2]:

> If neither RTLD_GLOBAL nor RTLD_LOCAL are specified, then the default
> behavior is unspecified.

On Linux, RTLD_LOCAL is the default behaviour[3], and hence is defined
as 0[4], therefore I have done the same here.

Patch attached.  Since this doesn't actually do anything, I wasn't sure
if I should bump CYGWIN_VERSION_API_MINOR for this or not; I can
certainly do so before committing if desired.

Yaakov

[1] http://www.opengroup.org/onlinepubs/9699919799/basedefs/dlfcn.h.html
[2] http://www.opengroup.org/onlinepubs/9699919799/functions/dlopen.html
[3] http://linux.die.net/man/3/dlopen
[4] http://sourceware.org/git/?p=glibc.git;a=blob;f=bits/dlfcn.h

Attachment (winsup-RTLD_LOCAL.patch): text/x-patch, 800 bytes
Václav Haisman | 8 Aug 10:35 2010
Picon
Picon

Re: [PATCH] define RTLD_LOCAL


Yaakov (Cygwin/X) wrote, On 8.8.2010 7:49:
> POSIX requires RTLD_LOCAL to be defined in <dlfcn.h>[1].  While our
> dlopen() does nothing with its second argument, portable software can
> rightfully expect the definition to exist alongside the other RTLD_*
> macros.
> 
> So why 0?  POSIX states wrt dlopen()[2]:
> 
>> If neither RTLD_GLOBAL nor RTLD_LOCAL are specified, then the default
>> behavior is unspecified.
> 
> On Linux, RTLD_LOCAL is the default behaviour[3], and hence is defined
> as 0[4], therefore I have done the same here.
> 
> Patch attached.  Since this doesn't actually do anything, I wasn't sure
> if I should bump CYGWIN_VERSION_API_MINOR for this or not; I can
> certainly do so before committing if desired.
Is it not undefined in Cygwin because Windows cannot support the behaviour?
AFAIK once you do LoadLibrary(A.dll) then any subsequent reference to A.dll
and its exports will be satisfied from the already loaded A.dll. IOW, Windows
cannot satisfy "The object's symbols shall not be made available for the
relocation processing of any other object," as specified by [2].

> [1] http://www.opengroup.org/onlinepubs/9699919799/basedefs/dlfcn.h.html
> [2] http://www.opengroup.org/onlinepubs/9699919799/functions/dlopen.html
> [3] http://linux.die.net/man/3/dlopen
> [4] http://sourceware.org/git/?p=glibc.git;a=blob;f=bits/dlfcn.h

--

-- 
(Continue reading)

Christopher Faylor | 8 Aug 18:33 2010

Re: [PATCH] POSIX monotonic clock

On Sun, Aug 08, 2010 at 12:06:18AM -0500, Yaakov (Cygwin/X) wrote:
>On Tue, 2010-08-03 at 10:04 -0400, Christopher Faylor wrote:
>> On Tue, Aug 03, 2010 at 09:32:47AM +0200, V??clav Haisman wrote:
>> >This looks like you could get monotonic clock going backwards.
>> 
>> That's a good point.  We have that very problem here where I work.
>> However, Yaakov isn't adding anything new here so, if this is a problem,
>> it would be a long-standing one.
>> 
>> It sounds like it would be trivially solvable by setting the processor
>> affinity mask but I'm not sure what that would mean for performance.
>
>So should I hold off on my patch until this can be fixed?

Nah.  Go ahead.  Thanks.

cgf


Gmane