Re: [PATCH v1] sched: idle: Make skipping governor callbacks more consistent

[Qais Yousef]

On 03/09/26 13:26, Rafael J. Wysocki wrote:
> On Mon, Mar 9, 2026 at 10:13 AM Christian Loehle
> <christian.loehle@xxxxxxx> wrote:
> >
> > On 3/7/26 16:12, Rafael J. Wysocki wrote:
> > > From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> > >
> > > If the cpuidle governor .select() callback is skipped because there
> > > is only one idle state in the cpuidle driver, the .reflect() callback
> > > should be skipped as well, at least for consistency (if not for
> > > correctness), so do it.
> > >
> > > Fixes: e5c9ffc6ae1b ("cpuidle: Skip governor when only one idle state is available")
> > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> > > ---
> > >  drivers/cpuidle/cpuidle.c |   10 ----------
> > >  kernel/sched/idle.c       |   11 ++++++++++-
> > >  2 files changed, 10 insertions(+), 11 deletions(-)
> > >
> > > --- a/drivers/cpuidle/cpuidle.c
> > > +++ b/drivers/cpuidle/cpuidle.c
> > > @@ -359,16 +359,6 @@ noinstr int cpuidle_enter_state(struct c
> > >  int cpuidle_select(struct cpuidle_driver *drv, struct cpuidle_device *dev,
> > >                  bool *stop_tick)
> > >  {
> > > -     /*
> > > -      * If there is only a single idle state (or none), there is nothing
> > > -      * meaningful for the governor to choose. Skip the governor and
> > > -      * always use state 0 with the tick running.
> > > -      */
> > > -     if (drv->state_count <= 1) {
> > > -             *stop_tick = false;
> > > -             return 0;
> > > -     }
> > > -
> > >       return cpuidle_curr_governor->select(drv, dev, stop_tick);
> > >  }
> > >
> > > --- a/kernel/sched/idle.c
> > > +++ b/kernel/sched/idle.c
> > > @@ -221,7 +221,7 @@ static void cpuidle_idle_call(void)
> > >
> > >               next_state = cpuidle_find_deepest_state(drv, dev, max_latency_ns);
> > >               call_cpuidle(drv, dev, next_state);
> > > -     } else {
> > > +     } else if (drv->state_count > 1) {
> > >               bool stop_tick = true;
> > >
> > >               /*
> > > @@ -239,6 +239,15 @@ static void cpuidle_idle_call(void)
> > >                * Give the governor an opportunity to reflect on the outcome
> > >                */
> > >               cpuidle_reflect(dev, entered_state);
> > > +     } else {
> > > +             tick_nohz_idle_retain_tick();
> > > +
> > > +             /*
> > > +              * If there is only a single idle state (or none), there is
> > > +              * nothing meaningful for the governor to choose.  Skip the
> > > +              * governor and always use state 0.
> > > +              */
> > > +             call_cpuidle(drv, dev, 0);
> > >       }
> > >
> > >  exit_idle:
> > >
> > >
> > >
> >
> > Duh, good catch.
> > Reviewed-by: Christian Loehle <christian.loehle@xxxxxxx>
> 
> OK, so any objections or concerns from anyone?
> 
> I'm about to queue this up for the next -rc.

LGTM too