On Thu, Sep 24 2020, Jeremie Courreges-Anglas <j...@wxcvbn.org> wrote: > On Wed, Sep 23 2020, "Ted Unangst" <t...@tedunangst.com> wrote: >> On 2020-09-23, Jeremie Courreges-Anglas wrote: >> >>> ok? >> >> Seems fine. >> >> >>> 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? >> >> If you're benchmarking or something, it's convenient to toggle between >> high and auto. There's less use for low, since that's what auto defaults to. > > Well you can do auto->high easily with apm(8) -H or sysctl > hw.perfpolicy=manual hw.setperf=100. I'm not sure a special "high" > hw.perfpolicy choice brings much value and I would appreciate a simple > "manual" vs "auto" situation. > > This has an impact on documentation. sysctl(2) doesn't mention that > setting hw.perfpolicy=high also locks hw.setperf=100. The apm(8) > manpage only talks about -H setting hw.setperf=100. The description for > apmd(8) -H says > > Start apmd in *manual* performance adjustment mode, > initialising hw.setperf to 100. > > which is not the whole story. If you use apm/apmd -H you can't later > just run sysctl hw.setperf=50: > > sysctl: hw.setperf: Operation not permitted
The diff below teaches apmd(8) -H to set hw.perfpolicy="manual" and hw.setperf=100, instead of setting hw.perfpolicy="high". - matches the manpage - apm(8) reporting becomes accurate - more symmetry with -L ("low") - avoids the "sysctl: hw.setperf: Operation not permitted" quirk Teaching apm(8) how to print hw.perfpolicy="high" then becomes low priority. While here, simplify the sysctl(2) calls: in this function we don't care about the old values. ok? Index: apmd.c =================================================================== --- apmd.c.orig +++ apmd.c @@ -650,27 +650,27 @@ void setperfpolicy(char *policy) { int hw_perfpol_mib[] = { CTL_HW, HW_PERFPOLICY }; - char oldpolicy[32]; - size_t oldsz = sizeof(oldpolicy); - int setlo = 0; + int hw_perf_mib[] = { CTL_HW, HW_SETPERF }; + int new_perf = -1; if (strcmp(policy, "low") == 0) { policy = "manual"; - setlo = 1; + new_perf = 0; + } else if (strcmp(policy, "high") == 0) { + policy = "manual"; + new_perf = 100; } - if (sysctl(hw_perfpol_mib, 2, oldpolicy, &oldsz, + if (sysctl(hw_perfpol_mib, 2, NULL, NULL, policy, strlen(policy) + 1) == -1) logmsg(LOG_INFO, "cannot set hw.perfpolicy"); - if (setlo == 1) { - int hw_perf_mib[] = {CTL_HW, HW_SETPERF}; - int perf; - int new_perf = 0; - size_t perf_sz = sizeof(perf); - if (sysctl(hw_perf_mib, 2, &perf, &perf_sz, &new_perf, perf_sz) == -1) - logmsg(LOG_INFO, "cannot set hw.setperf"); - } + if (new_perf == -1) + return; + + if (sysctl(hw_perf_mib, 2, NULL, NULL, + &new_perf, sizeof(new_perf)) == -1) + logmsg(LOG_INFO, "cannot set hw.setperf"); } void -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE