Date: Tue, 05 Feb 2008 08:35:31 -0500 From: "Alexandre \"Sunny\" Kovalenko" <alex.kovalenko@verizon.net> To: Ian Smith <smithi@nimnet.asn.au> Cc: freebsd-acpi@freebsd.org, Johannes Dieterich <dieterich.joh@googlemail.com> Subject: Re: [RFC] Patch to enable temperature ceiling in powerd Message-ID: <1202218531.815.6.camel@RabbitsDen> In-Reply-To: <Pine.BSF.3.96.1080205210644.7200A-100000@gaia.nimnet.asn.au> References: <Pine.BSF.3.96.1080205210644.7200A-100000@gaia.nimnet.asn.au>
next in thread | previous in thread | raw e-mail | index | archive | help
--Boundary_(ID_R/ClezF+eqUygbAW99Xw7Q) Content-type: text/plain; charset=utf-8 Content-transfer-encoding: 8BIT On Tue, 2008-02-05 at 21:22 +1100, Ian Smith wrote: > On Mon, 4 Feb 2008, Alexandre "Sunny" Kovalenko wrote: > > On Thu, 2008-01-31 at 05:35 -0500, John Baldwin wrote: > > > On Wednesday 30 January 2008 05:56:19 pm Alexandre "Sunny" Kovalenko wrote: > > > > Some time ago I have put together patch for powerd, which allows user to > > > > specify the temperature threshold at which powerd will lower CPU > > > > frequency no matter what the load was at the time. I recently had to > > > > adapt it to the 7.0-PRERELEASE for someone with the overheating laptop, > > > > which got me to think that it might be useful for someone else yet. > > > > > > > > Basic idea is fairly simple -- check temperature in TZ0 and, if it has > > > > reached certain value, either override frequency with the lowest > > > > available (in the case of 'max' setting) or change idle time to 100% and > > > > let adaptive algorithm decrease frequency gradually. > > > > > > > > I imagine it also could be poor man's substitute for the low noise > > > > acoustic policy ;) > > > > > > > > If there is an interest, I will go ahead and submit a PR, otherwise it > > > > will live in the mail archives for someone to find. Any comments, > > > > suggestions or criticisms are welcome. > > > > > > > > Temperature threshold (in Celsius) could be set by means of '-T' command > > > > line option (as in '-T 60'). > > > > > > A couple of suggestions: > > > > > > - I would make the default temperature 0 instead of 200 and just disable the > > > feature altogether if it is set to 0 (i.e. don't read the current > > > temperature and don't do any checks if it is 0). > > > - I would allow the temperature to be specified in either C, K or F with a > > > suffix to indicate the scale. (e.g., "80C", "120F", "300K") > > > - I would let the thermal zone name be configurable with a default of "tz0". > > > (e.g. "-z tz3"). You would then snprintf the sysctl mib name that gets > > > passed to sysctlbyname(3). > > > > > John, > > I have attached new patch, implementing your suggestions (some of these > > were already implemented by Ian Smith and sent to me privately). I have > > also attached first crack at the patch to powerd.8. Both patches are > > against 7.0 as of late January 31, EST. > > Alexandre, > > please remove my gratuitous 4-line comment at the top, it's OTT and was > really just to document for myself what I was playing around with then. Done. > > Though it's implied, I think the mod to powerd(8) should mention that > tz0 is the default. Done. > Also I think 999C is just a wee bit high to test > against, when 150C would melt most laptops (not to mention laps :) I > recall seeing 150C tested against somewhere else as a 'reasonable' max. Even 999K will melt quite a few things... I did not test for the absolute value -- I just wanted to limit number of characters, used in the option value, to simplify my own life. Two-digit temperatures are not enough even for C and three should be plenty. I can introduce test for 150C (and equivalent thereof) if you think it is necessary. > > Cheers, Ian > > > Johannes, > > could you, by any chance, apply the attached patch to the original copy > > of the powerd.c and see if it still allows you to use your system? Do > > not expect any improvements, though ;) > > > > -- > > Alexandre "Sunny" Kovalenko (ОлекÑандр Коваленко) > > > -- Alexandre "Sunny" Kovalenko (Олександр Коваленко) --Boundary_(ID_R/ClezF+eqUygbAW99Xw7Q) Content-type: text/x-patch; name=powerd.8.patch; charset=utf-8 Content-transfer-encoding: 7BIT Content-disposition: attachment; filename=powerd.8.patch --- powerd.8.orig 2008-02-04 22:48:14.000000000 -0500 +++ powerd.8 2008-02-05 08:25:08.000000000 -0500 @@ -39,7 +39,9 @@ .Op Fl p Ar ival .Op Fl P Ar pidfile .Op Fl r Ar percent +.Op Fl T Ar temperature .Op Fl v +.Op Fl z Ar thermal zone .Sh DESCRIPTION The .Nm @@ -92,11 +94,25 @@ adaptive mode should consider the CPU running and increase performance. The default is 65% or lower. +.It Fl T Ar temperature +Specifies temperature which will cause powerd to switch to the lowest +available frequency in the maximum mode or to reduce frequency in the +adaptive mode. Temperature could be specified using qualifiers C, F and K, +for Celsius, Fahrenheit and Kelvin respectively. Number without the qualifier +will be treated as the number with the qualifier C. Please, note that +negative temperature values and temperatures in the excess of 999 degrees, with +any qualifier, are considered invalid. .It Fl v Verbose mode. Messages about power changes will be printed to stdout and .Nm will operate in the foreground. +.It Fl z Ar thermal zone +Specifies the name of the thermal zone, used to monitor temperature for the 'T' +option above. This will be used as the part of the mib name, e.g. '-z tz2' will +result in 'hw.acpi.thermal.tz2.temperature' being monitored. If no thermal zone +name was specified on the command line, 'tz0' is assumed. In the absence of the 'T' +option, this option is ignored. .El .Sh SEE ALSO .Xr acpi 4 , --Boundary_(ID_R/ClezF+eqUygbAW99Xw7Q) Content-type: text/x-patch; name=powerd.c.patch; charset=utf-8 Content-transfer-encoding: 7BIT Content-disposition: attachment; filename=powerd.c.patch --- powerd.c.orig 2008-01-31 22:46:42.000000000 -0500 +++ powerd.c 2008-02-04 22:44:01.000000000 -0500 @@ -40,6 +47,7 @@ #include <errno.h> #include <fcntl.h> #include <libutil.h> +#include <regex.h> #include <signal.h> #include <stdio.h> #include <stdlib.h> @@ -87,18 +95,22 @@ static void handle_sigs(int sig); static void parse_mode(char *arg, int *mode, int ch); static void usage(void); +static int convert_temperature_to_acpi(const char *temp); /* Sysctl data structures. */ static int cp_time_mib[2]; static int freq_mib[4]; static int levels_mib[4]; static int acline_mib[3]; +static int temp_mib[5]; /* Configuration */ static int cpu_running_mark; static int cpu_idle_mark; static int poll_ival; +static int passive_cooling_mark; static int vflag; +static int tflag; static volatile sig_atomic_t exit_requested; static power_src_t acline_status; @@ -357,10 +369,67 @@ { fprintf(stderr, -"usage: powerd [-v] [-a mode] [-b mode] [-i %%] [-n mode] [-p ival] [-r %%] [-P pidfile]\n"); +"usage: powerd [-v] [-a mode] [-b mode] [-i %%] [-n mode] [-p ival] [-r %%] [-P pidfile] [-T temperature] [-z thermal zone]\n"); exit(1); } +/* Convert temperature in the form of nnC, nnK and nnF into tenths + * of the K as used by ACPI subsystem. Temperatures without qualifier + * are assumed to be in Celsius. Temperatures, longer then three + * digits or having qualifiers other then C, K or F are considered + * invalid. Function will return negative value if invalid temperature + * is encountered as well as upon reaching error condition. + */ +int convert_temperature_to_acpi(const char *temp) +{ + regex_t preg; + regmatch_t pmatch[3]; + int result = 0; + char temp_value[4]; + char qualifier = 'C'; /* If no qualifier is specified, defaulting to Celsius */ + + /* That would be an internal error -- return -1 */ + if (regcomp(&preg, "^([0-9]+)([CKF]?)$", REG_EXTENDED)) + result = -1; + /* If it looks like nothing we expect -- return -2 */ + if (!result && (regexec(&preg, temp, 3, pmatch, 0) == REG_NOMATCH)) + result = -2; + /* If we were able to successfully allocate 'preg' we need to free it */ + if (result != -1) + regfree(&preg); + /* If there were no problems so far, let's interpret the string */ + if (!result) { + if (pmatch[2].rm_so != pmatch[2].rm_eo) + qualifier = temp[pmatch[2].rm_so]; + /* Three digits of the temperature are enough for practical purposes */ + if ((pmatch[1].rm_eo - pmatch[1].rm_so) <= 3) { + memcpy(temp_value, &temp[pmatch[1].rm_so], pmatch[1].rm_eo - pmatch[1].rm_so); + temp_value[pmatch[1].rm_eo - pmatch[1].rm_so] = '\0'; + result = atoi(temp_value); + } + else + result = -3; + + if (result >= 0) { + switch (qualifier) { + case 'F': + result = ((result - 32) * 5) / 9; + /* Fallthrough is intentional */ + case 'C': + result += 273; + /* Fallthrough is intentional */ + case 'K': + result *= 10; + break; + default: + result = -4; + break; + } + } + } + return(result); +} + int main(int argc, char * argv[]) { @@ -371,6 +440,7 @@ const char *pidfile = NULL; long idle, total; int curfreq, *freqs, i, *mwatts, numfreqs; + int temperature; int ch, mode, mode_ac, mode_battery, mode_none; uint64_t mjoules_used; size_t len; @@ -381,13 +451,18 @@ cpu_idle_mark = DEFAULT_IDLE_PERCENT; poll_ival = DEFAULT_POLL_INTERVAL; mjoules_used = 0; + tflag = 0; vflag = 0; + char tz_mib_name[40]; /* This should be sufficient to hold "hw.acpi.thermal.%s.temperature" */ /* User must be root to control frequencies. */ if (geteuid() != 0) errx(1, "must be root to run"); - while ((ch = getopt(argc, argv, "a:b:i:n:p:P:r:v")) != EOF) + /* Set default mib name for the thermal zone */ + snprintf(tz_mib_name, sizeof(tz_mib_name), "hw.acpi.thermal.%s.temperature", "tz0"); + + while ((ch = getopt(argc, argv, "a:b:i:n:p:P:r:T:v:z:")) != EOF) switch (ch) { case 'a': parse_mode(optarg, &mode_ac, ch); @@ -424,9 +499,26 @@ usage(); } break; + case 'T': + passive_cooling_mark = convert_temperature_to_acpi(optarg); + if (passive_cooling_mark < 0) { + warnx("%d is not valid temperature for passive cooling", + passive_cooling_mark); + usage(); + } else if (passive_cooling_mark > 0) + tflag = 1; + break; case 'v': vflag = 1; break; + case 'z': + /* + * We will decipher thermal zone here but it will not + * be used unless -T was also present + */ + snprintf(tz_mib_name, sizeof(tz_mib_name), + "hw.acpi.thermal.%s.temperature", optarg); + break; default: usage(); } @@ -446,6 +538,11 @@ len = 4; if (sysctlnametomib("dev.cpu.0.freq_levels", levels_mib, &len)) err(1, "lookup freq_levels"); + if (tflag) { /* if no -T option don't fail if temp not available */ + len = 5; + if (sysctlnametomib(tz_mib_name, temp_mib, &len)) + err(1, "lookup temperature"); + } /* Check if we can read the idle time and supported freqs. */ if (read_usage_times(NULL, NULL)) @@ -528,6 +625,13 @@ warn("error reading current CPU frequency"); continue; } + /* Read current temperature if -T option is set */ + if(tflag) { + len = sizeof(temperature); + if(sysctl(temp_mib, 5, &temperature, &len, NULL, 0)) + err(1, "error reading current temperature"); + } + if (vflag) { for (i = 0; i < numfreqs; i++) { @@ -571,6 +675,19 @@ if (set_freq(freqs[0]) != 0) { warn("error setting CPU freq %d", freqs[0]); + /* ... unless passive cooling override */ + if(tflag && temperature > passive_cooling_mark) { + if(curfreq != freqs[numfreqs - 1]) { + if (vflag) { + printf("passive cooling override; " + "changing frequency to %d MHz\n", + freqs[numfreqs - 1]); + } + if (set_freq(freqs[numfreqs - 1])) + err(1, "error setting CPU freq %d", + freqs[numfreqs - 1]); + } + } continue; } } @@ -583,6 +700,14 @@ warn("read_usage_times() failed"); continue; } + /* + * If temperature has risen over passive cooling mark, we + * would want to decrease frequency regardless of the load, + * Simplest way to go about this would be to report 100% + * idle CPU and let adaptive algorithm do its job. + */ + if(temperature > passive_cooling_mark) + idle = total; /* * If we're idle less than the active mark, bump up two levels. --Boundary_(ID_R/ClezF+eqUygbAW99Xw7Q)--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1202218531.815.6.camel>