Re: [PATCH RESEND] sched/fair: Fix overflow in vruntime_eligible()

[Peter Zijlstra]

On Fri, May 01, 2026 at 02:05:21PM +0100, David Laight wrote:

> > Anyway, I had a poke around with godbolt, and the below seems to
> > generate the best code for things like x86_64 and arm64.
> > 
> > Specifically, the __builtin_mul_overflow() already has to compute the
> > 128 bit product anyway for most architectures, so using that directly
> > then leads to saner asm and easier to understand code.
> 
> Older versions of gcc (maybe before 10?) generate a call to the full
> 128 bit multiply (_muldid3 ?) on mips64 - rather than just using the instruction
> that generates the high part of the product.
> The library function is now included but is slightly more complex because
> it does the high*low multiples as well.
> 
> The same happens on sparc64 and the asm multiply code looks strange.

godbolt only has mips64 gcc-9.5 (no 8.1 which is the very oldest gcc we
support) and that doesn't seem to generate calls to library functions.

I don't particularly care about the quality of old compiler output, nor
do I really care for the performance aspect on these fringe
architectures. As long as it builds and isn't obviously broken, things
are good.

Using the gcc-8.5 toolchains from kernel.org (the oldest still supported
set) I see no library calls for MIPS64.

I do see them for Sparc64, but that actually has that library call
implemented.

Anyway, I'll throw it at the robots, see what if anything pops out.

> > diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> > index 9f63b15d309d..f15f4468efe5 100644
> > --- a/kernel/sched/sched.h
> > +++ b/kernel/sched/sched.h
> > @@ -146,7 +146,7 @@ extern struct list_head asym_cap_list;
> >   * Really only required when CONFIG_FAIR_GROUP_SCHED=y is also set, but to
> >   * increase coverage and consistency always enable it on 64-bit platforms.
> >   */
> > -#ifdef CONFIG_64BIT
> > +#if defined(CONFIG_64BIT) && defined(__SIZEOF_INT128__)
> >  # define NICE_0_LOAD_SHIFT	(SCHED_FIXEDPOINT_SHIFT + SCHED_FIXEDPOINT_SHIFT)
> >  # define scale_load(w)		((w) << SCHED_FIXEDPOINT_SHIFT)
> >  # define scale_load_down(w)					\
> > @@ -157,10 +157,12 @@ extern struct list_head asym_cap_list;
> >  		__w = max(2UL, __w >> SCHED_FIXEDPOINT_SHIFT);	\
> >  	__w;							\
> >  })
> > +typedef __int128 sched_double_long_t;
> >  #else
> >  # define NICE_0_LOAD_SHIFT	(SCHED_FIXEDPOINT_SHIFT)
> >  # define scale_load(w)		(w)
> >  # define scale_load_down(w)	(w)
> > +typedef s64 sched_double_long_t;
> 
> Seems strange that this is signed but the 64bit one is unsigned.

They both signed. __int128 is a signed type.