Re: git: 108de784513d - main - Redefine CLOCK_BOOTTIME to alias CLOCK_MONOTONIC, not CLOCK_UPTIME

On Sat, Jun 1, 2024, 4:37 AM Nuno Teixeira <eduardo@freebsd.org> wrote:

> Hello,
>
> Having issues building graphics/mesa-dri:
>
> ../src/intel/common/xe/intel_gem.c:72:9: error: duplicate case value '4'
>    72 |    case CLOCK_BOOTTIME:
>       |         ^
> /usr/include/sys/_clock_id.h:81:25: note: expanded from macro
> 'CLOCK_BOOTTIME'
>    81 | #define CLOCK_BOOTTIME          CLOCK_MONOTONIC
>       |                                 ^
> /usr/include/sys/_clock_id.h:56:26: note: expanded from macro
> 'CLOCK_MONOTONIC'
>    56 | #define CLOCK_MONOTONIC         4
>       |                                 ^
> ../src/intel/common/xe/intel_gem.c:66:9: note: previous case defined here
>    66 |    case CLOCK_MONOTONIC:
>       |         ^
> /usr/include/sys/_clock_id.h:56:26: note: expanded from macro
> 'CLOCK_MONOTONIC'
>    56 | #define CLOCK_MONOTONIC         4
>       |                                 ^
> 1 error generated.
>

Mesa needs to be updated to not assume these are distinct values.

Warner


> Thanks
>
> Warner Losh <imp@freebsd.org> escreveu (sexta, 31/05/2024 à(s) 15:47):
>
>> The branch main has been updated by imp:
>>
>> URL:
>> https://cgit.FreeBSD.org/src/commit/?id=108de784513d87bbe850e7b003a73e26b5b54caa
>>
>> commit 108de784513d87bbe850e7b003a73e26b5b54caa
>> Author:     Val Packett <val@packett.cool>
>> AuthorDate: 2024-05-31 14:45:02 +0000
>> Commit:     Warner Losh <imp@FreeBSD.org>
>> CommitDate: 2024-05-31 14:45:02 +0000
>>
>>     Redefine CLOCK_BOOTTIME to alias CLOCK_MONOTONIC, not CLOCK_UPTIME
>>
>>     The suspend-awareness situation with monotonic clocks across platforms
>>     is kind of a mess, let's try not making it worse.
>>
>>     On Linux, CLOCK_MONOTONIC does NOT count suspended time, and
>>     CLOCK_BOOTTIME was introduced to INCLUDE suspended time.
>>
>>     On OpenBSD, CLOCK_MONOTONIC DOES count suspended time, and
>> CLOCK_UPTIME
>>     was introduced to EXCLUDE suspended time.
>>
>>     On macOS, it's the same as OpenBSD, but with CLOCK_UPTIME_RAW.
>>
>>     Right now, we do not have a monotonic clock that counts suspended
>> time.
>>     We have CLOCK_UPTIME as a distinct ID alias, and CLOCK_BOOTTIME as a
>>     preprocessor alias, both being effectively `CLOCK_MONOTONIC` for now.
>>
>>     When we introduce a suspend-aware clock in the future, it would make a
>>     lot more sense to do it the OpenBSD/macOS way, i.e. to make
>>     CLOCK_MONOTONIC include suspended time and make CLOCK_UPTIME exclude
>> it,
>>     because that's what the name CLOCK_UPTIME implies: a deviation from
>> the
>>     default intended for the uptime command to allow it to only show the
>>     time the system was actually up and not suspended.
>>
>>     Let's change the define right now to make sure software using the
>> define
>>     would not end up using the ID of the wrong clock in the future, and
>> fix
>>     the IDs in the Linux compat code to match the expected changes too.
>>
>>     See https://bugzilla.mozilla.org/show_bug.cgi?id=1824084
>>     for more discussion.
>>
>>     Fixes:          155f15118a77 ("clock_gettime: Add Linux aliases for
>> CLOCK_*")
>>     Fixes:          25ada637362d ("Map Linux CLOCK_BOOTTIME to native
>> CLOCK_UPTIME.")
>>     Sponsored by:   https://www.patreon.com/valpackett
>>     Reviewed by:    kib, imp
>>     Differential Revision:  https://reviews.freebsd.org/D39270
>> ---
>>  sys/compat/linux/linux_time.c | 6 +++---
>>  sys/sys/_clock_id.h           | 2 +-
>>  2 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/sys/compat/linux/linux_time.c b/sys/compat/linux/linux_time.c
>> index e9e5cf075210..f4dd26dd3d2a 100644
>> --- a/sys/compat/linux/linux_time.c
>> +++ b/sys/compat/linux/linux_time.c
>> @@ -287,7 +287,7 @@ linux_to_native_clockid(clockid_t *n, clockid_t l)
>>                 *n = CLOCK_REALTIME;
>>                 break;
>>         case LINUX_CLOCK_MONOTONIC:
>> -               *n = CLOCK_MONOTONIC;
>> +               *n = CLOCK_UPTIME;
>>                 break;
>>         case LINUX_CLOCK_PROCESS_CPUTIME_ID:
>>                 *n = CLOCK_PROCESS_CPUTIME_ID;
>> @@ -300,10 +300,10 @@ linux_to_native_clockid(clockid_t *n, clockid_t l)
>>                 break;
>>         case LINUX_CLOCK_MONOTONIC_COARSE:
>>         case LINUX_CLOCK_MONOTONIC_RAW:
>> -               *n = CLOCK_MONOTONIC_FAST;
>> +               *n = CLOCK_UPTIME_FAST;
>>                 break;
>>         case LINUX_CLOCK_BOOTTIME:
>> -               *n = CLOCK_UPTIME;
>> +               *n = CLOCK_MONOTONIC;
>>                 break;
>>         case LINUX_CLOCK_REALTIME_ALARM:
>>         case LINUX_CLOCK_BOOTTIME_ALARM:
>> diff --git a/sys/sys/_clock_id.h b/sys/sys/_clock_id.h
>> index 47a551428dc3..728346a0f0ab 100644
>> --- a/sys/sys/_clock_id.h
>> +++ b/sys/sys/_clock_id.h
>> @@ -78,7 +78,7 @@
>>   * Linux compatible names.
>>   */
>>  #if __BSD_VISIBLE
>> -#define        CLOCK_BOOTTIME          CLOCK_UPTIME
>> +#define        CLOCK_BOOTTIME          CLOCK_MONOTONIC
>>  #define        CLOCK_REALTIME_COARSE   CLOCK_REALTIME_FAST
>>  #define        CLOCK_MONOTONIC_COARSE  CLOCK_MONOTONIC_FAST
>>  #endif
>>
>>
>
> --
> Nuno Teixeira
> FreeBSD UNIX:  <eduardo@FreeBSD.org>   Web:  https://FreeBSD.org
>