Re: [PATCH 1/3] time/jiffies: Don't saturate usecs_to_jiffies() on a truncated limit

[shashank Jain]

Thanks for the review.

Agreed on all three points. For v2 I'll split patch 1 into the removal
of the bogus limit and the rounding fix, drop the now-pointless
__builtin_constant_p() branch in usecs_to_jiffies(), and rewrite the
changelogs and the cover letter much shorter, with a table of the wrong
values as in Zhan's patch.

I'll wait a few days for other comments before sending v2.

Shashank


On Mon, Sep 28, 2026 at 5:08 PM Joel Granados <joel.granados@xxxxxxxxxx> wrote:
>
> On Mon, Sep 28, 2026 at 03:05:59PM +0530, Shashank Mohan Jain wrote:
> > usecs_to_jiffies() and __usecs_to_jiffies() return MAX_JIFFY_OFFSET, an
> > effectively infinite timeout, when
> >
> >       u > jiffies_to_usecs(MAX_JIFFY_OFFSET)
> >
> > but jiffies_to_usecs() returns an unsigned int, so the limit is
> > MAX_JIFFY_OFFSET microseconds truncated to 32 bits, an arbitrary value.
> > With HZ=300 the out-of-line jiffies_to_usecs() also overflows 64 bits in
> > j * HZ_TO_USEC_NUM, and the limit ends up at 1431649098 us on 64-bit and
> > 1431649781 us on 32-bit. Every timeout between about 23.9 and 71.6
> > minutes then becomes infinite: usecs_to_jiffies(2000000000) returns
> > MAX_JIFFY_OFFSET instead of 600000. For instance, a PIE tupdate or a
> > DAMOS watermark interval of 30 minutes never fires at HZ=300. With
> > HZ=100, 250 and 1000 the limit is 4294947296, 4294959296 and 4294965296
> > us, so there only the top 19999, 7999 and 1999 values are affected.
> >
> > The check is not needed at all. MAX_JIFFY_OFFSET is at least 2^30 - 2
> > jiffies and HZ is below 12288, so UINT_MAX microseconds are far below
> > MAX_JIFFY_OFFSET jiffies for every supported HZ. Drop it, update the
> > kernel-doc, and add a static_assert() for the assumption.
> >
> > The bogus limit was also hiding a wrap: when HZ divides USEC_PER_SEC,
> > _usecs_to_jiffies() rounds up with (u + USEC_PER_SEC / HZ - 1) in
> > unsigned int, which wraps for u close to UINT_MAX (usecs_to_jiffies(
> > UINT_MAX) would become 0 at HZ=100). Round up with a remainder test
> > instead. The reciprocal multiplication used for the other HZ values is
> > done in 64 bits and does not wrap for any 32-bit input.
>
> Looks like your fixing two things in one patch. I would suggest making
> two patches
>
> >
> > Values above the old limit, including UINT_MAX, now give a finite
> > timeout (at most about 71.6 minutes) with every HZ. No caller uses
> > UINT_MAX as an "infinite" marker; the callers that clamp to UINT_MAX or
> > range-check the result (TCP_RTO_MIN_US, TCP_DELACK_MAX_US) keep working.
> >
> > When HZ does not divide USEC_PER_SEC, jiffies_to_usecs() is out of line,
> > so the check also defeated the constant folding: every
> > usecs_to_jiffies(<constant>) compiled to a call to jiffies_to_usecs()
> > and a compare. Those now fold to a constant.
>
> This commit message (and the others in this series as well as the cover
> letter) is too verbose, hard to read. It is not obvious what you are
> doing and why.
>
> I suggest a table of erroneous values. Like [1]
>
> [1] https://lore.kernel.org/20260923134237.3628505-1-zhanxusheng@xxxxxxxxxx
> >
> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > Assisted-by: LLM
> > Signed-off-by: Shashank Mohan Jain <jain.sm@xxxxxxxxx>
> > ---
> > Checked in userspace over all 2^32 inputs, with the current code and
> > the code from this patch both extracted verbatim from the tree and
> > built with the generated timeconst.h, for every Kconfig HZ value (24,
> > 32, 48, 64, 100, 128, 200, 250, 256, 300, 500, 1000, 1024, 1200),
> > 32- and 64-bit. The KUnit test in patch 3 fails without this patch on
> > UML x86_64 (HZ=100) and on qemu x86_64 and i386 with HZ=300, and passes
> > with it. Prepared with Claude Code (Anthropic), model Claude Opus 5.5
> > (claude-opus-5-5).
> >
> >  include/linux/jiffies.h | 22 ++++++++--------------
> >  kernel/time/time.c      | 10 +++++++---
> >  2 files changed, 15 insertions(+), 17 deletions(-)
> >
> > diff --git a/include/linux/jiffies.h b/include/linux/jiffies.h
> > index bbd57061802c..d3f2663e4435 100644
> > --- a/include/linux/jiffies.h
> > +++ b/include/linux/jiffies.h
> > @@ -575,7 +575,8 @@ extern unsigned long __usecs_to_jiffies(const unsigned int u);
> >  #if !(USEC_PER_SEC % HZ)
> >  static inline unsigned long _usecs_to_jiffies(const unsigned int u)
> >  {
> > -     return (u + (USEC_PER_SEC / HZ) - 1) / (USEC_PER_SEC / HZ);
> > +     /* Round up without overflowing for u close to UINT_MAX */
> > +     return u / (USEC_PER_SEC / HZ) + !!(u % (USEC_PER_SEC / HZ));
> >  }
> >  #else
> >  static inline unsigned long _usecs_to_jiffies(const unsigned int u)
> > @@ -589,14 +590,10 @@ static inline unsigned long _usecs_to_jiffies(const unsigned int u)
> >   * usecs_to_jiffies: - convert microseconds to jiffies
> >   * @u:       time in microseconds
> >   *
> > - * conversion is done as follows:
> > - *
> > - * - 'too large' values [that would result in larger than
> > - *   MAX_JIFFY_OFFSET values] mean 'infinite timeout' too.
> > - *
> > - * - all other values are converted to jiffies by either multiplying
> > - *   the input value by a factor or dividing it with a factor and
> > - *   handling any 32-bit overflows as for msecs_to_jiffies.
> > + * conversion is done by either multiplying the input value by a factor
> > + * or dividing it with a factor, rounding up. Any unsigned int number of
> > + * microseconds is far below MAX_JIFFY_OFFSET jiffies for every supported
> > + * HZ, so, unlike msecs_to_jiffies(), there is no 'infinite timeout' case.
> >   *
> >   * usecs_to_jiffies() checks for the passed in value being a constant
> >   * via __builtin_constant_p() allowing gcc to eliminate most of the
> > @@ -611,13 +608,10 @@ static inline unsigned long _usecs_to_jiffies(const unsigned int u)
> >   */
> >  static __always_inline unsigned long usecs_to_jiffies(const unsigned int u)
> >  {
> > -     if (__builtin_constant_p(u)) {
> > -             if (u > jiffies_to_usecs(MAX_JIFFY_OFFSET))
> > -                     return MAX_JIFFY_OFFSET;
> > +     if (__builtin_constant_p(u))
> >               return _usecs_to_jiffies(u);
> > -     } else {
> > +     else
> >               return __usecs_to_jiffies(u);
> > -     }
>
> This effectively becomes
>
> if (__builtin_constant_p(u))
>   return _usecs_to_jiffies(u);
> else
>   return _usecs_to_jiffies(u);
>
> What is the point of this?
>
> Best
> >
> >  extern unsigned long timespec64_to_jiffies(const struct timespec64 *value);
> > diff --git a/kernel/time/time.c b/kernel/time/time.c
> > index 079ab34f61db..470195e88ca0 100644
> > --- a/kernel/time/time.c
> > +++ b/kernel/time/time.c
> > @@ -591,16 +591,20 @@ unsigned long __msecs_to_jiffies(const unsigned int m)
> >  }
> >  EXPORT_SYMBOL(__msecs_to_jiffies);
> >
> > +/*
> > + * UINT_MAX microseconds are far less than MAX_JIFFY_OFFSET jiffies for every
> > + * supported HZ, so usecs_to_jiffies() never needs to saturate.
> > + */
> > +static_assert((u64)UINT_MAX * HZ / USEC_PER_SEC < MAX_JIFFY_OFFSET);
> > +
> >  /**
> >   * __usecs_to_jiffies: - convert microseconds to jiffies
> > - * @u:       time in milliseconds
> > + * @u:       time in microseconds
> >   *
> >   * Return: jiffies value
> >   */
> >  unsigned long __usecs_to_jiffies(const unsigned int u)
> >  {
> > -     if (u > jiffies_to_usecs(MAX_JIFFY_OFFSET))
> > -             return MAX_JIFFY_OFFSET;
> >       return _usecs_to_jiffies(u);
> >  }
> >  EXPORT_SYMBOL(__usecs_to_jiffies);
> > --
> > 2.43.0
> >