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. > If we are setting cpufreq_opt_governor to enter hwp_available(), but > then HWP isn't available, it seems to me that we need to reset > cpufreq_opt_governor when exiting hwp_available() false. This may be necessary in the future, but shouldn't be necessary right now. It's not entirely clear to me how that future is going to look like, command line option wise. Jan
