Re: [PATCH] Re: [2.5.39] (3/5) CPUfreq i386 drivers

[Dominik Brodowski]

On Sat, Sep 28, 2002 at 01:47:39PM -0400, Gerald Britton wrote:
> > -		if (!speedstep_low_freq || !speedstep_high_freq || 
> > +		if (!low || !high || 
> >  		    (speedstep_low_freq == speedstep_high_freq))
> >  			return -EIO;
> 
> This is still obviously broken.
> 
> First time through the loop, high and low are 0, one of the two of them gets
> set and the other is still 0.  This !low || !high test is still within the loop
> so it will drop out with -EIO.
Oh, thanks for pointing this out. I'll fix it for the next realease. 

>  I'd been using cpufreq under 2.4 for a while
> and with the recent updates (profile api), speedstep hasn't started up because
> of this, or because it was sending bad data into the notifier chains.  Your
> patch here passes bogus data into the notifier chains, which could be bad imho.
Well, so far the only x86 notifier is the one in arch/i386/kernel/time.c.
But yes, you're right, it's bad; and so I'll add a workaround for this, too. 
 
> If I fix the init by moving the !low || !high test below the loop, and prevent
> bad data from being passed into the notifier chains, I start getting memory
> corruption being detected by slab poisioning.
Any idea why this is happening??? The only dynamically allocated struct is
struct cpfureq_driver driver; and it is only kfree'd in speedstep_exit... 

> /* Disable IRQs */
> local_irq_save(flags);
> local_irq_disable();
>   . . .
> /* Enable IRQs */
> local_irq_enable();
> local_irq_restore(flags);

Thanks. Fixed.
BTW, unfortunately I can't test speedstep.c locally - my notebook has the
old 440?X ISSCL speedstep interface which is undocumented. So I'm really
glad whenever others test speedstep.c. Many thanks,

	Dominik