Date: Mon, 04 Feb 2008 23:20:41 -0500 From: "Alexandre \"Sunny\" Kovalenko" <alex.kovalenko@verizon.net> To: John Baldwin <jhb@freebsd.org> Cc: freebsd-acpi@freebsd.org, Ian Smith <smithi@nimnet.asn.au>, Johannes Dieterich <dieterich.joh@googlemail.com> Subject: Re: [RFC] Patch to enable temperature ceiling in powerd Message-ID: <1202185241.821.8.camel@RabbitsDen> In-Reply-To: <200801310535.15540.jhb@freebsd.org> References: <1201733779.902.18.camel@RabbitsDen> <200801310535.15540.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--Boundary_(ID_CcJ724C+3L/87kIVLlCe0w)
Content-type: text/plain; charset=UTF-8
Content-transfer-encoding: 8BIT
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.
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 (Олександр Коваленко)
--Boundary_(ID_CcJ724C+3L/87kIVLlCe0w)
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-04 23:08:59.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,23 @@
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.
.El
.Sh SEE ALSO
.Xr acpi 4 ,
--Boundary_(ID_CcJ724C+3L/87kIVLlCe0w)
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
@@ -28,6 +28,13 @@
#include <sys/cdefs.h>
__FBSDID("$FreeBSD: src/usr.sbin/powerd/powerd.c,v 1.21 2007/06/13 19:05:11 marck Exp $");
+/*
+ * 29/9/6 Ian Smith <smithi@nimnet.asn.au> merged Alexandre "Sunny" Kovalenko's
+ * patch providing passive cooling override option (thanks David Wolfskill)
+ * + just in case, only read and test temperature when -T option is specified
+ * + for MODE_MAX, only switch high if not switching low for over temperature
+ */
+
#include <sys/param.h>
#include <sys/ioctl.h>
#include <sys/sysctl.h>
@@ -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_CcJ724C+3L/87kIVLlCe0w)--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1202185241.821.8.camel>