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>