Date: Fri, 10 Jan 2025 09:58:26 -0700 From: Warner Losh <imp@bsdimp.com> To: Jessica Clarke <jrtc27@freebsd.org> Cc: Mark Johnston <markj@freebsd.org>, "src-committers@freebsd.org" <src-committers@freebsd.org>, "dev-commits-src-all@freebsd.org" <dev-commits-src-all@freebsd.org>, "dev-commits-src-main@freebsd.org" <dev-commits-src-main@freebsd.org> Subject: Re: git: 6b82130e6c9a - main - clock: Add a long ticks variable, ticksl Message-ID: <CANCZdfrt6ezJdeRKpKQzo7k5PPuznb253Ljx=w7_UVzTg7hkeQ@mail.gmail.com> In-Reply-To: <040B6FE0-1C9F-411F-BFA9-D578462C572E@freebsd.org> References: <202501101600.50AG0jk6062308@gitrepo.freebsd.org> <9B1B709F-5828-489C-81B5-74ED9E4502FC@freebsd.org> <040B6FE0-1C9F-411F-BFA9-D578462C572E@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--0000000000007d90f7062b5d015b Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Fri, Jan 10, 2025 at 9:55=E2=80=AFAM Jessica Clarke <jrtc27@freebsd.org>= wrote: > On 10 Jan 2025, at 16:37, Jessica Clarke <jrtc27@FreeBSD.org> wrote: > > On 10 Jan 2025, at 16:00, Mark Johnston <markj@FreeBSD.org> wrote: > >> > >> The branch main has been updated by markj: > >> > >> URL: > https://cgit.FreeBSD.org/src/commit/?id=3D6b82130e6c9add4a8892ca897df5a0e= c04663ea2 > >> > >> commit 6b82130e6c9add4a8892ca897df5a0ec04663ea2 > >> Author: Mark Johnston <markj@FreeBSD.org> > >> AuthorDate: 2025-01-10 15:37:07 +0000 > >> Commit: Mark Johnston <markj@FreeBSD.org> > >> CommitDate: 2025-01-10 15:42:59 +0000 > >> > >> clock: Add a long ticks variable, ticksl > >> > >> For compatibility with Linux, it's useful to have a tick counter of > >> width sizeof(long), but our tick counter is an int. Currently the > >> linuxkpi tries paper over this difference, but this cannot really be > >> done reliably, so it's desirable to have a wider tick counter. This > >> change introduces ticksl, keeping the existing ticks variable. > >> > >> Follow a suggestion from kib to avoid having to maintain two separat= e > >> counters and to avoid converting existing code to use ticksl: change > >> hardclock() to update ticksl instead of ticks, and then use assemble= r > >> directives to make ticks and ticksl overlap such that loading ticks > >> gives the bottom 32 bits. This makes it possible to use ticksl in t= he > >> linuxkpi without having to convert any native code, and without maki= ng > >> hardclock() more complicated or expensive. Then, the linuxkpi can b= e > >> modified to use ticksl instead of ticks. > >> > >> Reviewed by: olce, kib, emaste > >> MFC after: 1 month > >> Differential Revision: https://reviews.freebsd.org/D48383 > >> --- > >> sys/conf/files | 1 + > >> sys/kern/kern_clock.c | 26 ++++++++++++++------------ > >> sys/kern/kern_tc.c | 4 ++-- > >> sys/kern/subr_param.c | 2 +- > >> sys/kern/subr_ticks.s | 44 +++++++++++++++++++++++++++++++++++++++++++= + > >> sys/sys/kernel.h | 9 +++++++++ > >> sys/sys/timetc.h | 2 +- > >> 7 files changed, 72 insertions(+), 16 deletions(-) > >> > >> diff --git a/sys/conf/files b/sys/conf/files > >> index d358737c5613..a630d9dd72bc 100644 > >> --- a/sys/conf/files > >> +++ b/sys/conf/files > >> @@ -3932,6 +3932,7 @@ kern/subr_stack.c optional ddb | stack | ktr > >> kern/subr_stats.c optional stats > >> kern/subr_taskqueue.c standard > >> kern/subr_terminal.c optional vt > >> +kern/subr_ticks.s standard > >> kern/subr_trap.c standard > >> kern/subr_turnstile.c standard > >> kern/subr_uio.c standard > >> diff --git a/sys/kern/kern_clock.c b/sys/kern/kern_clock.c > >> index 6fa2272ed54a..b11c0d235139 100644 > >> --- a/sys/kern/kern_clock.c > >> +++ b/sys/kern/kern_clock.c > >> @@ -323,7 +323,7 @@ read_cpu_time(long *cp_time) > >> > >> #include <sys/watchdog.h> > >> > >> -static int watchdog_ticks; > >> +static long watchdog_ticks; > >> static int watchdog_enabled; > >> static void watchdog_fire(void); > >> static void watchdog_config(void *, u_int, int *); > >> @@ -369,10 +369,9 @@ watchdog_attach(void) > >> int stathz; > >> int profhz; > >> int profprocs; > >> -volatile int ticks; > >> int psratio; > >> > >> -DPCPU_DEFINE_STATIC(int, pcputicks); /* Per-CPU version of ticks. */ > >> +DPCPU_DEFINE_STATIC(long, pcputicks); /* Per-CPU version of ticks. */ > >> #ifdef DEVICE_POLLING > >> static int devpoll_run =3D 0; > >> #endif > >> @@ -480,14 +479,14 @@ hardclock(int cnt, int usermode) > >> struct pstats *pstats; > >> struct thread *td =3D curthread; > >> struct proc *p =3D td->td_proc; > >> - int *t =3D DPCPU_PTR(pcputicks); > >> - int global, i, newticks; > >> + long global, newticks, *t; > >> > >> /* > >> * Update per-CPU and possibly global ticks values. > >> */ > >> + t =3D DPCPU_PTR(pcputicks); > >> *t +=3D cnt; > >> - global =3D ticks; > >> + global =3D atomic_load_long(&ticksl); > >> do { > >> newticks =3D *t - global; > >> if (newticks <=3D 0) { > >> @@ -496,7 +495,7 @@ hardclock(int cnt, int usermode) > >> newticks =3D 0; > >> break; > >> } > >> - } while (!atomic_fcmpset_int(&ticks, &global, *t)); > >> + } while (!atomic_fcmpset_long(&ticksl, &global, *t)); > >> > >> /* > >> * Run current process's virtual and profile time, as needed. > >> @@ -525,8 +524,10 @@ hardclock(int cnt, int usermode) > >> } > >> #endif /* DEVICE_POLLING */ > >> if (watchdog_enabled > 0) { > >> - i =3D atomic_fetchadd_int(&watchdog_ticks, -newticks); > >> - if (i > 0 && i <=3D newticks) > >> + long left; > >> + > >> + left =3D atomic_fetchadd_long(&watchdog_ticks, -newticks); > >> + if (left > 0 && left <=3D newticks) > >> watchdog_fire(); > >> } > >> intr_event_handle(clk_intr_event, NULL); > >> @@ -540,11 +541,12 @@ hardclock(int cnt, int usermode) > >> void > >> hardclock_sync(int cpu) > >> { > >> - int *t; > >> + long *t; > >> + > >> KASSERT(!CPU_ABSENT(cpu), ("Absent CPU %d", cpu)); > >> - t =3D DPCPU_ID_PTR(cpu, pcputicks); > >> > >> - *t =3D ticks; > >> + t =3D DPCPU_ID_PTR(cpu, pcputicks); > >> + *t =3D ticksl; > >> } > >> > >> /* > >> diff --git a/sys/kern/kern_tc.c b/sys/kern/kern_tc.c > >> index 26f09cb60260..a797a101bf6f 100644 > >> --- a/sys/kern/kern_tc.c > >> +++ b/sys/kern/kern_tc.c > >> @@ -1916,9 +1916,9 @@ SYSCTL_INT(_kern_timecounter, OID_AUTO, tick, > CTLFLAG_RD, &tc_tick, 0, > >> "Approximate number of hardclock ticks in a millisecond"); > >> > >> void > >> -tc_ticktock(int cnt) > >> +tc_ticktock(long cnt) > >> { > >> - static int count; > >> + static long count; > >> > >> if (mtx_trylock_spin(&tc_setclock_mtx)) { > >> count +=3D cnt; > >> diff --git a/sys/kern/subr_param.c b/sys/kern/subr_param.c > >> index 19169ba63061..f4359efec466 100644 > >> --- a/sys/kern/subr_param.c > >> +++ b/sys/kern/subr_param.c > >> @@ -197,7 +197,7 @@ init_param1(void) > >> * Arrange for ticks to wrap 10 minutes after boot to help catch > >> * sign problems sooner. > >> */ > >> - ticks =3D INT_MAX - (hz * 10 * 60); > >> + ticksl =3D INT_MAX - (hz * 10 * 60); > >> > >> vn_lock_pair_pause_max =3D hz / 100; > >> if (vn_lock_pair_pause_max =3D=3D 0) > >> diff --git a/sys/kern/subr_ticks.s b/sys/kern/subr_ticks.s > >> new file mode 100644 > >> index 000000000000..6565ba424137 > >> --- /dev/null > >> +++ b/sys/kern/subr_ticks.s > >> @@ -0,0 +1,44 @@ > >> +/*- > >> + * SPDX-License-Identifier: BSD-2-Clause > >> + * > >> + * Copyright (c) 2025 Mark Johnston <markj@FreeBSD.org> > >> + */ > >> + > >> +/* > >> + * Define the "ticks" and "ticksl" variables. The former is overlaid > onto the > >> + * low bits of the latter. > >> + */ > >> + > >> +#if defined(__aarch64__) > >> +#include <sys/elf_common.h> > >> +#include <machine/asm.h> > >> + > >> +GNU_PROPERTY_AARCH64_FEATURE_1_NOTE(GNU_PROPERTY_AARCH64_FEATURE_1_VA= L) > >> +#endif > >> + > >> +#ifdef _ILP32 > >> +#define SIZEOF_TICKSL 4 > >> +#define TICKSL_INIT .long 0 > >> +#else > >> +#define SIZEOF_TICKSL 8 > >> +#define TICKSL_INIT .quad 0 > >> +#endif > >> + > >> +#if defined(_ILP32) || __BYTE_ORDER__ =3D=3D __ORDER_LITTLE_ENDIAN__ > >> +#define TICKS_OFFSET 0 > >> +#else > >> +#define TICKS_OFFSET 4 > >> +#endif > >> + > >> + .data > >> + > >> + .global ticksl > >> + .type ticksl, %object > >> + .align SIZEOF_TICKSL > >> +ticksl: TICKSL_INIT > >> + .size ticksl, SIZEOF_TICKSL > >> + > >> + .global ticks > >> + .type ticks, %object > >> +ticks =3Dticksl + TICKS_OFFSET > >> + .size ticks, 4 > > > > This can be simplified to: > > > > #if __BYTE_ORDER__ =3D=3D __ORDER_LITTLE_ENDIAN__ > > #define TICKS_OFFSET 0 > > #else > > #define TICKS_OFFSET (__SIZEOF_LONG__ - __SIZEOF_INT__) > > #endif > > > > .data > > > > .global ticksl > > .type ticksl, %object > > .align __SIZEOF_LONG__ > > ticksl: .zero __SIZEOF_LONG__ > > .size ticksl, __SIZEOF_LONG__ > > > > .global ticks > > .type ticks, %object > > ticks =3Dticksl + TICKS_OFFSET > > .size ticks, __SIZEOF_INT__ > > > > (excuse my mail client stripping the tabs...) > > > > No need to check the ABI beyond endianness. > > > > Also, shouldn=E2=80=99t these both be in .bss? > > And one more thing: the convention is to use .S, not .s, for assembly > files, since the former are preprocessed[^1]. I=E2=80=99m surprised this = even > builds when given the lower case suffix, as bsd.suffixes.mk passes > -x assembler for the .s.o rule. > > Jess > > [^1]: Unless you know it doesn=E2=80=99t need preprocessing and never wil= l... > I had this same comment in a review, but was busy and didn't get to make it= . We *NEVER* have .s except for weird things we might generate, by convention= . We spent a lot of time fixing this a few months ago. Warner --0000000000007d90f7062b5d015b Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable <div dir=3D"ltr"><div dir=3D"ltr"><br></div><br><div class=3D"gmail_quote g= mail_quote_container"><div dir=3D"ltr" class=3D"gmail_attr">On Fri, Jan 10,= 2025 at 9:55=E2=80=AFAM Jessica Clarke <<a href=3D"mailto:jrtc27@freebs= d.org">jrtc27@freebsd.org</a>> wrote:<br></div><blockquote class=3D"gmai= l_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,20= 4,204);padding-left:1ex">On 10 Jan 2025, at 16:37, Jessica Clarke <jrtc2= 7@FreeBSD.org> wrote:<br> > On 10 Jan 2025, at 16:00, Mark Johnston <markj@FreeBSD.org> wrot= e:<br> >> <br> >> The branch main has been updated by markj:<br> >> <br> >> URL: <a href=3D"https://cgit.FreeBSD.org/src/commit/?id=3D6b82130e= 6c9add4a8892ca897df5a0ec04663ea2" rel=3D"noreferrer" target=3D"_blank">http= s://cgit.FreeBSD.org/src/commit/?id=3D6b82130e6c9add4a8892ca897df5a0ec04663= ea2</a><br> >> <br> >> commit 6b82130e6c9add4a8892ca897df5a0ec04663ea2<br> >> Author:=C2=A0 =C2=A0 =C2=A0Mark Johnston <markj@FreeBSD.org>= <br> >> AuthorDate: 2025-01-10 15:37:07 +0000<br> >> Commit:=C2=A0 =C2=A0 =C2=A0Mark Johnston <markj@FreeBSD.org>= <br> >> CommitDate: 2025-01-10 15:42:59 +0000<br> >> <br> >>=C2=A0 =C2=A0clock: Add a long ticks variable, ticksl<br> >> <br> >>=C2=A0 =C2=A0For compatibility with Linux, it's useful to have = a tick counter of<br> >>=C2=A0 =C2=A0width sizeof(long), but our tick counter is an int.=C2= =A0 Currently the<br> >>=C2=A0 =C2=A0linuxkpi tries paper over this difference, but this ca= nnot really be<br> >>=C2=A0 =C2=A0done reliably, so it's desirable to have a wider t= ick counter.=C2=A0 This<br> >>=C2=A0 =C2=A0change introduces ticksl, keeping the existing ticks v= ariable.<br> >> <br> >>=C2=A0 =C2=A0Follow a suggestion from kib to avoid having to mainta= in two separate<br> >>=C2=A0 =C2=A0counters and to avoid converting existing code to use = ticksl: change<br> >>=C2=A0 =C2=A0hardclock() to update ticksl instead of ticks, and the= n use assembler<br> >>=C2=A0 =C2=A0directives to make ticks and ticksl overlap such that = loading ticks<br> >>=C2=A0 =C2=A0gives the bottom 32 bits.=C2=A0 This makes it possible= to use ticksl in the<br> >>=C2=A0 =C2=A0linuxkpi without having to convert any native code, an= d without making<br> >>=C2=A0 =C2=A0hardclock() more complicated or expensive.=C2=A0 Then,= the linuxkpi can be<br> >>=C2=A0 =C2=A0modified to use ticksl instead of ticks.<br> >> <br> >>=C2=A0 =C2=A0Reviewed by:=C2=A0 =C2=A0 olce, kib, emaste<br> >>=C2=A0 =C2=A0MFC after:=C2=A0 =C2=A0 =C2=A0 1 month<br> >>=C2=A0 =C2=A0Differential Revision:=C2=A0 <a href=3D"https://review= s.freebsd.org/D48383" rel=3D"noreferrer" target=3D"_blank">https://reviews.= freebsd.org/D48383</a><br> >> ---<br> >> sys/conf/files=C2=A0 =C2=A0 =C2=A0 =C2=A0 |=C2=A0 1 +<br> >> sys/kern/kern_clock.c | 26 ++++++++++++++------------<br> >> sys/kern/kern_tc.c=C2=A0 =C2=A0 |=C2=A0 4 ++--<br> >> sys/kern/subr_param.c |=C2=A0 2 +-<br> >> sys/kern/subr_ticks.s | 44 +++++++++++++++++++++++++++++++++++++++= +++++<br> >> sys/sys/kernel.h=C2=A0 =C2=A0 =C2=A0 |=C2=A0 9 +++++++++<br> >> sys/sys/timetc.h=C2=A0 =C2=A0 =C2=A0 |=C2=A0 2 +-<br> >> 7 files changed, 72 insertions(+), 16 deletions(-)<br> >> <br> >> diff --git a/sys/conf/files b/sys/conf/files<br> >> index d358737c5613..a630d9dd72bc 100644<br> >> --- a/sys/conf/files<br> >> +++ b/sys/conf/files<br> >> @@ -3932,6 +3932,7 @@ kern/subr_stack.c optional ddb | stack | ktr= <br> >> kern/subr_stats.c optional stats<br> >> kern/subr_taskqueue.c standard<br> >> kern/subr_terminal.c optional vt<br> >> +kern/subr_ticks.s standard<br> >> kern/subr_trap.c standard<br> >> kern/subr_turnstile.c standard<br> >> kern/subr_uio.c standard<br> >> diff --git a/sys/kern/kern_clock.c b/sys/kern/kern_clock.c<br> >> index 6fa2272ed54a..b11c0d235139 100644<br> >> --- a/sys/kern/kern_clock.c<br> >> +++ b/sys/kern/kern_clock.c<br> >> @@ -323,7 +323,7 @@ read_cpu_time(long *cp_time)<br> >> <br> >> #include <sys/watchdog.h><br> >> <br> >> -static int watchdog_ticks;<br> >> +static long watchdog_ticks;<br> >> static int watchdog_enabled;<br> >> static void watchdog_fire(void);<br> >> static void watchdog_config(void *, u_int, int *);<br> >> @@ -369,10 +369,9 @@ watchdog_attach(void)<br> >> int stathz;<br> >> int profhz;<br> >> int profprocs;<br> >> -volatile int ticks;<br> >> int psratio;<br> >> <br> >> -DPCPU_DEFINE_STATIC(int, pcputicks); /* Per-CPU version of ticks.= */<br> >> +DPCPU_DEFINE_STATIC(long, pcputicks); /* Per-CPU version of ticks= . */<br> >> #ifdef DEVICE_POLLING<br> >> static int devpoll_run =3D 0;<br> >> #endif<br> >> @@ -480,14 +479,14 @@ hardclock(int cnt, int usermode)<br> >> struct pstats *pstats;<br> >> struct thread *td =3D curthread;<br> >> struct proc *p =3D td->td_proc;<br> >> - int *t =3D DPCPU_PTR(pcputicks);<br> >> - int global, i, newticks;<br> >> + long global, newticks, *t;<br> >> <br> >> /*<br> >> * Update per-CPU and possibly global ticks values.<br> >> */<br> >> + t =3D DPCPU_PTR(pcputicks);<br> >> *t +=3D cnt;<br> >> - global =3D ticks;<br> >> + global =3D atomic_load_long(&ticksl);<br> >> do {<br> >> newticks =3D *t - global;<br> >> if (newticks <=3D 0) {<br> >> @@ -496,7 +495,7 @@ hardclock(int cnt, int usermode)<br> >> newticks =3D 0;<br> >> break;<br> >> }<br> >> - } while (!atomic_fcmpset_int(&ticks, &global, *t));<br> >> + } while (!atomic_fcmpset_long(&ticksl, &global, *t));<br= > >> <br> >> /*<br> >> * Run current process's virtual and profile time, as needed.<b= r> >> @@ -525,8 +524,10 @@ hardclock(int cnt, int usermode)<br> >> }<br> >> #endif /* DEVICE_POLLING */<br> >> if (watchdog_enabled > 0) {<br> >> - i =3D atomic_fetchadd_int(&watchdog_ticks, -newticks);<br> >> - if (i > 0 && i <=3D newticks)<br> >> + long left;<br> >> +<br> >> + left =3D atomic_fetchadd_long(&watchdog_ticks, -newticks);<b= r> >> + if (left > 0 && left <=3D newticks)<br> >> watchdog_fire();<br> >> }<br> >> intr_event_handle(clk_intr_event, NULL);<br> >> @@ -540,11 +541,12 @@ hardclock(int cnt, int usermode)<br> >> void<br> >> hardclock_sync(int cpu)<br> >> {<br> >> - int *t;<br> >> + long *t;<br> >> +<br> >> KASSERT(!CPU_ABSENT(cpu), ("Absent CPU %d", cpu));<br> >> - t =3D DPCPU_ID_PTR(cpu, pcputicks);<br> >> <br> >> - *t =3D ticks;<br> >> + t =3D DPCPU_ID_PTR(cpu, pcputicks);<br> >> + *t =3D ticksl;<br> >> }<br> >> <br> >> /*<br> >> diff --git a/sys/kern/kern_tc.c b/sys/kern/kern_tc.c<br> >> index 26f09cb60260..a797a101bf6f 100644<br> >> --- a/sys/kern/kern_tc.c<br> >> +++ b/sys/kern/kern_tc.c<br> >> @@ -1916,9 +1916,9 @@ SYSCTL_INT(_kern_timecounter, OID_AUTO, tick= , CTLFLAG_RD, &tc_tick, 0,<br> >>=C2=A0 =C2=A0 "Approximate number of hardclock ticks in a mill= isecond");<br> >> <br> >> void<br> >> -tc_ticktock(int cnt)<br> >> +tc_ticktock(long cnt)<br> >> {<br> >> - static int count;<br> >> + static long count;<br> >> <br> >> if (mtx_trylock_spin(&tc_setclock_mtx)) {<br> >> count +=3D cnt;<br> >> diff --git a/sys/kern/subr_param.c b/sys/kern/subr_param.c<br> >> index 19169ba63061..f4359efec466 100644<br> >> --- a/sys/kern/subr_param.c<br> >> +++ b/sys/kern/subr_param.c<br> >> @@ -197,7 +197,7 @@ init_param1(void)<br> >> * Arrange for ticks to wrap 10 minutes after boot to help catch<br= > >> * sign problems sooner.<br> >> */<br> >> - ticks =3D INT_MAX - (hz * 10 * 60);<br> >> + ticksl =3D INT_MAX - (hz * 10 * 60);<br> >> <br> >> vn_lock_pair_pause_max =3D hz / 100;<br> >> if (vn_lock_pair_pause_max =3D=3D 0)<br> >> diff --git a/sys/kern/subr_ticks.s b/sys/kern/subr_ticks.s<br> >> new file mode 100644<br> >> index 000000000000..6565ba424137<br> >> --- /dev/null<br> >> +++ b/sys/kern/subr_ticks.s<br> >> @@ -0,0 +1,44 @@<br> >> +/*-<br> >> + * SPDX-License-Identifier: BSD-2-Clause<br> >> + *<br> >> + * Copyright (c) 2025 Mark Johnston <markj@FreeBSD.org><br> >> + */<br> >> +<br> >> +/*<br> >> + * Define the "ticks" and "ticksl" variables.= =C2=A0 The former is overlaid onto the<br> >> + * low bits of the latter.<br> >> + */<br> >> +<br> >> +#if defined(__aarch64__)<br> >> +#include <sys/elf_common.h><br> >> +#include <machine/asm.h><br> >> +<br> >> +GNU_PROPERTY_AARCH64_FEATURE_1_NOTE(GNU_PROPERTY_AARCH64_FEATURE_= 1_VAL)<br> >> +#endif<br> >> +<br> >> +#ifdef _ILP32<br> >> +#define SIZEOF_TICKSL 4<br> >> +#define TICKSL_INIT .long 0<br> >> +#else<br> >> +#define SIZEOF_TICKSL 8<br> >> +#define TICKSL_INIT .quad 0<br> >> +#endif<br> >> +<br> >> +#if defined(_ILP32) || __BYTE_ORDER__ =3D=3D __ORDER_LITTLE_ENDIA= N__<br> >> +#define TICKS_OFFSET 0<br> >> +#else<br> >> +#define TICKS_OFFSET 4<br> >> +#endif<br> >> +<br> >> + .data<br> >> +<br> >> + .global ticksl<br> >> + .type ticksl, %object<br> >> + .align SIZEOF_TICKSL<br> >> +ticksl: TICKSL_INIT<br> >> + .size ticksl, SIZEOF_TICKSL<br> >> +<br> >> + .global ticks<br> >> + .type ticks, %object<br> >> +ticks =3Dticksl + TICKS_OFFSET<br> >> + .size ticks, 4<br> > <br> > This can be simplified to:<br> > <br> > #if __BYTE_ORDER__ =3D=3D __ORDER_LITTLE_ENDIAN__<br> > #define TICKS_OFFSET 0<br> > #else<br> > #define TICKS_OFFSET (__SIZEOF_LONG__ - __SIZEOF_INT__)<br> > #endif<br> > <br> > .data<br> > <br> > .global ticksl<br> > .type ticksl, %object<br> > .align __SIZEOF_LONG__<br> > ticksl: .zero __SIZEOF_LONG__<br> > .size ticksl, __SIZEOF_LONG__<br> > <br> > .global ticks<br> > .type ticks, %object<br> > ticks =3Dticksl + TICKS_OFFSET<br> > .size ticks, __SIZEOF_INT__<br> > <br> > (excuse my mail client stripping the tabs...)<br> > <br> > No need to check the ABI beyond endianness.<br> > <br> > Also, shouldn=E2=80=99t these both be in .bss?<br> <br> And one more thing: the convention is to use .S, not .s, for assembly<br> files, since the former are preprocessed[^1]. I=E2=80=99m surprised this ev= en<br> builds when given the lower case suffix, as <a href=3D"http://bsd.suffixes.= mk" rel=3D"noreferrer" target=3D"_blank">bsd.suffixes.mk</a> passes<br> -x assembler for the .s.o rule.<br> <br> Jess<br> <br> [^1]: Unless you know it doesn=E2=80=99t need preprocessing and never will.= ..<br></blockquote><div><br></div><div>I had this same comment in a review,= but was busy and didn't get to make it.</div><div><br></div><div>We *N= EVER* have .s except for weird things we might generate, by convention.</di= v><div>We spent a lot of time fixing this a few months ago.</div><div><br><= /div><div>Warner=C2=A0</div></div></div> --0000000000007d90f7062b5d015b--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfrt6ezJdeRKpKQzo7k5PPuznb253Ljx=w7_UVzTg7hkeQ>