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
--000000000000a7274a0619d0017c Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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 =C3=A0(s) 15:47): > The branch main has been updated by imp: > > URL: > https://cgit.FreeBSD.org/src/commit/?id=3D108de784513d87bbe850e7b003a73e2= 6b5b54caa > > 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 platform= s > 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_UPTI= ME > 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 tim= e. > 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 t= he > 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 f= ix > the IDs in the Linux compat code to match the expected changes too. > > See https://bugzilla.mozilla.org/show_bug.cgi?id=3D1824084 > 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 =3D CLOCK_REALTIME; > break; > case LINUX_CLOCK_MONOTONIC: > - *n =3D CLOCK_MONOTONIC; > + *n =3D CLOCK_UPTIME; > break; > case LINUX_CLOCK_PROCESS_CPUTIME_ID: > *n =3D 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 =3D CLOCK_MONOTONIC_FAST; > + *n =3D CLOCK_UPTIME_FAST; > break; > case LINUX_CLOCK_BOOTTIME: > - *n =3D CLOCK_UPTIME; > + *n =3D 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 > > --=20 Nuno Teixeira FreeBSD UNIX: <eduardo@FreeBSD.org> Web: https://FreeBSD.org --000000000000a7274a0619d0017c Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable <div dir=3D"ltr"><div>Hello,</div><div><br></div><div>Having issues buildin= g graphics/mesa-dri:</div><div><br></div><div>../src/intel/common/xe/intel_= gem.c:72:9: error: duplicate case value '4'<br>=C2=A0 =C2=A072 | = =C2=A0 =C2=A0case CLOCK_BOOTTIME:<br>=C2=A0 =C2=A0 =C2=A0 | =C2=A0 =C2=A0 = =C2=A0 =C2=A0 ^<br>/usr/include/sys/_clock_id.h:81:25: note: expanded from = macro 'CLOCK_BOOTTIME'<br>=C2=A0 =C2=A081 | #define CLOCK_BOOTTIME = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0CLOCK_MONOTONIC<br>=C2=A0 =C2=A0 =C2=A0 |= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ^<br>/usr/include/sys/_clock_id.h:56= :26: note: expanded from macro 'CLOCK_MONOTONIC'<br>=C2=A0 =C2=A056= | #define CLOCK_MONOTONIC =C2=A0 =C2=A0 =C2=A0 =C2=A0 4<br>=C2=A0 =C2=A0 = =C2=A0 | =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ^<br>../src/intel/common/xe/i= ntel_gem.c:66:9: note: previous case defined here<br>=C2=A0 =C2=A066 | =C2= =A0 =C2=A0case CLOCK_MONOTONIC:<br>=C2=A0 =C2=A0 =C2=A0 | =C2=A0 =C2=A0 =C2= =A0 =C2=A0 ^<br>/usr/include/sys/_clock_id.h:56:26: note: expanded from mac= ro 'CLOCK_MONOTONIC'<br>=C2=A0 =C2=A056 | #define CLOCK_MONOTONIC = =C2=A0 =C2=A0 =C2=A0 =C2=A0 4<br>=C2=A0 =C2=A0 =C2=A0 | =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 ^<br>1 error generated.</div><div><font color=3D"#8888= 88"><br></font></div><div><font color=3D"#888888">Thanks<br clear=3D"all"><= /font></div><div></div></div><br><div class=3D"gmail_quote"><div dir=3D"ltr= " class=3D"gmail_attr">Warner Losh <<a href=3D"mailto:imp@freebsd.org">i= mp@freebsd.org</a>> escreveu (sexta, 31/05/2024 =C3=A0(s) 15:47):<br></d= iv><blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;bord= er-left:1px solid rgb(204,204,204);padding-left:1ex">The branch main has be= en updated by imp:<br> <br> URL: <a href=3D"https://cgit.FreeBSD.org/src/commit/?id=3D108de784513d87bbe= 850e7b003a73e26b5b54caa" rel=3D"noreferrer" target=3D"_blank">https://cgit.= FreeBSD.org/src/commit/?id=3D108de784513d87bbe850e7b003a73e26b5b54caa</a><b= r> <br> commit 108de784513d87bbe850e7b003a73e26b5b54caa<br> Author:=C2=A0 =C2=A0 =C2=A0Val Packett <val@packett.cool><br> AuthorDate: 2024-05-31 14:45:02 +0000<br> Commit:=C2=A0 =C2=A0 =C2=A0Warner Losh <imp@FreeBSD.org><br> CommitDate: 2024-05-31 14:45:02 +0000<br> <br> =C2=A0 =C2=A0 Redefine CLOCK_BOOTTIME to alias CLOCK_MONOTONIC, not CLOCK_U= PTIME<br> <br> =C2=A0 =C2=A0 The suspend-awareness situation with monotonic clocks across = platforms<br> =C2=A0 =C2=A0 is kind of a mess, let's try not making it worse.<br> <br> =C2=A0 =C2=A0 On Linux, CLOCK_MONOTONIC does NOT count suspended time, and<= br> =C2=A0 =C2=A0 CLOCK_BOOTTIME was introduced to INCLUDE suspended time.<br> <br> =C2=A0 =C2=A0 On OpenBSD, CLOCK_MONOTONIC DOES count suspended time, and CL= OCK_UPTIME<br> =C2=A0 =C2=A0 was introduced to EXCLUDE suspended time.<br> <br> =C2=A0 =C2=A0 On macOS, it's the same as OpenBSD, but with CLOCK_UPTIME= _RAW.<br> <br> =C2=A0 =C2=A0 Right now, we do not have a monotonic clock that counts suspe= nded time.<br> =C2=A0 =C2=A0 We have CLOCK_UPTIME as a distinct ID alias, and CLOCK_BOOTTI= ME as a<br> =C2=A0 =C2=A0 preprocessor alias, both being effectively `CLOCK_MONOTONIC` = for now.<br> <br> =C2=A0 =C2=A0 When we introduce a suspend-aware clock in the future, it wou= ld make a<br> =C2=A0 =C2=A0 lot more sense to do it the OpenBSD/macOS way, i.e. to make<b= r> =C2=A0 =C2=A0 CLOCK_MONOTONIC include suspended time and make CLOCK_UPTIME = exclude it,<br> =C2=A0 =C2=A0 because that's what the name CLOCK_UPTIME implies: a devi= ation from the<br> =C2=A0 =C2=A0 default intended for the uptime command to allow it to only s= how the<br> =C2=A0 =C2=A0 time the system was actually up and not suspended.<br> <br> =C2=A0 =C2=A0 Let's change the define right now to make sure software u= sing the define<br> =C2=A0 =C2=A0 would not end up using the ID of the wrong clock in the futur= e, and fix<br> =C2=A0 =C2=A0 the IDs in the Linux compat code to match the expected change= s too.<br> <br> =C2=A0 =C2=A0 See <a href=3D"https://bugzilla.mozilla.org/show_bug.cgi?id= =3D1824084" rel=3D"noreferrer" target=3D"_blank">https://bugzilla.mozilla.o= rg/show_bug.cgi?id=3D1824084</a><br> =C2=A0 =C2=A0 for more discussion.<br> <br> =C2=A0 =C2=A0 Fixes:=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 155f15118a77 ("= clock_gettime: Add Linux aliases for CLOCK_*")<br> =C2=A0 =C2=A0 Fixes:=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 25ada637362d ("= Map Linux CLOCK_BOOTTIME to native CLOCK_UPTIME.")<br> =C2=A0 =C2=A0 Sponsored by:=C2=A0 =C2=A0<a href=3D"https://www.patreon.com/= valpackett" rel=3D"noreferrer" target=3D"_blank">https://www.patreon.com/va= lpackett</a><br> =C2=A0 =C2=A0 Reviewed by:=C2=A0 =C2=A0 kib, imp<br> =C2=A0 =C2=A0 Differential Revision:=C2=A0 <a href=3D"https://reviews.freeb= sd.org/D39270" rel=3D"noreferrer" target=3D"_blank">https://reviews.freebsd= .org/D39270</a><br> ---<br> =C2=A0sys/compat/linux/linux_time.c | 6 +++---<br> =C2=A0sys/sys/_clock_id.h=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0| 2 +-<br= > =C2=A02 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> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 *n =3D CLOCK_REALTI= ME;<br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 break;<br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 case LINUX_CLOCK_MONOTONIC:<br> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0*n =3D CLOCK_MONOTO= NIC;<br> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0*n =3D CLOCK_UPTIME= ;<br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 break;<br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 case LINUX_CLOCK_PROCESS_CPUTIME_ID:<br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 *n =3D CLOCK_PROCES= S_CPUTIME_ID;<br> @@ -300,10 +300,10 @@ linux_to_native_clockid(clockid_t *n, clockid_t l)<br= > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 break;<br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 case LINUX_CLOCK_MONOTONIC_COARSE:<br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 case LINUX_CLOCK_MONOTONIC_RAW:<br> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0*n =3D CLOCK_MONOTO= NIC_FAST;<br> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0*n =3D CLOCK_UPTIME= _FAST;<br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 break;<br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 case LINUX_CLOCK_BOOTTIME:<br> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0*n =3D CLOCK_UPTIME= ;<br> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0*n =3D CLOCK_MONOTO= NIC;<br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 break;<br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 case LINUX_CLOCK_REALTIME_ALARM:<br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 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> =C2=A0 * Linux compatible names.<br> =C2=A0 */<br> =C2=A0#if __BSD_VISIBLE<br> -#define=C2=A0 =C2=A0 =C2=A0 =C2=A0 CLOCK_BOOTTIME=C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 CLOCK_UPTIME<br> +#define=C2=A0 =C2=A0 =C2=A0 =C2=A0 CLOCK_BOOTTIME=C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 CLOCK_MONOTONIC<br> =C2=A0#define=C2=A0 =C2=A0 =C2=A0 =C2=A0 CLOCK_REALTIME_COARSE=C2=A0 =C2=A0= CLOCK_REALTIME_FAST<br> =C2=A0#define=C2=A0 =C2=A0 =C2=A0 =C2=A0 CLOCK_MONOTONIC_COARSE=C2=A0 CLOCK= _MONOTONIC_FAST<br> =C2=A0#endif<br> <br> </blockquote></div><br clear=3D"all"><br><span class=3D"gmail_signature_pre= fix">-- </span><br><div dir=3D"ltr" class=3D"gmail_signature"><div dir=3D"l= tr"><div><font color=3D"#888888">Nuno Teixeira</font></div><div><div><font = color=3D"#888888"> FreeBSD UNIX:=C2=A0 <eduardo@FreeBSD.org>=C2=A0 =C2=A0Web:=C2=A0 <a h= ref=3D"https://FreeBSD.org" rel=3D"noreferrer" target=3D"_blank">https://Fr= eeBSD.org</a><br></font></div></div></div></div> --000000000000a7274a0619d0017c--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAFDf7ULP1X_LgDOrKhMEFF7AgBQPPaJGhbRsaNcUpUOBo9JN9g>