Re: widening ticks

On Wed, 8 Jan 2025 18:07:47 -0500
Mark Johnston <markj@freebsd.org> wrote:

> On Thu, Jan 09, 2025 at 12:18:48AM +0200, Konstantin Belousov wrote:
> > On Wed, Jan 08, 2025 at 04:31:16PM -0500, Mark Johnston wrote:
> > > The global "ticks" variable counts hardclock ticks, it's widely used in
> > > the kernel for low-precision timekeeping.  The linuxkpi provides a very
> > > similar variable, "jiffies", but there's an incompatibility: the former
> > > is a signed int and the latter is an unsigned long.  It's not
> > > particularly easy to paper over this difference, which has been
> > > responsible for some nasty bugs, and modifying drivers to store the
> > > jiffies value in a signed int is error-prone and a maintenance burden
> > > that the linuxkpi is supposed to avoid.
> > > 
> > > It would be nice to provide a compatible implementation of jiffies.  I
> > > can see a few approaches:
> > > - Define a 64-bit ticks variable, say ticks64, and make hardclock()
> > >   update both ticks and ticks64.  Then #define jiffies ticks64 on 64-bit
> > >   platforms.  This is the simplest to implement, but it adds extra work
> > >   to hardclock() and is somewhat ugly.
> > > - Make ticks an int64_t or a long and convert our native code
> > >   accordingly.  This is cleaner but requires a lot of auditing to avoid
> > >   introducing bugs, though perhaps some code could be left unmodified,
> > >   implicitly truncating the value to an int.  For example I think
> > >   sched_pctcpu_update() is fine.  I've gotten an amd64 kernel to compile
> > >   and boot with this change, but it's hard to be confident in it.  This
> > >   approach also has the potential downside of bloating structures that
> > >   store a ticks value, and it can't be MFCed.
> > > - Introduce a 64-bit ticks variable, ticks64, and
> > >   #define ticks ((int)ticks64).  This requires renaming any struct
> > >   fields and local vars named "ticks", of which there's a decent number,
> > >   but that can be done fairly mechanically.
> > > 
> > > Is there another solution which avoids these pitfalls?  If not, should
> > > we go ahead with one of these approaches?  If so, which one?
> > 
> > You cannot do this in C, but can in asm:
> >         .data
> >         .globl  ticksl, ticks
> >         .type   ticksl, @object
> >         .type   ticks, @object
> > ticksl: .quad
> >         .size   ticksl, 8
> > ticks   =ticksl		/* for little-endian */
> > /* ticks	=ticksl + 4  for big-endian */
> >         .size   ticks, 4
> > 
> > 
> > Then update only ticksl in the hardclock().
> 
> I implemented your suggestion here: https://reviews.freebsd.org/D48383

As this is already committed to main, commenting here instead of review
D48383.

Maybe I'm too paranoid and overlooking something, but...

*If "jiffies" in LinuxKPI is really unsigned, isn't there any
 possibilities that relies on its value to be larger than
 0x7fffffffffffffff as a threshold?
 (Yes, it should be silly and non-realistic, but theoretically
  possible.)

*Is anywhere checking carry (sign) bit for int on LP32?
 Maybe it would be the reason if "jiffies" in LinuxKPI is really
 unsigned.

Regards.

-- 
Tomoaki AOKI    <junchoon@dec.sakura.ne.jp>