[PATCH xserver 3/4] os: Make CLOCK_REALTIME the CLOCK_MONOTONIC fallback

[Jeffrey Smith]

On Wed, Dec 27, 2017 at 3:53 AM, Mark Kettenis <mark.kettenis at xs4all.nl> wrote:
>> From: Jeff Smith <whydoubt at gmail.com>
>> Date: Tue, 26 Dec 2017 22:10:54 -0600
>>
>> When clock_gettime(CLOCK_MONOTONIC) fails, xserver falls back on
>> gettimeofday().  However, gettimeofday() is deprecated in favor of
>> clock_gettime(CLOCK_REALTIME).
>>
>> Fall back on CLOCK_REALTIME if CLOCK_MONOTONIC fails.  As long as
>> clock_gettime() is available, passing CLOCK_REALTIME can be expected to
>> work.  Leave the gettimeofday() implementation available for
>> configurations that do not try to obtain a monotonic clock.
>
> What problem are you trying to fix?  Are there any modern systems
> where CLOCK_MONOTONIC isn't available?  Adding more "legacy" codepaths
> isn't going to improve the quality of the code!  Especially if those codepaths are redundant.

For testing purposes, I want to be able to explicitly use the realtime
clock in a sensible way.

The uninitialized value for clockid was 0, and ~0L was the fallback
(monotonic unavailable) value.
I made ~0L the uninitialized value, and 0 (CLOCK_REALTIME) the fallback value.
With that, once clockid is set, it will be a known good value for
calling clock_gettime.
So clock_gettime can be called without checking for the ~0L fallback value.
With the clock_gettime and gettimeofday codepaths no longer
interwoven, they can be separated.

The "legacy" (gettimeofday) codepath (i.e. when MONOTONIC_CLOCK is not
defined) is
effectively the same after this patch.  It was also being used as a
run-time fallback for the
"modern" (clock_gettime) codepath, and this patch simplifies that
codepath.  This seems
like an improvement regardless of enabling ForceClockId(CLOCK_REALTIME) to work.

>
>
>> diff --git a/os/utils.c b/os/utils.c
>> index 876841c..f3d0f71 100644
>> --- a/os/utils.c
>> +++ b/os/utils.c
>> @@ -200,8 +200,8 @@ sig_atomic_t inSignalContext = FALSE;
>>  #endif
>>
>>  #ifdef MONOTONIC_CLOCK
>> -static clockid_t clockid;
>> -static clockid_t clockid_micro;
>> +static clockid_t clockid = ~0L;
>> +static clockid_t clockid_micro = ~0L;
>>  #endif
>>
>>  OsSigHandlerPtr
>> @@ -427,8 +427,8 @@ ForceClockId(clockid_t forced_clockid)
>>  {
>>      struct timespec tp;
>>
>> -    BUG_RETURN(clockid);
>> -    BUG_RETURN(clockid_micro);
>> +    BUG_RETURN(clockid != ~0L);
>> +    BUG_RETURN(clockid_micro != ~0L);
>>
>>      if (clock_gettime(forced_clockid, &tp) != 0) {
>>          FatalError("Forced clock id failed to retrieve current time: %s\n",
>> @@ -452,16 +452,13 @@ GetTimeInMicros(void)
>>  {
>>      return (CARD64) GetTickCount() * 1000;
>>  }
>> -#else
>> +#elif defined(MONOTONIC_CLOCK)
>>  CARD32
>>  GetTimeInMillis(void)
>>  {
>> -    struct timeval tv;
>> -
>> -#ifdef MONOTONIC_CLOCK
>>      struct timespec tp;
>>
>> -    if (!clockid) {
>> +    if (clockid == ~0L) {
>>  #ifdef CLOCK_MONOTONIC_COARSE
>>          if (clock_getres(CLOCK_MONOTONIC_COARSE, &tp) == 0 &&
>>              (tp.tv_nsec / 1000) <= 1000 &&
>> @@ -472,32 +469,38 @@ GetTimeInMillis(void)
>>          if (clock_gettime(CLOCK_MONOTONIC, &tp) == 0)
>>              clockid = CLOCK_MONOTONIC;
>>          else
>> -            clockid = ~0L;
>> +            clockid = CLOCK_REALTIME;
>>      }
>> -    if (clockid != ~0L && clock_gettime(clockid, &tp) == 0)
>> -        return (tp.tv_sec * 1000) + (tp.tv_nsec / 1000000L);
>> -#endif
>> -
>> -    X_GETTIMEOFDAY(&tv);
>> -    return (tv.tv_sec * 1000) + (tv.tv_usec / 1000);
>> +    clock_gettime(clockid, &tp);
>> +    return (tp.tv_sec * 1000) + (tp.tv_nsec / 1000000L);
>>  }
>> -
>>  CARD64
>>  GetTimeInMicros(void)
>>  {
>> -    struct timeval tv;
>> -#ifdef MONOTONIC_CLOCK
>>      struct timespec tp;
>>
>> -    if (!clockid_micro) {
>> +    if (clockid_micro == ~0L) {
>>          if (clock_gettime(CLOCK_MONOTONIC, &tp) == 0)
>>              clockid_micro = CLOCK_MONOTONIC;
>>          else
>> -            clockid_micro = ~0L;
>> +            clockid_micro = CLOCK_REALTIME;
>>      }
>> -    if (clockid_micro != ~0L && clock_gettime(clockid_micro, &tp) == 0)
>> -        return (CARD64) tp.tv_sec * (CARD64)1000000 + tp.tv_nsec / 1000;
>> -#endif
>> +    clock_gettime(clockid_micro, &tp);
>> +    return (CARD64) tp.tv_sec * (CARD64)1000000 + tp.tv_nsec / 1000;
>> +}
>> +#else
>> +CARD32
>> +GetTimeInMillis(void)
>> +{
>> +    struct timeval tv;
>> +
>> +    X_GETTIMEOFDAY(&tv);
>> +    return (tv.tv_sec * 1000) + (tv.tv_usec / 1000);
>> +}
>> +CARD64
>> +GetTimeInMicros(void)
>> +{
>> +    struct timeval tv;
>>
>>      X_GETTIMEOFDAY(&tv);
>>      return (CARD64) tv.tv_sec * (CARD64)1000000 + (CARD64) tv.tv_usec;
>> --
>> 2.9.4
>>
>> _______________________________________________
>> xorg-devel at lists.x.org: X.Org development
>> Archives: http://lists.x.org/archives/xorg-devel
>> Info: https://lists.x.org/mailman/listinfo/xorg-devel
>>