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