On 12.05.2023 03:02, Marek Marczykowski-Górecki wrote: > On Wed, May 10, 2023 at 04:19:57PM +0200, Jan Beulich wrote: >> On 10.05.2023 15:54, Jason Andryuk wrote: >>> On Mon, May 8, 2023 at 2:33 AM Jan Beulich <jbeul...@suse.com> wrote: >>>> On 05.05.2023 17:35, Jason Andryuk wrote: >>>>> On Fri, May 5, 2023 at 3:01 AM Jan Beulich <jbeul...@suse.com> wrote: >>>>> The other issue is that if you select "hwp" as the governor, but HWP >>>>> hardware support is not available, then hwp_available() needs to reset >>>>> the governor back to the default. This feels like a layering >>>>> violation. >>>> >>>> Layering violation - yes. But why would the governor need resetting in >>>> this case? If HWP was asked for but isn't available, I don't think any >>>> other cpufreq handling (and hence governor) should be put in place. >>>> And turning off cpufreq altogether (if necessary in the first place) >>>> wouldn't, to me, feel as much like a layering violation. >>> >>> My goal was for Xen to use HWP if available and fallback to the acpi >>> cpufreq driver if not. That to me seems more user-friendly than >>> disabling cpufreq. >>> >>> if ( hwp_available() ) >>> ret = hwp_register_driver(); >>> else >>> ret = cpufreq_register_driver(&acpi_cpufreq_driver); >> >> That's fine as a (future) default, but for now using hwp requires a >> command line option, and if that option says "hwp" then it ought to >> be hwp imo. > > As a downstrem distribution, I'd strongly prefer to have an option that > would enable HWP when present and fallback to other driver otherwise, > even if that isn't the default upstream. I can't possibly require large > group of users (either HWP-having or HWP-not-having) to edit the Xen > cmdline to get power management working well. > > If the meaning for cpufreq=hwp absolutely must include "nothing if HWP > is not available", then maybe it should be named cpufreq=try-hwp > instead, or cpufreq=prefer-hwp or something else like this?
Any new sub-option needs to fit the existing ones in its meaning. I could see e.g. "cpufreq=xen" alone to effect what you want (once hwp becomes available for use by default). But (for now at least) I continue to think that a request for "hwp" ought to mean HWP. Jan
