Re: [PATCH] xen/cpufreq: Reset policy after enabling/disabling turbo status

[Jan Beulich <jbeulich@suse.com>]

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