Patch to stop i386 longhaul from deadlocking the kernel

Hi,

I've had some problems with the 'longhaul' driver deadlocking my kernel when the module is inserted. I tracked this down to the module failing to discover the fsb speed.

This patch stops the deadlock in the case where the speed of the fsb is undetected. I've also made the guess function more vague so it can detect the correct speed for my cpu, it might need to be even more vague for other cpu's.

I am still unable to change the CPU speed even with this patch however.

The problem arrived in about the -test6 timeframe and still exists in the latest test9 I've tested. The motherboard is a mini-itx from via running at 800.261 Mhz.

The relevent kernel output is:

Before:

longhaul: VIA C3 'Ezra' [C5C] CPU detected. Longhaul v1 supported.
longhaul: MinMult=3.0x MaxMult=6.0x
longhaul: FSB: 0MHz Lowestspeed=0MHz Highestspeed=0MHz

At this point the whole system would lock solid.

After:

longhaul: VIA C3 'Ezra' [C5C] CPU detected. Longhaul v1 supported.
longhaul: MinMult=3.0x MaxMult=6.0x
longhaul: FSB: 133MHz Lowestspeed=399MHz Highestspeed=798MHz

The module is now inserted and appears in '/proc/modules'

Ashley,

diff -u linux-2.6.0-test9-dist/arch/i386/kernel/cpu/cpufreq/longhaul.c linux-2.6.0-test9/arch/i386/kernel/cpu/cpufreq/longhaul.c
@@ -30,6 +30,9 @@
 
 #include "longhaul.h"
 
+#define SPEEDKLUDGE 0xbf
+
+
 #define DEBUG
 
 #ifdef DEBUG

@@ -177,13 +180,13 @@
        target = ((maxmult/10)*guess);
        if (maxmult%10 != 0)
                target += (guess/2);
-       target &= ~0xf;
+       target &= ~SPEEDKLUDGE;
        return target;
 }
 
 static int guess_fsb(int maxmult)
 {
-       int speed = (cpu_khz/1000) & ~0xf;
+       int speed = (cpu_khz/1000) & ~SPEEDKLUDGE;
        int i;
        int speeds[3] = { 66, 100, 133 };
 

@@ -248,6 +251,10 @@
        dprintk (KERN_INFO PFX "FSB: %dMHz Lowestspeed=%dMHz Highestspeed=%dMHz\n",
                 fsb, lowest_speed, highest_speed);
 
+       if ( fsb == 0 ) {
+               return -EINVAL;
+       }
+
        longhaul_table = kmalloc((numscales + 1) * sizeof(struct cpufreq_frequency_table), GFP_KERNEL);
        if(!longhaul_table)
                return -ENOMEM;

Ashley Pittman <[email protected]>

0xbf seems bizarre. Something went wrong somewhere.
Can you put some printk's in there, so I can see what values _guess() and guess_fsb() are being called with ?

You're not the first report I've had of this. I'm not sure what I broke. It was a case of 'fix for some, break others'.

Why it locks up is also odd.

Dave

The computer is at home today but it's one of the 800Mhz boards, the fsb is 133, multiplier of 60. The speed should therefore be 798 but cpu_khz is reporting it as 800. Then it's just a case of how many bits you need to remove of before 798==800. The result is that they both need to equal 768. I've attached a user-space program which demonstrates this with output.

I didn't get anywhere with that, I took the route of stopping the code when a error condition was detected, returning -EFAULT avoided the problem nicely.

Ashley,

optput:
ashley:cpufreq> ./a.out
Kludge is 15 0xf
Speed is 800 0x320 (800 0x320)
Working... guess 66 maxmult 60 target 384 0x180
Working... guess 100 maxmult 60 target 592 0x250
Working... guess 133 maxmult 60 target 784 0x310
The answer is 0
Kludge is 191 0xbf
Speed is 768 0x300 (800 0x320)
Working... guess 66 maxmult 60 target 256 0x100
Working... guess 100 maxmult 60 target 576 0x240
Working... guess 133 maxmult 60 target 768 0x300
The second answer is 133
ashley:cpufreq>

test.c:

int SPEEDKLUDGE = 0;
int cpu_khz = 800000;

static int _guess (int guess, int maxmult)

{
	int target;
	
	target = ((maxmult/10)*guess);
	if (maxmult%10 != 0)
		target += (guess/2);
	target &= ~SPEEDKLUDGE;
	printf("Working... guess %d maxmult %d target %d %#x\n",
	       guess,maxmult,target,target);
	return target;
}

static int guess_fsb(int maxmult)

{
	int speed = (cpu_khz/1000) & ~SPEEDKLUDGE;
	int i;
	int speeds[3] = { 66, 100, 133 };
	
	printf("Kludge is %d %#x\n",SPEEDKLUDGE,SPEEDKLUDGE);
	
	printf("Speed is %d %#x (%d %#x)\n",
	       speed,speed,
	       (cpu_khz/1000),(cpu_khz/1000));
	
	for (i=0; i<3; i++) {
		if (_guess(speeds[i],maxmult) == speed)
			return speeds[i];
	}
	return 0;
}

main () {
	SPEEDKLUDGE = 0xf;
	printf("The answer is %d\n",guess_fsb(60));
	SPEEDKLUDGE = 0xbf;
	printf("The second answer is %d\n",guess_fsb(60));
}

Right, so the anding is catching the case of overflow, but neglecting underflow. It seems to do the right thing if I change guess_fsb to do this..

    for (i=0; i<3; i++) {
        ret = _guess(speeds[i],maxmult);
        if (ret == speed)
            return speeds[i];
        if (ret < speed) {
            if ((ret+16) == speed)
                return speeds[i];
        }
    }
    return 0;
}

I need to think about this some more. I'm not entirely comfortable with that, it looks somewhat fragile, though I can see why it works. 133 * 6 = 798
798 & ~0xf = 784
784 + 16 = 800

I wonder how well this kludge works for other frequencies.

Dave

Isn't the correct thing to do to increase the speed before anding it? That way it will always end up in the right bin. The following code works for me and does feel right. A value of 0x7 is all thats needed to get the right answer on my machine in this case.

static int _guess (int guess, int maxmult)

{
	int target;
	
	target = ((maxmult/10)*guess);
	if (maxmult%10 != 0)
		target += (guess/2);
	target += SPEEDKLUDGE/2;
	target &= ~SPEEDKLUDGE;
	printf("Working... guess %d maxmult %d target %d %#x\n",
	       guess,maxmult,target,target);
	return target;
}

static int guess_fsb(int maxmult)

{
	int speed = (cpu_khz/1000);
	int i;
	int speeds[3] = { 66, 100, 133 };
	
	speed += SPEEDKLUDGE/2;
	speed &= ~SPEEDKLUDGE;
	
	printf("Kludge is %d %#x\n",(int)SPEEDKLUDGE,(int)SPEEDKLUDGE);
	
	printf("Speed is %d %#x (%d %#x)\n",
	       speed,speed,
	       (cpu_khz/1000),(cpu_khz/1000));
	
	for (i=0; i<3; i++) {
		if (_guess(speeds[i],maxmult) == speed)
			return speeds[i];
	}
	return 0;
}