Date: Sat, 1 Jun 2024 09:37:00 +0100 From: Nuno Teixeira <eduardo@freebsd.org> To: Warner Losh <imp@freebsd.org> Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: 108de784513d - main - Redefine CLOCK_BOOTTIME to alias CLOCK_MONOTONIC, not CLOCK_UPTIME Message-ID: <CAFDf7ULP1X_LgDOrKhMEFF7AgBQPPaJGhbRsaNcUpUOBo9JN9g@mail.gmail.com> In-Reply-To: <202405311447.44VEl1G1078199@gitrepo.freebsd.org> References: <202405311447.44VEl1G1078199@gitrepo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --]
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
[-- Attachment #2 --]
<div dir="ltr"><div>Hello,</div><div><br></div><div>Having issues building graphics/mesa-dri:</div><div><br></div><div>../src/intel/common/xe/intel_gem.c:72:9: error: duplicate case value '4'<br> 72 | case CLOCK_BOOTTIME:<br> | ^<br>/usr/include/sys/_clock_id.h:81:25: note: expanded from macro 'CLOCK_BOOTTIME'<br> 81 | #define CLOCK_BOOTTIME CLOCK_MONOTONIC<br> | ^<br>/usr/include/sys/_clock_id.h:56:26: note: expanded from macro 'CLOCK_MONOTONIC'<br> 56 | #define CLOCK_MONOTONIC 4<br> | ^<br>../src/intel/common/xe/intel_gem.c:66:9: note: previous case defined here<br> 66 | case CLOCK_MONOTONIC:<br> | ^<br>/usr/include/sys/_clock_id.h:56:26: note: expanded from macro 'CLOCK_MONOTONIC'<br> 56 | #define CLOCK_MONOTONIC 4<br> | ^<br>1 error generated.</div><div><font color="#888888"><br></font></div><div><font color="#888888">Thanks<br clear="all"></font></div><div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">Warner Losh <<a href="mailto:imp@freebsd.org">imp@freebsd.org</a>> escreveu (sexta, 31/05/2024 à(s) 15:47):<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">The branch main has been updated by imp:<br>
<br>
URL: <a href="https://cgit.FreeBSD.org/src/commit/?id=108de784513d87bbe850e7b003a73e26b5b54caa" rel="noreferrer" target="_blank">https://cgit.FreeBSD.org/src/commit/?id=108de784513d87bbe850e7b003a73e26b5b54caa</a><br>;
<br>
commit 108de784513d87bbe850e7b003a73e26b5b54caa<br>
Author: Val Packett <val@packett.cool><br>
AuthorDate: 2024-05-31 14:45:02 +0000<br>
Commit: Warner Losh <imp@FreeBSD.org><br>
CommitDate: 2024-05-31 14:45:02 +0000<br>
<br>
Redefine CLOCK_BOOTTIME to alias CLOCK_MONOTONIC, not CLOCK_UPTIME<br>
<br>
The suspend-awareness situation with monotonic clocks across platforms<br>
is kind of a mess, let's try not making it worse.<br>
<br>
On Linux, CLOCK_MONOTONIC does NOT count suspended time, and<br>
CLOCK_BOOTTIME was introduced to INCLUDE suspended time.<br>
<br>
On OpenBSD, CLOCK_MONOTONIC DOES count suspended time, and CLOCK_UPTIME<br>
was introduced to EXCLUDE suspended time.<br>
<br>
On macOS, it's the same as OpenBSD, but with CLOCK_UPTIME_RAW.<br>
<br>
Right now, we do not have a monotonic clock that counts suspended time.<br>
We have CLOCK_UPTIME as a distinct ID alias, and CLOCK_BOOTTIME as a<br>
preprocessor alias, both being effectively `CLOCK_MONOTONIC` for now.<br>
<br>
When we introduce a suspend-aware clock in the future, it would make a<br>
lot more sense to do it the OpenBSD/macOS way, i.e. to make<br>
CLOCK_MONOTONIC include suspended time and make CLOCK_UPTIME exclude it,<br>
because that's what the name CLOCK_UPTIME implies: a deviation from the<br>
default intended for the uptime command to allow it to only show the<br>
time the system was actually up and not suspended.<br>
<br>
Let's change the define right now to make sure software using the define<br>
would not end up using the ID of the wrong clock in the future, and fix<br>
the IDs in the Linux compat code to match the expected changes too.<br>
<br>
See <a href="https://bugzilla.mozilla.org/show_bug.cgi?id=1824084" rel="noreferrer" target="_blank">https://bugzilla.mozilla.org/show_bug.cgi?id=1824084</a><br>;
for more discussion.<br>
<br>
Fixes: 155f15118a77 ("clock_gettime: Add Linux aliases for CLOCK_*")<br>
Fixes: 25ada637362d ("Map Linux CLOCK_BOOTTIME to native CLOCK_UPTIME.")<br>
Sponsored by: <a href="https://www.patreon.com/valpackett" rel="noreferrer" target="_blank">https://www.patreon.com/valpackett</a><br>;
Reviewed by: kib, imp<br>
Differential Revision: <a href="https://reviews.freebsd.org/D39270" rel="noreferrer" target="_blank">https://reviews.freebsd.org/D39270</a><br>;
---<br>
sys/compat/linux/linux_time.c | 6 +++---<br>
sys/sys/_clock_id.h | 2 +-<br>
2 files changed, 4 insertions(+), 4 deletions(-)<br>
<br>
diff --git a/sys/compat/linux/linux_time.c b/sys/compat/linux/linux_time.c<br>
index e9e5cf075210..f4dd26dd3d2a 100644<br>
--- a/sys/compat/linux/linux_time.c<br>
+++ b/sys/compat/linux/linux_time.c<br>
@@ -287,7 +287,7 @@ linux_to_native_clockid(clockid_t *n, clockid_t l)<br>
*n = CLOCK_REALTIME;<br>
break;<br>
case LINUX_CLOCK_MONOTONIC:<br>
- *n = CLOCK_MONOTONIC;<br>
+ *n = CLOCK_UPTIME;<br>
break;<br>
case LINUX_CLOCK_PROCESS_CPUTIME_ID:<br>
*n = CLOCK_PROCESS_CPUTIME_ID;<br>
@@ -300,10 +300,10 @@ linux_to_native_clockid(clockid_t *n, clockid_t l)<br>
break;<br>
case LINUX_CLOCK_MONOTONIC_COARSE:<br>
case LINUX_CLOCK_MONOTONIC_RAW:<br>
- *n = CLOCK_MONOTONIC_FAST;<br>
+ *n = CLOCK_UPTIME_FAST;<br>
break;<br>
case LINUX_CLOCK_BOOTTIME:<br>
- *n = CLOCK_UPTIME;<br>
+ *n = CLOCK_MONOTONIC;<br>
break;<br>
case LINUX_CLOCK_REALTIME_ALARM:<br>
case LINUX_CLOCK_BOOTTIME_ALARM:<br>
diff --git a/sys/sys/_clock_id.h b/sys/sys/_clock_id.h<br>
index 47a551428dc3..728346a0f0ab 100644<br>
--- a/sys/sys/_clock_id.h<br>
+++ b/sys/sys/_clock_id.h<br>
@@ -78,7 +78,7 @@<br>
* Linux compatible names.<br>
*/<br>
#if __BSD_VISIBLE<br>
-#define CLOCK_BOOTTIME CLOCK_UPTIME<br>
+#define CLOCK_BOOTTIME CLOCK_MONOTONIC<br>
#define CLOCK_REALTIME_COARSE CLOCK_REALTIME_FAST<br>
#define CLOCK_MONOTONIC_COARSE CLOCK_MONOTONIC_FAST<br>
#endif<br>
<br>
</blockquote></div><br clear="all"><br><span class="gmail_signature_prefix">-- </span><br><div dir="ltr" class="gmail_signature"><div dir="ltr"><div><font color="#888888">Nuno Teixeira</font></div><div><div><font color="#888888">
FreeBSD UNIX: <eduardo@FreeBSD.org> Web: <a href="https://FreeBSD.org" rel="noreferrer" target="_blank">https://FreeBSD.org</a><br></font></div></div></div></div>;
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAFDf7ULP1X_LgDOrKhMEFF7AgBQPPaJGhbRsaNcUpUOBo9JN9g>