Prompted by a report from Miod: setting hw.setperf works only if the
kernel doesn't have a usable cpu_setperf implementation.  The current
apmd(8) code warns if setting hw.perfpolicy fails, but then handles
back bogus values to apm(8) clients.

The easy fix is to just query the kernel about the actual hw.setperf
value.  This fixes other incorrect apmd(8) assumptions:
- hw.setperf is initially set to "manual"
- hw.setperf can't change behind apmd's back

ok?


Note: I inlined the apmd(8)->apm(8) perfpolicy conversion for now, which
brings a question.  I find it weird that there is a special "high"
perfpolicy (effectively similar to perfpolicy=manual + setperf=100) but
no "low" perfpolicy.  Is "high" useful to anyone?


Index: apmd.c
===================================================================
RCS file: /d/cvs/src/usr.sbin/apmd/apmd.c,v
retrieving revision 1.97
diff -u -p -r1.97 apmd.c
--- apmd.c      23 Sep 2020 05:50:26 -0000      1.97
+++ apmd.c      23 Sep 2020 10:46:37 -0000
@@ -59,8 +59,6 @@
 
 int debug = 0;
 
-int doperf = PERF_NONE;
-
 extern char *__progname;
 
 void usage(void);
@@ -255,7 +253,10 @@ handle_client(int sock_fd, int ctl_fd)
        socklen_t fromlen;
        struct apm_command cmd;
        struct apm_reply reply;
-       int cpuspeed_mib[] = {CTL_HW, HW_CPUSPEED};
+       int perfpol_mib[] = { CTL_HW, HW_PERFPOLICY };
+       char perfpol[32];
+       size_t perfpol_sz = sizeof(perfpol);
+       int cpuspeed_mib[] = { CTL_HW, HW_CPUSPEED };
        int cpuspeed = 0;
        size_t cpuspeed_sz = sizeof(cpuspeed);
 
@@ -290,19 +291,16 @@ handle_client(int sock_fd, int ctl_fd)
                reply.newstate = HIBERNATING;
                break;
        case SETPERF_LOW:
-               doperf = PERF_MANUAL;
                reply.newstate = NORMAL;
                logmsg(LOG_NOTICE, "setting hw.perfpolicy to low");
                setperfpolicy("low");
                break;
        case SETPERF_HIGH:
-               doperf = PERF_MANUAL;
                reply.newstate = NORMAL;
                logmsg(LOG_NOTICE, "setting hw.perfpolicy to high");
                setperfpolicy("high");
                break;
        case SETPERF_AUTO:
-               doperf = PERF_AUTO;
                reply.newstate = NORMAL;
                logmsg(LOG_NOTICE, "setting hw.perfpolicy to auto");
                setperfpolicy("auto");
@@ -312,11 +310,22 @@ handle_client(int sock_fd, int ctl_fd)
                break;
        }
 
-       if (sysctl(cpuspeed_mib, 2, &cpuspeed, &cpuspeed_sz, NULL, 0) == -1)
-               logmsg(LOG_INFO, "cannot read hw.cpuspeed");
+       reply.perfmode = PERF_NONE;
+       if (sysctl(perfpol_mib, 2, perfpol, &perfpol_sz, NULL, 0) == -1)
+               logmsg(LOG_INFO, "cannot read hw.perfpolicy");
+       else {
+               if (strcmp(perfpol, "manual") == 0 ||
+                   strcmp(perfpol, "high") == 0) {
+                       reply.perfmode = PERF_MANUAL;
+               } else if (strcmp(perfpol, "auto") == 0)
+                       reply.perfmode = PERF_AUTO;
+       }
 
+       if (sysctl(cpuspeed_mib, 2, &cpuspeed, &cpuspeed_sz, NULL, 0) == -1) {
+               logmsg(LOG_INFO, "cannot read hw.cpuspeed");
+               cpuspeed = 0;
+       }
        reply.cpuspeed = cpuspeed;
-       reply.perfmode = doperf;
        reply.vno = APMD_VNO;
        if (send(cli_fd, &reply, sizeof(reply), 0) != sizeof(reply))
                logmsg(LOG_INFO, "reply to client botched");
@@ -386,6 +395,7 @@ main(int argc, char *argv[])
        const char *errstr;
        int kq, nchanges;
        struct kevent ev[2];
+       int doperf = PERF_NONE;
 
        while ((ch = getopt(argc, argv, "aACdHLsf:t:S:z:Z:")) != -1)
                switch(ch) {

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Reply via email to