From nobody Fri Jan 10 16:58:26 2025 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4YV7CL6Z4Vz5kfD2 for ; Fri, 10 Jan 2025 16:58:38 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-pj1-x1035.google.com (mail-pj1-x1035.google.com [IPv6:2607:f8b0:4864:20::1035]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "WR4" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4YV7CL4jpkz44Sj for ; Fri, 10 Jan 2025 16:58:38 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Authentication-Results: mx1.freebsd.org; none Received: by mail-pj1-x1035.google.com with SMTP id 98e67ed59e1d1-2ef6c56032eso2969897a91.2 for ; Fri, 10 Jan 2025 08:58:38 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdimp-com.20230601.gappssmtp.com; s=20230601; t=1736528317; x=1737133117; darn=freebsd.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=tQwiRvEN4HhLBDItRsPtE9zzSMZnFxatLpv4gFBuQm4=; b=TBgDuCuKPRXNN2KFUn4bv9j+jDvlG62bIKlXxrhfqcCW5Q/Pf5Fsl+DdiBmqJN0JDj Mb4IfqP3hxF4PyEouHGD7kJRwNOplGmQRbpPiafbCQHnHsMwNSHH2xQP9J2qXIYVxV56 Vez0+5Xbd/dkf8sU8fqmY1GjXDmXo/8r860arxRR+F+ifuskHTcQRfct3L1hRyJijvp3 vZuC/raN8UR4F10OqZvLufEhh81DFLB/YX0ERAUMHmufN3Dkl0RjgKmyoZSbiwMXKnRY yhyzWD8WoXoSlk8vpKFTbSzl15383K3xxDTX+ac0/yqmAECwO6Irwc4Guq/fII4L3IDs LVwg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736528317; x=1737133117; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=tQwiRvEN4HhLBDItRsPtE9zzSMZnFxatLpv4gFBuQm4=; b=Z7Q8181iZmxU3LXo+06QdAb1nMNRao7GQWZqAbYPSCoNwNwxQL5CUEew41NM7/dLCr QJiRAW5FlZ4IA45VuGBl3xv/z/K93zDdcK1wJm9AJDY/spDRLUHoSSlYaRcKSSrbFEL6 GdFfKIoJe561CmEM2dyBKuWnGr1mr2v+Sw7V/vgjn+UDeoKTnWIL27CfEnBTwTkRfdFU QWbjSuLm795ti+i9mWx4DHnvJQbhho+03ROIfnUiQk99ZNJs6xmcFXFbiQBx5lTXVO6s 7oJWgv/i0rJqecfPsm0uZjylKA1uLo+ZWoF6ieIsbA8hvBiNjC9wfRA6LgsmbxLhyk1o huTQ== X-Forwarded-Encrypted: i=1; AJvYcCXjQotQwPYMJMZjG+UHBr5F3X75AF0RjhvkkoYbroqGiHGC/7a5yy7zjyESDzFwUQgBJTdVYbb5/Aa4lOMd/bjVeFP8ug==@freebsd.org X-Gm-Message-State: AOJu0YxnI1n/s+yEB1WVZ+IXesoWFvU0aI3V1UBidb6y8O4M3eYT8vPF lpvRkrNQCCxS0akwgkw+q+OrqHBoJLMYOO5GY4Qn+q0fE4V+iHkQUaQuX9KJ7AV3esvhTxXlYYf G5ytxSoA3bg3a7qYtqp8qT77jbTgJnKBWA7TxXq57Wureap112Hg= X-Gm-Gg: ASbGncsq1MkhSr2M61yMioTDjKZ19TkBAyJkrZdx2QYKVxTFQ+VtxeA9f0efhN8Z0GT hzwGPVvXslESifQNV6u23/jRcGxcNDEihmlvlaA== X-Google-Smtp-Source: AGHT+IHxZqVKRegm3PMU1o5t+WQOlBg8fhMbyov5ePGWTH56Om2t76a8BhqdoFsZRdIHMcTck9Cu5cD4boKrc5slG6Y= X-Received: by 2002:a17:90b:4a44:b0:2f4:f7f8:fc8a with SMTP id 98e67ed59e1d1-2f548f7649amr17531126a91.33.1736528317211; Fri, 10 Jan 2025 08:58:37 -0800 (PST) List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-main@freebsd.org Sender: owner-dev-commits-src-main@FreeBSD.org MIME-Version: 1.0 References: <202501101600.50AG0jk6062308@gitrepo.freebsd.org> <9B1B709F-5828-489C-81B5-74ED9E4502FC@freebsd.org> <040B6FE0-1C9F-411F-BFA9-D578462C572E@freebsd.org> In-Reply-To: <040B6FE0-1C9F-411F-BFA9-D578462C572E@freebsd.org> From: Warner Losh Date: Fri, 10 Jan 2025 09:58:26 -0700 X-Gm-Features: AbW1kvZUEVBzV2daATWdRzsRS3KcrWe0Kxl3hpGSD17B3Vi00vDPAuap9Xr0mqw Message-ID: Subject: Re: git: 6b82130e6c9a - main - clock: Add a long ticks variable, ticksl To: Jessica Clarke Cc: Mark Johnston , "src-committers@freebsd.org" , "dev-commits-src-all@freebsd.org" , "dev-commits-src-main@freebsd.org" Content-Type: multipart/alternative; boundary="0000000000007d90f7062b5d015b" X-Rspamd-Queue-Id: 4YV7CL4jpkz44Sj X-Spamd-Bar: ---- X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:15169, ipnet:2607:f8b0::/32, country:US] --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 = wrote: > On 10 Jan 2025, at 16:37, Jessica Clarke wrote: > > On 10 Jan 2025, at 16:00, Mark Johnston wrote: > >> > >> The branch main has been updated by markj: > >> > >> URL: > https://cgit.FreeBSD.org/src/commit/?id=3D6b82130e6c9add4a8892ca897df5a0e= c04663ea2 > >> > >> commit 6b82130e6c9add4a8892ca897df5a0ec04663ea2 > >> Author: Mark Johnston > >> AuthorDate: 2025-01-10 15:37:07 +0000 > >> Commit: Mark Johnston > >> 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 > >> > >> -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 > >> + */ > >> + > >> +/* > >> + * Define the "ticks" and "ticksl" variables. The former is overlaid > onto the > >> + * low bits of the latter. > >> + */ > >> + > >> +#if defined(__aarch64__) > >> +#include > >> +#include > >> + > >> +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


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 <jrtc2= 7@FreeBSD.org> wrote:
> On 10 Jan 2025, at 16:00, Mark Johnston <markj@FreeBSD.org> wrot= e:
>>
>> The branch main has been updated by markj:
>>
>> URL: http= s://cgit.FreeBSD.org/src/commit/?id=3D6b82130e6c9add4a8892ca897df5a0ec04663= ea2
>>
>> commit 6b82130e6c9add4a8892ca897df5a0ec04663ea2
>> Author:=C2=A0 =C2=A0 =C2=A0Mark Johnston <markj@FreeBSD.org>=
>> AuthorDate: 2025-01-10 15:37:07 +0000
>> Commit:=C2=A0 =C2=A0 =C2=A0Mark Johnston <markj@FreeBSD.org>=
>> CommitDate: 2025-01-10 15:42:59 +0000
>>
>>=C2=A0 =C2=A0clock: Add a long ticks variable, ticksl
>>
>>=C2=A0 =C2=A0For compatibility with Linux, it's useful to have = a tick counter of
>>=C2=A0 =C2=A0width sizeof(long), but our tick counter is an int.=C2= =A0 Currently the
>>=C2=A0 =C2=A0linuxkpi tries paper over this difference, but this ca= nnot really be
>>=C2=A0 =C2=A0done reliably, so it's desirable to have a wider t= ick counter.=C2=A0 This
>>=C2=A0 =C2=A0change introduces ticksl, keeping the existing ticks v= ariable.
>>
>>=C2=A0 =C2=A0Follow a suggestion from kib to avoid having to mainta= in two separate
>>=C2=A0 =C2=A0counters and to avoid converting existing code to use = ticksl: change
>>=C2=A0 =C2=A0hardclock() to update ticksl instead of ticks, and the= n use assembler
>>=C2=A0 =C2=A0directives to make ticks and ticksl overlap such that = loading ticks
>>=C2=A0 =C2=A0gives the bottom 32 bits.=C2=A0 This makes it possible= to use ticksl in the
>>=C2=A0 =C2=A0linuxkpi without having to convert any native code, an= d without making
>>=C2=A0 =C2=A0hardclock() more complicated or expensive.=C2=A0 Then,= the linuxkpi can be
>>=C2=A0 =C2=A0modified to use ticksl instead of ticks.
>>
>>=C2=A0 =C2=A0Reviewed by:=C2=A0 =C2=A0 olce, kib, emaste
>>=C2=A0 =C2=A0MFC after:=C2=A0 =C2=A0 =C2=A0 1 month
>>=C2=A0 =C2=A0Differential Revision:=C2=A0 https://reviews.= freebsd.org/D48383
>> ---
>> sys/conf/files=C2=A0 =C2=A0 =C2=A0 =C2=A0 |=C2=A0 1 +
>> sys/kern/kern_clock.c | 26 ++++++++++++++------------
>> sys/kern/kern_tc.c=C2=A0 =C2=A0 |=C2=A0 4 ++--
>> sys/kern/subr_param.c |=C2=A0 2 +-
>> sys/kern/subr_ticks.s | 44 +++++++++++++++++++++++++++++++++++++++= +++++
>> sys/sys/kernel.h=C2=A0 =C2=A0 =C2=A0 |=C2=A0 9 +++++++++
>> sys/sys/timetc.h=C2=A0 =C2=A0 =C2=A0 |=C2=A0 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,
>>=C2=A0 =C2=A0 "Approximate number of hardclock ticks in a mill= isecond");
>>
>> 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.= =C2=A0 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_VAL)
>> +#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_ENDIA= N__
>> +#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 ev= en
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 will.= ..

I had this same comment in a review,= but was busy and didn't get to make it.

We *N= EVER* have .s except for weird things we might generate, by convention.
We spent a lot of time fixing this a few months ago.

<= /div>
Warner=C2=A0
--0000000000007d90f7062b5d015b--