Re: [PATCH 4/5] cpuidle: governor: Export the needed symbols

[Peter Zijlstra <peterz@infradead.org>]

On Thu, Nov 05, 2020 at 03:04:10PM +0100, Rafael J. Wysocki wrote:
> > diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> > index 2d95dc3f4644..ceba61bb364d 100644
> > --- a/kernel/sched/core.c
> > +++ b/kernel/sched/core.c
> > @@ -3838,6 +3838,7 @@ unsigned long nr_iowait_cpu(int cpu)
> >  {
> >         return atomic_read(&cpu_rq(cpu)->nr_iowait);
> >  }
> > +EXPORT_SYMBOL_GPL(nr_iowait_cpu);
> 
> Hmm.  See below.

Did anyone read the comment above this function? It's garbage, it should
be deleted, not made available to a wider audience.

> >  /*
> >   * IO-wait accounting, and how its mostly bollocks (on SMP).
> > diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
> > index f0199a4ba1ad..537716124d46 100644
> > --- a/kernel/time/tick-sched.c
> > +++ b/kernel/time/tick-sched.c
> > @@ -500,12 +500,19 @@ static int __init setup_tick_nohz(char *str)
> >
> >  __setup("nohz=", setup_tick_nohz);
> >
> > +bool tick_nohz_is_enabled(void)
> > +{
> > +       return tick_nohz_enabled;
> > +}
> > +EXPORT_SYMBOL_GPL(tick_nohz_is_enabled);
> > +
> >  bool tick_nohz_tick_stopped(void)
> >  {
> >         struct tick_sched *ts = this_cpu_ptr(&tick_cpu_sched);
> >
> >         return ts->tick_stopped;
> >  }
> > +EXPORT_SYMBOL_GPL(tick_nohz_tick_stopped);
> >
> >  bool tick_nohz_tick_stopped_cpu(int cpu)
> >  {
> > @@ -1066,6 +1073,7 @@ bool tick_nohz_idle_got_tick(void)
> >         }
> >         return false;
> >  }
> > +EXPORT_SYMBOL_GPL(tick_nohz_idle_got_tick);
> >
> >  /**
> >   * tick_nohz_get_next_hrtimer - return the next expiration time for the hrtimer
> > @@ -1117,6 +1125,7 @@ ktime_t tick_nohz_get_sleep_length(ktime_t *delta_next)
> >
> >         return ktime_sub(next_event, now);
> >  }
> > +EXPORT_SYMBOL_GPL(tick_nohz_get_sleep_length);
> 
> Peter please correct me if I'm mistaken, but IMV the above are core
> kernel internals and they should not be accessible to random modular
> stuff.

Yeah,... making this available seems unfortunate. Also, I don't really
see the point, why do we want the idle governors as modules? On the
cpufreq side we're trying to move away from modules and multiple
governors.