On Wed, Sep 23 2020, Jeremie Courreges-Anglas <j...@wxcvbn.org> wrote: > 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.
Some corrections: > The easy fix is to just query the kernel about the actual hw.setperf hw.perfpolicy > value. This fixes other incorrect apmd(8) assumptions: > - hw.setperf is initially set to "manual" hw.perfpolicy > - hw.setperf can't change behind apmd's back hw.perfpolicy > 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