Re: [Xen-devel] [PATCH 2/2] cpufreq: fix turbo mode state reporting

[Jacob Shin <jacob.shin@xxxxxxx>]

On Thu, Jun 20, 2013 at 09:01:22AM +0100, Jan Beulich wrote:
> >>> On 19.06.13 at 22:14, Jacob Shin <jacob.shin@xxxxxxx> wrote:
> > Currently we report back 0 or 1, which is broken since xenpm expects
> > CPUFREQ_TURBO_DISABLED, CPUFREQ_TURBO_UNSUPPORTED, or
> > CPUFREQ_TURBO_ENABLED. Report back actual policy->turbo value instead.
> 
> I think that it's xenpm that's wrong here - the three constants above
> aren't exposed in public headers, and hence aren't part of the ABI.
> Furthermore the structure member name that the result of this
> function gets stored into is "turbo_enabled", which also suggests a
> boolean value to me. I.e. if we want to change the value set that
> the hypervisor may return here, the field should also get renamed,
> and the value set exposed.

Okay I'll fix xenpm instead.

> 
> > --- a/xen/drivers/cpufreq/utility.c
> > +++ b/xen/drivers/cpufreq/utility.c
> > @@ -428,7 +428,10 @@ int cpufreq_get_turbo_status(int cpuid)
> >      struct cpufreq_policy *policy;
> >  
> >      policy = per_cpu(cpufreq_cpu_policy, cpuid);
> > -    return policy && policy->turbo;
> > +    if (!policy)
> > +        return CPUFREQ_TURBO_UNSUPPORTED;
> > +
> > +    return policy->turbo;
> 
> Nevertheless this would need adjustment even for the boolean
> value case:
> 
> -    return policy && policy->turbo;
> +    return policy && policy->turbo == CPUFREQ_TURBO_ENABLED;

Yup that's correct.

Thanks,


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel