Re: [Xen-devel] [PATCH v5 8/9] x86/intel_pstate: support the use of intel_pstate in pmstat.c

["Wang, Wei W" <wei.w.wang@xxxxxxxxx>]

On 26/10/2015 17:42,  Jan Beulich wrote:
> >>> On 26.10.15 at 08:59, <wei.w.wang@xxxxxxxxx> wrote:
> > On 26/10/2015 15:03,  Jan Beulich wrote:
> >> >>> "Wang, Wei W" <wei.w.wang@xxxxxxxxx> 10/26/15 7:27 AM >>>
> >> >On 08/10/2015 12:11,  Jan Beulich wrote:
> >> >> >>> On 14.09.15 at 04:32, <wei.w.wang@xxxxxxxxx> wrote:
> >> >> > @@ -309,23 +326,13 @@ struct xen_get_cpufreq_para {
> >> >> >      XEN_GUEST_HANDLE_64(uint32) scaling_available_frequencies;
> >> >> >      XEN_GUEST_HANDLE_64(char)   scaling_available_governors;
> >> >> >      char scaling_driver[CPUFREQ_NAME_LEN];
> >> >> > -
> >> >> > -    uint32_t cpuinfo_cur_freq;
> >> >> > -    uint32_t cpuinfo_max_freq;
> >> >> > -    uint32_t cpuinfo_min_freq;
> >> >> > -    uint32_t scaling_cur_freq;
> >> >> > -
> >> >> >      char scaling_governor[CPUFREQ_NAME_LEN];
> >> >> > -    uint32_t scaling_max_freq;
> >> >> > -    uint32_t scaling_min_freq;
> >> >> >
> >> >> >      /* for specific governor */
> >> >> >      union {
> >> >> >          struct  xen_userspace userspace;
> >> >> >          struct  xen_ondemand ondemand;
> >> >> >      } u;
> >> >> > -
> >> >> > -    int32_t turbo_enabled;
> >> >> >  };
> >> >>
> >> >> Is all of this re-arrangement really needed? Also can't
> >> >> turbo_enabled and scaling_turbo_pct be combined into a single field?
> >> >
> >> >Personally, we should not combine the two.
> >> > turbo_enabled is used by both the old pstate driver and
> >> >intel_pstate, but  scaling_turbo_pct is only used in intel_pstate.
> >> >If we use
> >> "scaling_turbo_pct=0"
> >> > to represent "turbo_enabled=0", and "scaling_turbo_pct>0" to
> >> >represent " turbo_enabled=1", then we will need to modify the old
> >> >driver to use scaling_turbo_pct, i.e. changing the old driver to be aware 
> >> >of
> the "percentage"
> >> > concept, which is proposed in intel_pstate. On the other side, I
> >> >think keeping  turbo_enabled and scaling_turbo_pct separated makes
> >> >the code
> >> easier to read.
> >>
> >> Note that "combine" doesn't necessarily mean "eliminate the old one"
> >> - they could as well become field of a union. The basic question you
> >> should ask yourself in such cases is: "Do both fields have a meaning
> >> at the same time?" If the answer is yes, then of course they should
> >> remain separate. If the answer is no _and_ their purpose is
> >> reasonably similar, then combining them should at least be considered.
> >
> > Ok. I will keep the two separated, since they do have their own
> > meaning at the same time.
> 
> Being which?

Keeping both of the two there. Just as what they are now - two independent 
fields.

Best,
Wei



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