Code review comment for ~joalif/ubuntu/+source/systemd:lp1806012

Revision history for this message
Dan Streetman (ddstreet) wrote :

> I should say that respecting the setting for cpufrequtils is sensible

cpufrequtils has not seen updates since trusty:

$ rmadison cpufrequtils
 cpufrequtils | 007-2 | precise/universe | source, amd64, armel, armhf, i386, powerpc
 cpufrequtils | 008-1 | trusty/universe | source, amd64, arm64, armhf, i386, powerpc, ppc64el
 cpufrequtils | 008-1 | xenial/universe | source, amd64, arm64, armhf, i386, powerpc, ppc64el, s390x
 cpufrequtils | 008-1build1 | bionic/universe | source, amd64, arm64, armhf, i386, ppc64el, s390x
 cpufrequtils | 008-1build1 | cosmic/universe | source, amd64, arm64, armhf, i386, ppc64el, s390x
 cpufrequtils | 008-1.1 | disco/universe | source, amd64, arm64, armhf, i386, s390x
 cpufrequtils | 008-1.1 | eoan/universe | source, amd64, arm64, armhf, i386, s390x

additionally, refusing to make 'ondemand' configurable means anyone wanting to configure their system has to both install cpufrequtils (which is unsupported for UA customers, since it's universe) as well as disabling 'ondemand'.

Seeing the future, it's quite possible that debian (and/or us) simply drop cpufrequtils at some point.

> /etc/default files are not a particularly good interface overall

however, they are widely used for configuration in debian and ubuntu.

> use cases presented are all addressed by disabling the systemd unit

not without *also* installing the cpufrequtils package. Simply disabling ondemand will leave the governor at the kernel-compiled default.

> but I
> think this should be accomplished by making this unit a no-op in the presence
> of that file, not having ondemand reapply the same policy.

option 1) change cpu-setfreq to be a noop when /etc/default/cpufrequtils is detected, AND install cpufrequtils
option 2) change cpu-setfreq to honor /etc/default/cpufrequtils selection, with *or* without cpufrequtils installed

the end result is the same; the only difference is your design requires cpufrequtils to be installed. Is there a reason that design is better than this merge proposal?

and of course:

option 3) change cpu-setfreq to be a noop when /etc/default/cpufrequtils is detected, but without cpufrequtils installed -> this leads to the governor being completely unset, defaulting to the kernel compile default. Probably not exactly expected.

« Back to merge proposal