On Wed, Sep 23 2020, Jeremie Courreges-Anglas <[email protected]> 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
