Re: git: 6b82130e6c9a - main - clock: Add a long ticks variable, ticksl

On Sat, Jan 11, 2025 at 10:59:08AM -0500, John Baldwin wrote:
> On 1/10/25 11:00, Mark Johnston wrote:
> > The branch main has been updated by markj:
> > 
> > URL: https://cgit.FreeBSD.org/src/commit/?id=6b82130e6c9add4a8892ca897df5a0ec04663ea2
> > 
> > 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 separate
> >      counters and to avoid converting existing code to use ticksl: change
> >      hardclock() to update ticksl instead of ticks, and then use assembler
> >      directives to make ticks and ticksl overlap such that loading ticks
> >      gives the bottom 32 bits.  This makes it possible to use ticksl in the
> >      linuxkpi without having to convert any native code, and without making
> >      hardclock() more complicated or expensive.  Then, the linuxkpi can be
> >      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/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 = INT_MAX - (hz * 10 * 60);
> > +	ticksl = INT_MAX - (hz * 10 * 60);
> >   	vn_lock_pair_pause_max = hz / 100;
> >   	if (vn_lock_pair_pause_max == 0)
> 
> Did you consider using LONG_MAX here instead of INT_MAX?  That would make
> both values roll over 10 minutes after boot so that sign problems can be
> tested for both counters.

Yes, an earlier revision of the diff did that.  I changed it because a
64-bit tick counter won't overflow in a reasonable amount of time, and
because we still have to worry about overflow of the 32-bit counter
since that's what the kernel still uses everywhere.