[BUG?] Moving drivers to drivers/cpufreq/ causes all to be loaded

[BUG?] Moving drivers to drivers/cpufreq/ causes all to be loaded

[Jonathan Nieder]

(-cc: linux-arm-kernel, +cc: linux-kernel, Mattia)
Hi,

Dave Jones wrote:

> in cpufreq.next I moved the x86 drivers over.  Someone look it over ?

Some people[1] have been seeing regressions after this change (when
moving from 2.6.39 to 3.0, presumably from commit bb0a56ecc4ba,
"[CPUFREQ] Move x86 drivers to drivers/cpufreq/").  The first symptom
was messages at boot:

| Loading cpufreq kernel modules...FATAL: Error inserting powernow_k7 (/lib/modules/3.0.0-1-486/kernel/drivers/cpufreq/powernow-k7.ko): No such device
| FATAL: Error inserting speedstep_ich (/lib/modules/3.0.0-1-486/kernel/drivers/cpufreq/speedstep-ich.ko): No such device
| FATAL: Error inserting acpi_cpufreq (/lib/modules/3.0.0-1-486/kernel/drivers/cpufreq/acpi-cpufreq.ko): Device or resource busy
[etc]

The second symptom was the wrong cpufreq driver being loaded
(p4-clockmod instead of acpi-cpufreq).  The cause seems to be some
code in init scripts that originated in powernowd 0.97-2ubuntu1 (2007)
or some time before that:

|         #get list of available modules (governors and helpers)
|         LOC="/lib/modules/$(uname -r)/kernel/drivers/cpufreq"
|         if [ -d $LOC ]; then
|           MODAVAIL=$( ( find $LOC -type f -name "*.o" -printf "basename %f .o\n"; \
|               find $LOC -type f -name "*.ko" -printf "basename %f .ko\n" ) | /bin/sh)
|         else
|           MODAVAIL=""
|         fi
| 
|         #echo "Loading cpufreq modules:"
|         for mod in $MODAVAIL; do
|         #        echo "     $mod"
|                 echo $LIST| grep -q -w "$mod" || modprobe $mod >/dev/null || /bin/true
|         done

This takes all kernel modules in drivers/cpufreq, blindly assumes
they must be governors or helpers, and loads them.  Nowadays it is
in the loadcpufreq script in cpufrequtils; so in cpufrequtils 007-2
(03 Aug 2011), the pattern changed to drivers/cpufreq/cpufreq_*.ko
which just matches the governors and helpers and everybody's happy.

Except:

 (1) This is still incredibly fragile.  What *should* cpufrequtils
     be doing to get the drivers it needs?

 (2) Using the 3.0 or later kernel with old userspace gives bad
     results (e.g., 30% increase in power consumption for one
     reporter).  That's a regression.  Bad kernel, no biscuit.

Ideas?

[1] http://bugs.debian.org/635348

Re: [BUG?] Moving drivers to drivers/cpufreq/ causes all to be loaded

[Jonathan Nieder]

Jonathan Nieder wrote:

>  (1) This is still incredibly fragile.  What *should* cpufrequtils
>      be doing to get the modules it needs?
>
>  (2) Using the 3.0 or later kernel with old userspace gives bad
>      results (e.g., 30% increase in power consumption for one
>      reporter).  That's a regression.

The "30% increase" part was an unrelated bug (i915.i915_enable_rc6=1
brings power consumption back to normal), for those who were
wondering. :)

Old userspace automatically loading the wrong cpufreq drivers still
does not seem great to me, though I don't have any great ideas about
how to prevent that (a separate drivers/cpufreq-drivers/ directory
does not sound too appealing).  I guess I'd be most interested in how
to fix (1) first.

Thanks,
Jonathan

Re: [BUG?] Moving drivers to drivers/cpufreq/ causes all to be loaded

[Dave Jones]

On Sat, Aug 13, 2011 at 02:02:46PM -0500, Jonathan Nieder wrote:
 > Jonathan Nieder wrote:
 > 
 > >  (1) This is still incredibly fragile.  What *should* cpufrequtils
 > >      be doing to get the modules it needs?
 > >
 > >  (2) Using the 3.0 or later kernel with old userspace gives bad
 > >      results (e.g., 30% increase in power consumption for one
 > >      reporter).  That's a regression.
 > 
 > The "30% increase" part was an unrelated bug (i915.i915_enable_rc6=1
 > brings power consumption back to normal), for those who were
 > wondering. :)
 > 
 > Old userspace automatically loading the wrong cpufreq drivers still
 > does not seem great to me, though I don't have any great ideas about
 > how to prevent that (a separate drivers/cpufreq-drivers/ directory
 > does not sound too appealing).  I guess I'd be most interested in how
 > to fix (1) first.

If we have to move stuff again, we could do drivers/cpufreq/x86/ etc..
Even if we do that though, you really want to fix that userspace, because
you're right that "load everything and see what sticks" is fragile,
and pure luck that it ever did the right thing.

What we used to do in Fedora grew more and more complex over time.
Here's the last incarnation: http://fpaste.org/uvDb/
As you can see in the start() function, it's pretty hairy, and even
that doesn't cover every possible case, which is why it allows a user
override in the $DRIVER variable. Messy.

Basic thinking is
- If AMD64, load powernow-k8
- if that fails to load : try acpi-cpufreq
- For everything else : acpi-cpufreq
- If acpi-cpufreq fails : p4-clockmod

What we're moving towards for Fedora 16 is to change all the drivers
to be linked in rather than modular, and rely on link order to do the right thing.
It sounds like a step backwards in some ways, towards the 'see what sticks'
approach, but the difference here is the link-order specifying the correct
order for initialisation. You get none of that with modules.

In an ideal world, we'd auto-load the right module on a hotplug event
from a cpu, but we're not there yet. I believe Kay is working on that.

Over time, 'the right driver' seems to be converging on acpi-cpufreq.
There's some patches pending to even move some of the powernow-k8 use-cases
to use acpi-cpufreq instead. But modern intel should be using that,
(where modern = almost everything since p4, except those that lack P-states)


And then there's the 'which governor' mess.
I'd really like that to eventually converge so that 'ondemand' is always the
right answer.  For this to happen, there needs to be no difference between
an idle machine running powersave, and ondemand or (harder) a busy machine
running performance.
(conservative with some work could be just a runtime mode for ondemand).

	Dave

Re: [BUG?] Moving drivers to drivers/cpufreq/ causes all to be loaded

[Mattia Dongili]

On Sat, Aug 13, 2011 at 05:11:42PM -0400, Dave Jones wrote:
> On Sat, Aug 13, 2011 at 02:02:46PM -0500, Jonathan Nieder wrote:
>  > Jonathan Nieder wrote:
>  > 
>  > >  (1) This is still incredibly fragile.  What *should* cpufrequtils
>  > >      be doing to get the modules it needs?
>  > >
>  > >  (2) Using the 3.0 or later kernel with old userspace gives bad
>  > >      results (e.g., 30% increase in power consumption for one
>  > >      reporter).  That's a regression.
>  > 
>  > The "30% increase" part was an unrelated bug (i915.i915_enable_rc6=1
>  > brings power consumption back to normal), for those who were
>  > wondering. :)
>  > 
>  > Old userspace automatically loading the wrong cpufreq drivers still
>  > does not seem great to me, though I don't have any great ideas about
>  > how to prevent that (a separate drivers/cpufreq-drivers/ directory
>  > does not sound too appealing).  I guess I'd be most interested in how
>  > to fix (1) first.
> 
> If we have to move stuff again, we could do drivers/cpufreq/x86/ etc..
> Even if we do that though, you really want to fix that userspace, because
> you're right that "load everything and see what sticks" is fragile,
> and pure luck that it ever did the right thing.

not sure why this bug landed here finally, it was clearly an
overlook in the Debian startup script and it's only specific to Debian.

The "load everything" part was not for cpu drivers but for governors
and helpers that used to sit into drivers/cpufreq alone.
The cpu driver loading part is fairly complex (or yes, messy as you say)
and not too dissimilar than the one from fedora.

-- 
mattia
:wq!

Re: [BUG?] Moving drivers to drivers/cpufreq/ causes all to be loaded

[Jonathan Nieder]

(-cc: ARM/etc people; +cc: Kay)

Dave Jones wrote:

> If we have to move stuff again, we could do drivers/cpufreq/x86/ etc..

Unfortunately the old script used "find", not "ls", so that wouldn't
work. :/

> What we used to do in Fedora grew more and more complex over time.
> Here's the last incarnation: http://fpaste.org/uvDb/

The main difference from Debian seems to be that this script loads the
module corresponding to the chosen governor, while Debian's init
script loads all governor modules early (using a heuristic I would
like to avoid that involves running "find" to list them) and chooses
the governor to use later.

> In an ideal world, we'd auto-load the right module on a hotplug event
> from a cpu, but we're not there yet. I believe Kay is working on that.

Yes, that is what I was hoping for.  Are there patches to test?

The comment at
http://thread.gmane.org/gmane.linux.kernel/796450/focus=796874 also
looks promising.

Thanks much for the help,
Jonathan

Re: [BUG?] Moving drivers to drivers/cpufreq/ causes all to be loaded

[Kay Sievers]

On Sun, Aug 14, 2011 at 19:01, Jonathan Nieder <jrnieder@gmail.com> wrote:
> (-cc: ARM/etc people; +cc: Kay)
>
> Dave Jones wrote:
>
>> If we have to move stuff again, we could do drivers/cpufreq/x86/ etc..
>
> Unfortunately the old script used "find", not "ls", so that wouldn't
> work. :/
>
>> What we used to do in Fedora grew more and more complex over time.
>> Here's the last incarnation: http://fpaste.org/uvDb/
>
> The main difference from Debian seems to be that this script loads the
> module corresponding to the chosen governor, while Debian's init
> script loads all governor modules early (using a heuristic I would
> like to avoid that involves running "find" to list them) and chooses
> the governor to use later.

Right, loading cpufreq drivers from userspace is fragile, and can not
properly work today. Only the kernel itself know which driver to try
to bind in which order. Modular cpufreq kernel modules make no real
sense here with the infrastructure we have available today.

>> In an ideal world, we'd auto-load the right module on a hotplug event
>> from a cpu, but we're not there yet. I believe Kay is working on that.
>
> Yes, that is what I was hoping for.  Are there patches to test?

Yeah, it's on the TODO list, I just get too many 'really broken'
things get sorted on top all time. :) But it will happen in the next
months.

> The comment at
> http://thread.gmane.org/gmane.linux.kernel/796450/focus=796874 also
> looks promising.

That could work, but we better go right for the CPU specific aliases
and do not try to load all of them, even when for completely different
systems. That would just encode another workaround into kernel
modules, which we better solve properly.

Kay