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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Wed, Nov 10, 2021 at 09:19:35AM +0000, Jane Malalane wrote:
> Before, user would change turbo status but this had no effect: boolean
> was set but policy wasn't reevaluated.  Policy must be reevaluated so
> that CPU frequency is chosen according to the turbo status and the
> current governor.
> 
> Therefore, add __cpufreq_governor() in cpufreq_update_turbo().
> 
> Reported-by: <edvin.torok@xxxxxxxxxx>
> Signed-off-by: <jane.malalane@xxxxxxxxxx>
> ---
> CC: Jan Beulich <jbeulich@xxxxxxxx>
> CC: Ian Jackson <iwj@xxxxxxxxxxxxxx>
> ---
> 
> Release rationale:
> Not taking this patch means that turbo status is misleading.
> 
> Taking this patch is low-risk as it only uses a function that already
> exists and is already used for setting the chosen scaling governor.
> Essentially, this change is equivalent to running 'xenpm
> en/disable-turbo-mode' and, subsequently, running 'xenpm
> set-scaling-governor <current governor>'.
> ---
>  xen/drivers/cpufreq/utility.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/drivers/cpufreq/utility.c b/xen/drivers/cpufreq/utility.c
> index b93895d4dd..5f200ff3ee 100644
> --- 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).

Thanks, Roger.