On 10.11.2021 13:39, Roger Pau Monné wrote: > On Wed, Nov 10, 2021 at 09:19:35AM +0000, Jane Malalane wrote: >> --- a/xen/drivers/cpufreq/utility.c >> +++ b/xen/drivers/cpufreq/utility.c >> @@ -417,10 +417,14 @@ int cpufreq_update_turbo(int cpuid, int new_state) >> { >> ret = cpufreq_driver.update(cpuid, policy); >> if (ret) >> + { >> policy->turbo = curr_state; >> + return ret; >> + } >> } >> >> - return ret; >> + /* Reevaluate current CPU policy. */ >> + return __cpufreq_governor(policy, CPUFREQ_GOV_LIMITS); > > Do you need to revert the policy->turbo value to the previous one if > the call to __cpufreq_governor returns an error? (much like it's done > for the .update call above).
I guess this can be viewed either way: Keeping the value would allow a later successful invocation of the .target() hook to observe the intended value. Obviously then it's questionable whether returning an error in that case isn't going to be misleading - failure of the policy update would then rather need to be indicated by some "deferred" indicator (which we don't have). Taking into account the behavior prior to this patch I wonder whether it's an option to simply ignore an error from __cpufreq_governor() here. Jan