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

From: Nuno Teixeira <eduardo_at_freebsd.org>
Date: Sat, 01 Jun 2024 08:37:00 UTC
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.

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