Re: [PATCH 2/2] sched/fair: Randomize equally shallow slow-path candidates

[Vincent Guittot]

On Thu, 17 Sept 2026 at 11:58, Christian Loehle
<christian.loehle@xxxxxxx> wrote:
>
> On 9/17/26 08:47, Christian Loehle wrote:
> > On 9/16/26 20:43, Shubhang wrote:
> >> Hi Christian,
> >>
> >> On Wed, 16 Sep 2026, Christian Loehle wrote:
> >>
> >>> +            } else if (!idle || idle->exit_latency == min_exit_latency) {
> >>> +                nr_candidates++;
> >>> +                if (nr_candidates == 1 ||
> >>> +                    !reciprocal_scale(sched_rng(), nr_candidates))
> >>> +                    shallowest_idle_cpu = i;
> >>
> >> available_idle_cpu(i) ensures that this is an idle CPU, but !idle means that no active cpuidle state, meaning no exit latency is available for comparison.
> >>
> >> [PATCH 1/2] treats such a CPU as a fallback i.e. it is selected only when no idle candidate has been found yet. Here it becomes an equal reservoir candidate, even after selecting a CPU with the minimum known exit latency. That is, it is added to the random selection pool and can replace shallowest_idle_cpu.
> >>
> >> Is that intentional ? If not, should reservoir sampling be limited to
> >> candidates with `idle->exit_latency == min_exit_latency`, while retaining the first !idle CPU only as the fallback?
> >
> > Hi Shubhang,
> >
> > Thanks for taking a look. Including !idle candidates was intentional,
> > although you're right that this changes their treatment.
> >
> > I think there's a case for giving NULL a zero ranking. With a working
> > cpuidle driver, NULL can mean the CPU is preparing for entry or finishing
> > after exit, making it a good low-latency candidate.
> > Architecture fallbacks are also reasonable candidates AFAICS.
> > In any case, I'd prefer to address that separately (perhaps through a
> > helper in the idle/cpuidle code rather than embedding those assumptions
> > in fair.c and will drop !idle CPUs competing with min_exit_latency CPUs,
> > the patch doesn't need it.
> Actually about the last part, I'm leaning towards sticking with v1, because
> that is actually the current upstream behaviour. !idle CPUs currently trump
> any CPU seen so far, but preserve min_exit_latency, therefore sticking it
> into the shallowest seen reservoir is consistent IMO (although we can
> certainly debate if that should be the case).
> I'd prefer sending a follow-up with an idle helper and making a properly
> defined policy there, but that's adjacent to this patch.

When a CPU idle driver is present, !idle means the CPU is entering or
leaving an idle state. A CPU entering idle could be a good candidate
but we don't want to select the one exiting its idle state, as it is
already scheduled for activity. The current version is trying to do so
with idle_stamp comparison although this is far from being robust.
That being said, a random choice might not be worse.