[PATCH] cpufreq: Change link order of x86 cpufreq modules

[PATCH] cpufreq: Change link order of x86 cpufreq modules

[Matthew Garrett]

Change the link order of the cpufreq modules to ensure that they're 
probed in the preferred order when statically linked in.
    
Signed-off-by: Matthew Garrett <mjg@redhat.com>

diff --git a/arch/x86/kernel/cpu/cpufreq/Makefile b/arch/x86/kernel/cpu/cpufreq/Makefile
index 560f776..509296d 100644
--- a/arch/x86/kernel/cpu/cpufreq/Makefile
+++ b/arch/x86/kernel/cpu/cpufreq/Makefile
@@ -1,6 +1,11 @@
+# Link order matters. K8 is preferred to ACPI because of firmware bugs in early
+# K8 systems. ACPI is preferred to all other hardware-specific drivers.
+# speedstep-* is preferred over p4-clockmod.
+
+obj-$(CONFIG_X86_POWERNOW_K8)		+= powernow-k8.o
+obj-$(CONFIG_X86_ACPI_CPUFREQ)		+= acpi-cpufreq.o
 obj-$(CONFIG_X86_POWERNOW_K6)		+= powernow-k6.o
 obj-$(CONFIG_X86_POWERNOW_K7)		+= powernow-k7.o
-obj-$(CONFIG_X86_POWERNOW_K8)		+= powernow-k8.o
 obj-$(CONFIG_X86_LONGHAUL)		+= longhaul.o
 obj-$(CONFIG_X86_E_POWERSAVER)		+= e_powersaver.o
 obj-$(CONFIG_ELAN_CPUFREQ)		+= elanfreq.o
@@ -10,7 +15,6 @@ obj-$(CONFIG_X86_GX_SUSPMOD)		+= gx-suspmod.o
 obj-$(CONFIG_X86_SPEEDSTEP_ICH)		+= speedstep-ich.o
 obj-$(CONFIG_X86_SPEEDSTEP_LIB)		+= speedstep-lib.o
 obj-$(CONFIG_X86_SPEEDSTEP_SMI)		+= speedstep-smi.o
-obj-$(CONFIG_X86_ACPI_CPUFREQ)		+= acpi-cpufreq.o
 obj-$(CONFIG_X86_SPEEDSTEP_CENTRINO)	+= speedstep-centrino.o
 obj-$(CONFIG_X86_P4_CLOCKMOD)		+= p4-clockmod.o
 obj-$(CONFIG_X86_CPUFREQ_NFORCE2)	+= cpufreq-nforce2.o
-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] cpufreq: Change link order of x86 cpufreq modules

[Ingo Molnar]

* Matthew Garrett <mjg59@srcf.ucam.org> wrote:

> Change the link order of the cpufreq modules to ensure that they're 
> probed in the preferred order when statically linked in.
>     
> Signed-off-by: Matthew Garrett <mjg@redhat.com>
> 
> diff --git a/arch/x86/kernel/cpu/cpufreq/Makefile b/arch/x86/kernel/cpu/cpufreq/Makefile
> index 560f776..509296d 100644
> --- a/arch/x86/kernel/cpu/cpufreq/Makefile
> +++ b/arch/x86/kernel/cpu/cpufreq/Makefile
> @@ -1,6 +1,11 @@
> +# Link order matters. K8 is preferred to ACPI because of firmware bugs in early
> +# K8 systems. ACPI is preferred to all other hardware-specific drivers.
> +# speedstep-* is preferred over p4-clockmod.

Nice fix! Where does this information come from? Distro module 
ordering magic? It's rather non-trivial.

(small style nit: the first line in the Makefile is overlong.)

	Ingo

Re: [PATCH] cpufreq: Change link order of x86 cpufreq modules

[Dave Jones]

On Wed, Feb 18, 2009 at 07:31:37PM +0100, Ingo Molnar wrote:
 > 
 > * Matthew Garrett <mjg59@srcf.ucam.org> wrote:
 > 
 > > Change the link order of the cpufreq modules to ensure that they're 
 > > probed in the preferred order when statically linked in.
 > >     
 > > Signed-off-by: Matthew Garrett <mjg@redhat.com>
 > > 
 > > diff --git a/arch/x86/kernel/cpu/cpufreq/Makefile b/arch/x86/kernel/cpu/cpufreq/Makefile
 > > index 560f776..509296d 100644
 > > --- a/arch/x86/kernel/cpu/cpufreq/Makefile
 > > +++ b/arch/x86/kernel/cpu/cpufreq/Makefile
 > > @@ -1,6 +1,11 @@
 > > +# Link order matters. K8 is preferred to ACPI because of firmware bugs in early
 > > +# K8 systems. ACPI is preferred to all other hardware-specific drivers.
 > > +# speedstep-* is preferred over p4-clockmod.
 > 
 > Nice fix! Where does this information come from? Distro module 
 > ordering magic? It's rather non-trivial.

it's a mess of shell script right now.
See /etc/init.d/cpuspeed on Fedora for eg. 

 > (small style nit: the first line in the Makefile is overlong.)
 
I'll fix that up before merging.

Thanks Matthew, queued for .30

	Dave 


-- 
http://www.codemonkey.org.uk

Re: [PATCH] cpufreq: Change link order of x86 cpufreq modules

[Matthew Garrett]

On Wed, Feb 18, 2009 at 07:31:37PM +0100, Ingo Molnar wrote:

> Nice fix! Where does this information come from? Distro module 
> ordering magic? It's rather non-trivial.

Pretty much. p4-clockmod is never the preferred option because it does 
no voltage scaling. speedstep-centrino is now almost entirely 
functionally replaced with acpi-cpufreq. The powernow-k8 issue was a 
personal communication from davej.

> (small style nit: the first line in the Makefile is overlong.)

Seems to be 79 characters?

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] cpufreq: Change link order of x86 cpufreq modules

[Ingo Molnar]

* Matthew Garrett <mjg59@srcf.ucam.org> wrote:

> On Wed, Feb 18, 2009 at 07:31:37PM +0100, Ingo Molnar wrote:
> 
> > Nice fix! Where does this information come from? Distro module 
> > ordering magic? It's rather non-trivial.
> 
> Pretty much. p4-clockmod is never the preferred option because 
> it does no voltage scaling. speedstep-centrino is now almost 
> entirely functionally replaced with acpi-cpufreq. The 
> powernow-k8 issue was a personal communication from davej.

I'm wondering whether that priority order should/could be 
expressed in the module space too - so that distros wouldnt have 
to replicate this. This is really a piece of information the 
kernel is best at maintaining.

> > (small style nit: the first line in the Makefile is overlong.)
> 
> Seems to be 79 characters?

[ oops, my eye-guesstimator is failing me :) ]

	Ingo

Re: [PATCH] cpufreq: Change link order of x86 cpufreq modules

[Alan Jenkins]

On 2/18/09, Ingo Molnar <mingo@elte.hu> wrote:
>
> * Matthew Garrett <mjg59@srcf.ucam.org> wrote:
>
>> On Wed, Feb 18, 2009 at 07:31:37PM +0100, Ingo Molnar wrote:
>>
>> > Nice fix! Where does this information come from? Distro module
>> > ordering magic? It's rather non-trivial.
>>
>> Pretty much. p4-clockmod is never the preferred option because
>> it does no voltage scaling. speedstep-centrino is now almost
>> entirely functionally replaced with acpi-cpufreq. The
>> powernow-k8 issue was a personal communication from davej.
>
> I'm wondering whether that priority order should/could be
> expressed in the module space too - so that distros wouldnt have
> to replicate this. This is really a piece of information the
> kernel is best at maintaining.

The latest development version of module-init-tools (in the git tree)
is designed to preserve the kernel link order when resolving builtin
aliases.  If you have two modules which provide the alias "pci:123",
they will be loaded in the same order as if they were builtin drivers.

It should work if all the cpufreq drivers provide an alias
"cpufreq-driver" and userspace just does "modprobe cpufreq-driver".
You just need to be sure none of the cpufreq drivers provide *other*
aliases which cause them to be loaded earlier, by udev or some other
bootscript.

Alan

Re: [PATCH] cpufreq: Change link order of x86 cpufreq modules

[Ingo Molnar]

* Alan Jenkins <sourcejedi.lkml@googlemail.com> wrote:

> On 2/18/09, Ingo Molnar <mingo@elte.hu> wrote:
> >
> > * Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> >
> >> On Wed, Feb 18, 2009 at 07:31:37PM +0100, Ingo Molnar wrote:
> >>
> >> > Nice fix! Where does this information come from? Distro module
> >> > ordering magic? It's rather non-trivial.
> >>
> >> Pretty much. p4-clockmod is never the preferred option because
> >> it does no voltage scaling. speedstep-centrino is now almost
> >> entirely functionally replaced with acpi-cpufreq. The
> >> powernow-k8 issue was a personal communication from davej.
> >
> > I'm wondering whether that priority order should/could be
> > expressed in the module space too - so that distros wouldnt have
> > to replicate this. This is really a piece of information the
> > kernel is best at maintaining.
> 
> The latest development version of module-init-tools (in the 
> git tree) is designed to preserve the kernel link order when 
> resolving builtin aliases.  If you have two modules which 
> provide the alias "pci:123", they will be loaded in the same 
> order as if they were builtin drivers.

very nifty ...

> It should work if all the cpufreq drivers provide an alias 
> "cpufreq-driver" and userspace just does "modprobe 
> cpufreq-driver". You just need to be sure none of the cpufreq 
> drivers provide *other* aliases which cause them to be loaded 
> earlier, by udev or some other bootscript.

yeah. Nice.

	Ingo

Re: [PATCH] cpufreq: Change link order of x86 cpufreq modules

[Scott James Remnant]

[-- Attachment #1: Type: text/plain, Size: 716 bytes --]

On Wed, 2009-02-18 at 18:36 +0000, Matthew Garrett wrote:

> On Wed, Feb 18, 2009 at 07:31:37PM +0100, Ingo Molnar wrote:
> 
> > Nice fix! Where does this information come from? Distro module 
> > ordering magic? It's rather non-trivial.
> 
> Pretty much. p4-clockmod is never the preferred option because it does 
> no voltage scaling. speedstep-centrino is now almost entirely 
> functionally replaced with acpi-cpufreq. The powernow-k8 issue was a 
> personal communication from davej.
> 
In fact, we've noticed severe regressions with p4-clockmod over simply
having no scaling driver at all - and are not going to built it into our
kernels.

Scott
-- 
Scott James Remnant
scott@canonical.com

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] cpufreq: Change link order of x86 cpufreq modules

[Matthew Garrett]

On Fri, Feb 20, 2009 at 05:29:52PM +0000, Scott James Remnant wrote:

> In fact, we've noticed severe regressions with p4-clockmod over simply
> having no scaling driver at all - and are not going to built it into our
> kernels.

It makes sense to have p4-clockmod from a thermal management 
perspective. We should probably bump its transition latency to more than 
10ms to prevent ondemand binding to it.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] cpufreq: Change link order of x86 cpufreq modules

[Scott James Remnant]

[-- Attachment #1: Type: text/plain, Size: 690 bytes --]

On Fri, 2009-02-20 at 17:36 +0000, Matthew Garrett wrote:

> On Fri, Feb 20, 2009 at 05:29:52PM +0000, Scott James Remnant wrote:
> 
> > In fact, we've noticed severe regressions with p4-clockmod over simply
> > having no scaling driver at all - and are not going to built it into our
> > kernels.
> 
> It makes sense to have p4-clockmod from a thermal management 
> perspective. We should probably bump its transition latency to more than 
> 10ms to prevent ondemand binding to it.
> 
If that's possible; that'd be good.

The trouble with it is that it never seems to bring the CPU anywhere
near maximum performance.

Scott
-- 
Scott James Remnant
scott@canonical.com

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] cpufreq: Change link order of x86 cpufreq modules

[Dave Jones]

On Fri, Feb 20, 2009 at 05:39:35PM +0000, Scott James Remnant wrote:
 > On Fri, 2009-02-20 at 17:36 +0000, Matthew Garrett wrote:
 > 
 > > On Fri, Feb 20, 2009 at 05:29:52PM +0000, Scott James Remnant wrote:
 > > 
 > > > In fact, we've noticed severe regressions with p4-clockmod over simply
 > > > having no scaling driver at all - and are not going to built it into our
 > > > kernels.
 > > 
 > > It makes sense to have p4-clockmod from a thermal management 
 > > perspective. We should probably bump its transition latency to more than 
 > > 10ms to prevent ondemand binding to it.
 > > 
 > If that's possible; that'd be good.
 > 
 > The trouble with it is that it never seems to bring the CPU anywhere
 > near maximum performance.

This is one reason why in .30 the user interface for p4-clockmod is disabled.
It'll only get throttled when ACPI goes into OMG I'M OVERHEATING mode,
and ramp back up once it cools off.

p4-clockmod and ondemand is a recipe for fail.

	Dave

-- 
http://www.codemonkey.org.uk

Re: [PATCH] cpufreq: Change link order of x86 cpufreq modules

[Thomas Renninger]

On Thursday 19 February 2009 12:54:06 pm Alan Jenkins wrote:
> On 2/18/09, Ingo Molnar <mingo@elte.hu> wrote:
> > * Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> >> On Wed, Feb 18, 2009 at 07:31:37PM +0100, Ingo Molnar wrote:
> >> > Nice fix! Where does this information come from? Distro module
> >> > ordering magic? It's rather non-trivial.
> >>
> >> Pretty much. p4-clockmod is never the preferred option because
> >> it does no voltage scaling. speedstep-centrino is now almost
> >> entirely functionally replaced with acpi-cpufreq. The
> >> powernow-k8 issue was a personal communication from davej.
> >
> > I'm wondering whether that priority order should/could be
> > expressed in the module space too - so that distros wouldnt have
> > to replicate this. This is really a piece of information the
> > kernel is best at maintaining.
>
> The latest development version of module-init-tools (in the git tree)
> is designed to preserve the kernel link order when resolving builtin
> aliases.  If you have two modules which provide the alias "pci:123",
> they will be loaded in the same order as if they were builtin drivers.
>
> It should work if all the cpufreq drivers provide an alias
> "cpufreq-driver" and userspace just does "modprobe cpufreq-driver".
> You just need to be sure none of the cpufreq drivers provide *other*
> aliases which cause them to be loaded earlier, by udev or some other
> bootscript.
I wonder whether a "cpu:vendor:fam:model:flags:..." alias makes sense.
This would solve autoloading of most cpufreq drivers, but it never looked
important enough.
Other drivers that could make use of this:
  - k8temp/k10temp (Don't know about this one, but AFAIK there exists an
    MSR to read out temperature on latest AMD for which a hwmon driver
    is/should be written)?
  - Microcode update?
  - more?

     Thomas

Re: [PATCH] cpufreq: Change link order of x86 cpufreq modules

[Kay Sievers]

On Mon, Feb 23, 2009 at 22:12, Thomas Renninger <trenn@suse.de> wrote:
> On Thursday 19 February 2009 12:54:06 pm Alan Jenkins wrote:
>> On 2/18/09, Ingo Molnar <mingo@elte.hu> wrote:
>> > * Matthew Garrett <mjg59@srcf.ucam.org> wrote:
>> >> On Wed, Feb 18, 2009 at 07:31:37PM +0100, Ingo Molnar wrote:
>> >> > Nice fix! Where does this information come from? Distro module
>> >> > ordering magic? It's rather non-trivial.
>> >>
>> >> Pretty much. p4-clockmod is never the preferred option because
>> >> it does no voltage scaling. speedstep-centrino is now almost
>> >> entirely functionally replaced with acpi-cpufreq. The
>> >> powernow-k8 issue was a personal communication from davej.
>> >
>> > I'm wondering whether that priority order should/could be
>> > expressed in the module space too - so that distros wouldnt have
>> > to replicate this. This is really a piece of information the
>> > kernel is best at maintaining.
>>
>> The latest development version of module-init-tools (in the git tree)
>> is designed to preserve the kernel link order when resolving builtin
>> aliases.  If you have two modules which provide the alias "pci:123",
>> they will be loaded in the same order as if they were builtin drivers.
>>
>> It should work if all the cpufreq drivers provide an alias
>> "cpufreq-driver" and userspace just does "modprobe cpufreq-driver".
>> You just need to be sure none of the cpufreq drivers provide *other*
>> aliases which cause them to be loaded earlier, by udev or some other
>> bootscript.
> I wonder whether a "cpu:vendor:fam:model:flags:..." alias makes sense.
> This would solve autoloading of most cpufreq drivers, but it never looked
> important enough.
> Other drivers that could make use of this:
>  - k8temp/k10temp (Don't know about this one, but AFAIK there exists an
>    MSR to read out temperature on latest AMD for which a hwmon driver
>    is/should be written)?
>  - Microcode update?
>  - more?

I thought about something similar, because distros probably do not
want to load all modules shipped with a single alias, but only the
ones match matching the actual hardware.

Unfortunately, the stuff in /sys/devices/system/cpu/ can not be used
to export a modalias until it is converted to proper buses and "struct
device"s.

If we come up with an extensible modalias string, the system boot
scripts can compose, and call modprobe to load cpufreq modules, it
sounds like a good thing to do.

Some day, the properly converted /sys/devices/system/ devices will
work, and can trigger the auto-loading out-of-the-box. If possible,
the modaliases added to the modules could be directly used for the
future system devices.

Thanks,
Kay