| Message ID | 1251303445-25317-5-git-send-email-elendil@planet.nl (mailing list archive) |
|---|---|
| State | RFC, archived |
| Headers | show |
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.
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. Would 1000 (1 °C) perhaps be more acceptable as a limit? I doubt there are valid use-cases for below 0 temps :-) -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 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); thanks, rui > Would 1000 (1 °C) perhaps be more acceptable as a limit? I doubt there are > valid use-cases for below 0 temps :-) -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2009-08-31 at 18:30 +0800, Frans Pop wrote: > 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? > I see many laptops with a passive trip point higher than 90C, so a passive trip point higher than 100C may be meaningful. I think we should use a higher value, say 2000? thanks, rui -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt index 2a036eb..09d5c88 100644 --- a/Documentation/thermal/sysfs-api.txt +++ b/Documentation/thermal/sysfs-api.txt @@ -206,6 +206,7 @@ passive point for the zone. Activation is done by polling with an interval of 1 second. Unit: millidegrees Celsius + Minimum value: 40000 RW, Optional ***************************** diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c index 0a69672..2d13d0d 100644 --- a/drivers/thermal/thermal_sys.c +++ b/drivers/thermal/thermal_sys.c @@ -225,6 +225,12 @@ passive_store(struct device *dev, struct device_attribute *attr, if (!sscanf(buf, "%d\n", &state)) return -EINVAL; + /* sanity check: values below 40000 millicelcius don't make sense + * and can cause the system to go into a thermal heart attack + */ + if (state && state < 40000) + return -EINVAL; + if (state && !tz->forced_passive) { mutex_lock(&thermal_list_lock); list_for_each_entry(cdev, &thermal_cdev_list, node) {
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. For values below 1000 an additional problem is that they would show as 0 in /proc/acpi/thermal/TZx/trip_points:passive. cat passive 0 echo -n 90000 >passive cat passive 90000 echo -n 30000 >passive bash: echo: write error: Invalid argument Signed-off-by: Frans Pop <elendil@planet.nl> Cc: Matthew Garrett <mjg@redhat.com> Cc: Zhang Rui <rui.zhang@intel.com> -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html