[Ubuntu PATCH] cpufreq: fix powernow-k8 load bug

[Ubuntu PATCH] cpufreq: fix powernow-k8 load bug

[Randy Dunlap]

Fix powernow-k8 doesn't load bug.
Reference: https://launchpad.net/distros/ubuntu/+source/linux-source-2.6.15/+bug/35145

http://www.kernel.org/git/?p=linux/kernel/git/bcollins/ubuntu-dapper.git;a=commitdiff;h=dce0ca36f2ae348f005735e9acd400d2c0954421



---
 arch/i386/kernel/cpu/cpufreq/powernow-k8.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- linux-2617-pv.orig/arch/i386/kernel/cpu/cpufreq/powernow-k8.c
+++ linux-2617-pv/arch/i386/kernel/cpu/cpufreq/powernow-k8.c
@@ -1008,7 +1008,7 @@ static int __cpuinit powernowk8_cpu_init
 		 * an UP version, and is deprecated by AMD.
 		 */
 
-		if ((num_online_cpus() != 1) || (num_possible_cpus() != 1)) {
+		if ((num_online_cpus() != 1)) {
 			printk(KERN_ERR PFX "MP systems not supported by PSB BIOS structure\n");
 			kfree(data);
 			return -ENODEV;

Re: [Ubuntu PATCH] cpufreq: fix powernow-k8 load bug

[Dave Jones]

On Tue, Jun 20, 2006 at 10:32:56PM -0700, Randy Dunlap wrote:
 > Fix powernow-k8 doesn't load bug.
 > Reference: https://launchpad.net/distros/ubuntu/+source/linux-source-2.6.15/+bug/35145
 > 
 > http://www.kernel.org/git/?p=linux/kernel/git/bcollins/ubuntu-dapper.git;a=commitdiff;h=dce0ca36f2ae348f005735e9acd400d2c0954421

Already fixed in -git1

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

Re: [Ubuntu PATCH] cpufreq: fix powernow-k8 load bug

[Dave Jones]

On Wed, Jun 21, 2006 at 01:41:44AM -0400, Dave Jones wrote:
 > On Tue, Jun 20, 2006 at 10:32:56PM -0700, Randy Dunlap wrote:
 >  > Fix powernow-k8 doesn't load bug.
 >  > Reference: https://launchpad.net/distros/ubuntu/+source/linux-source-2.6.15/+bug/35145
 >  > 
 >  > http://www.kernel.org/git/?p=linux/kernel/git/bcollins/ubuntu-dapper.git;a=commitdiff;h=dce0ca36f2ae348f005735e9acd400d2c0954421
 > 
 > Already fixed in -git1

My bad, I mixed it up with the similar fix for powernow-k7.
I'll queue this up tomorrow morning.

thanks,

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

Re: [Ubuntu PATCH] cpufreq: fix powernow-k8 load bug

[Pavel Machek]

Hi!

> Fix powernow-k8 doesn't load bug.
> Reference: https://launchpad.net/distros/ubuntu/+source/linux-source-2.6.15/+bug/35145
> 
> http://www.kernel.org/git/?p=linux/kernel/git/bcollins/ubuntu-dapper.git;a=commitdiff;h=dce0ca36f2ae348f005735e9acd400d2c0954421
> 
> 
> 
> ---
>  arch/i386/kernel/cpu/cpufreq/powernow-k8.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- linux-2617-pv.orig/arch/i386/kernel/cpu/cpufreq/powernow-k8.c
> +++ linux-2617-pv/arch/i386/kernel/cpu/cpufreq/powernow-k8.c
> @@ -1008,7 +1008,7 @@ static int __cpuinit powernowk8_cpu_init
>  		 * an UP version, and is deprecated by AMD.
>  		 */
>  
> -		if ((num_online_cpus() != 1) || (num_possible_cpus() != 1)) {
> +		if ((num_online_cpus() != 1)) {
>  			printk(KERN_ERR PFX "MP systems not supported by PSB BIOS structure\n");
>  			kfree(data);
>  			return -ENODEV;
> 

Seems wrong to me... what if I boot, then hotplug second cpu?
-- 
64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms         

Re: [Ubuntu PATCH] cpufreq: fix powernow-k8 load bug

[Dave Jones]

On Thu, Jun 22, 2006 at 10:38:56PM +0200, Pavel Machek wrote:
 > > --- linux-2617-pv.orig/arch/i386/kernel/cpu/cpufreq/powernow-k8.c
 > > +++ linux-2617-pv/arch/i386/kernel/cpu/cpufreq/powernow-k8.c
 > > @@ -1008,7 +1008,7 @@ static int __cpuinit powernowk8_cpu_init
 > >  		 * an UP version, and is deprecated by AMD.
 > >  		 */
 > >  
 > > -		if ((num_online_cpus() != 1) || (num_possible_cpus() != 1)) {
 > > +		if ((num_online_cpus() != 1)) {
 > >  			printk(KERN_ERR PFX "MP systems not supported by PSB BIOS structure\n");
 > >  			kfree(data);
 > >  			return -ENODEV;
 > > 
 > 
 > Seems wrong to me... what if I boot, then hotplug second cpu?

We only run this code if powernow_k8_cpu_init_acpi() has failed,
which it should never do on an SMP system.

So, you get exactly the same behaviour, as expected.
You can't support >1 CPU with PSB.

The above patch makes sure things continue to work if you run
an SMP kernel on UP hardware.

		Dave

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

Re: [Ubuntu PATCH] cpufreq: fix powernow-k8 load bug

[Pavel Machek]

Hi!

>  > > --- linux-2617-pv.orig/arch/i386/kernel/cpu/cpufreq/powernow-k8.c
>  > > +++ linux-2617-pv/arch/i386/kernel/cpu/cpufreq/powernow-k8.c
>  > > @@ -1008,7 +1008,7 @@ static int __cpuinit powernowk8_cpu_init
>  > >  		 * an UP version, and is deprecated by AMD.
>  > >  		 */
>  > >  
>  > > -		if ((num_online_cpus() != 1) || (num_possible_cpus() != 1)) {
>  > > +		if ((num_online_cpus() != 1)) {
>  > >  			printk(KERN_ERR PFX "MP systems not supported by PSB BIOS structure\n");
>  > >  			kfree(data);
>  > >  			return -ENODEV;
>  > > 
>  > 
>  > Seems wrong to me... what if I boot, then hotplug second cpu?
> 
> We only run this code if powernow_k8_cpu_init_acpi() has failed,
> which it should never do on an SMP system.
> 
> So, you get exactly the same behaviour, as expected.
> You can't support >1 CPU with PSB.
> 
> The above patch makes sure things continue to work if you run
> an SMP kernel on UP hardware.

I'm pretty sure you'll find SMP machine with broken ACPI cpufreq
(therefore cpu_init_acpi() will fail). And if user is perverse enough,
he might boot with one cpu then simulate hotplug of second one.

OTOH:

1) user is already doing perverse things at this point

and

2) machine BIOS is b0rken

...so... it is only theoretical and probably not worth fixing.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [Ubuntu PATCH] cpufreq: fix powernow-k8 load bug

[Dave Jones]

On Fri, Jun 23, 2006 at 02:46:56PM +0200, Pavel Machek wrote:

 > >  > > --- linux-2617-pv.orig/arch/i386/kernel/cpu/cpufreq/powernow-k8.c
 > >  > > +++ linux-2617-pv/arch/i386/kernel/cpu/cpufreq/powernow-k8.c
 > >  > > @@ -1008,7 +1008,7 @@ static int __cpuinit powernowk8_cpu_init
 > >  > >  		 * an UP version, and is deprecated by AMD.
 > >  > >  		 */
 > >  > >  
 > >  > > -		if ((num_online_cpus() != 1) || (num_possible_cpus() != 1)) {
 > >  > > +		if ((num_online_cpus() != 1)) {
 > >  > >  			printk(KERN_ERR PFX "MP systems not supported by PSB BIOS structure\n");
 > >  > >  			kfree(data);
 > >  > >  			return -ENODEV;
 > >  > > 
 > >  > 
 > >  > Seems wrong to me... what if I boot, then hotplug second cpu?
 > > 
 > > We only run this code if powernow_k8_cpu_init_acpi() has failed,
 > > which it should never do on an SMP system.
 > > 
 > > So, you get exactly the same behaviour, as expected.
 > > You can't support >1 CPU with PSB.
 > > 
 > > The above patch makes sure things continue to work if you run
 > > an SMP kernel on UP hardware.
 > 
 > I'm pretty sure you'll find SMP machine with broken ACPI cpufreq
 > (therefore cpu_init_acpi() will fail).

So? It fails now, it'll fail after this patch.
That's not the situation that this patch is changing.

If you have >1 CPU, you *must* have a working ACPI BIOS.

 > he might boot with one cpu then simulate hotplug of second one.
 > 
 > 1) user is already doing perverse things at this point
 > 
 > and
 > 
 > 2) machine BIOS is b0rken
 > 
 > ...so... it is only theoretical and probably not worth fixing.

SMP kernel on UP hardware is not theoretical, and is worth fixing.

		Dave

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

Re: [Ubuntu PATCH] cpufreq: fix powernow-k8 load bug

[Pavel Machek]

On Fri 2006-06-23 09:04:14, Dave Jones wrote:
> On Fri, Jun 23, 2006 at 02:46:56PM +0200, Pavel Machek wrote:
> 
>  > >  > > --- linux-2617-pv.orig/arch/i386/kernel/cpu/cpufreq/powernow-k8.c
>  > >  > > +++ linux-2617-pv/arch/i386/kernel/cpu/cpufreq/powernow-k8.c
>  > >  > > @@ -1008,7 +1008,7 @@ static int __cpuinit powernowk8_cpu_init
>  > >  > >  		 * an UP version, and is deprecated by AMD.
>  > >  > >  		 */
>  > >  > >  
>  > >  > > -		if ((num_online_cpus() != 1) || (num_possible_cpus() != 1)) {
>  > >  > > +		if ((num_online_cpus() != 1)) {
>  > >  > >  			printk(KERN_ERR PFX "MP systems not supported by PSB BIOS structure\n");
>  > >  > >  			kfree(data);
>  > >  > >  			return -ENODEV;
>  > >  > > 
>  > >  > 
>  > >  > Seems wrong to me... what if I boot, then hotplug second cpu?
>  > > 
>  > > We only run this code if powernow_k8_cpu_init_acpi() has failed,
>  > > which it should never do on an SMP system.
>  > > 
>  > > So, you get exactly the same behaviour, as expected.
>  > > You can't support >1 CPU with PSB.
>  > > 
>  > > The above patch makes sure things continue to work if you run
>  > > an SMP kernel on UP hardware.
>  > 
>  > I'm pretty sure you'll find SMP machine with broken ACPI cpufreq
>  > (therefore cpu_init_acpi() will fail).
> 
> So? It fails now, it'll fail after this patch.
> That's not the situation that this patch is changing.
> 
> If you have >1 CPU, you *must* have a working ACPI BIOS.
> 
>  > he might boot with one cpu then simulate hotplug of second one.
>  > 
>  > 1) user is already doing perverse things at this point
>  > 
>  > and
>  > 
>  > 2) machine BIOS is b0rken
>  > 
>  > ...so... it is only theoretical and probably not worth fixing.
> 
> SMP kernel on UP hardware is not theoretical, and is worth fixing.

Problem _I_ pointed out is theoretical, so the patch is okay and sorry
for the noise.
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html