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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20201109123416.GO2594@hirez.programming.kicks-ass.net>
Date:   Mon, 9 Nov 2020 13:34:16 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     "Rafael J. Wysocki" <rafael@...nel.org>
Cc:     Daniel Lezcano <daniel.lezcano@...aro.org>,
        "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux PM <linux-pm@...r.kernel.org>,
        Lina Iyer <ilina@...eaurora.org>,
        Frederic Weisbecker <frederic@...nel.org>
Subject: Re: [PATCH 4/5] cpuidle: governor: Export the needed symbols

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.