On Thu, Sep 24 2020, Jeremie Courreges-Anglas <[email protected]> wrote:
> On Wed, Sep 23 2020, "Ted Unangst" <[email protected]> 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
