Re: [PATCH 4/6] thermal: add sanity check for the passive attribute

[Frans Pop <elendil@planet.nl>]

On Monday 31 August 2009, Zhang Rui wrote:
> On Thu, 2009-08-27 at 00:48 +0800, Frans Pop wrote:
> > On Wednesday 26 August 2009, Matthew Garrett wrote:
> > > On Wed, Aug 26, 2009 at 06:17:23PM +0200, Frans Pop wrote:
> > > > Values below 40000 milli-celsius (limit is somewhat arbitrary)
> > > > don't make sense and can cause the system to go into a thermal
> > > > heart attack: the actual temperature will always be lower and
> > > > thus the system will be throttled down to its lowest setting.
> > >
> > > Not keen on this - it's a pretty arbitrary cutoff, and there are
> > > some cases where someone might want this value. Policy belongs in
> > > userspace, and all that.
> >
> > What cases do you see? Testing? Or systems that might have to operate
> > at such a low temperature? I deliberately chose a value that's at a
> > level that's easy to reach.
> >
> > I agree it is arbitrary, but it will prevent major confusion when
> > someone like me echo's 95 directly in sysfs.
>
> this is a problem.
> how about something like:
> #define THERMAL_PASSIVE_WARNING_LEVEL 0x40000

Hmmm. 40000 hexadecimal? That seems a bit high ;-)

> if (state < THERMAL_PASSIVE_WARNING_LEVEL)
>    printk(KERN_WARNING PREFIX "Passive trip point too low, this may"
>           "slow down your laptop because processors are throttled "
>           "whenever the temperature is higher than %dC\n", state/1000);

Disadvantage is that users are unlikely to actually see that message at 
the time they set the value, especially if they're working in some xterm. 
They'd have to check dmesg or log files. It also increases the .text size 
of the module for an option that very few people are likely to use.

> > Would 1000 (1 °C) perhaps be more acceptable as a limit? I doubt
> > there are valid use-cases for below 0 temps :-)

I'd prefer this option. Do you see any downside to this?

Cheers,
FJP