Re: [Xen-devel] [PATCH v3A 2/3] tools/libxc: allow controlling the max C-state sub-state

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2014-06-27 at 16:25 +0100, Jan Beulich wrote:
> >>> On 27.06.14 at 17:02, <Ian.Campbell@xxxxxxxxxx> wrote:
> > On Mon, 2014-06-23 at 16:43 +0100, Jan Beulich wrote:
> >> --- a/xen/include/public/sysctl.h
> >> +++ b/xen/include/public/sysctl.h
> >> @@ -354,7 +354,11 @@ struct xen_sysctl_pm_op {
> >>      /* set/reset scheduler power saving option */
> >>      #define XEN_SYSCTL_pm_op_set_sched_opt_smt    0x21
> >>  
> >> -    /* cpuidle max_cstate access command */
> >> +    /*
> >> +     * cpuidle max C-state and max C-sub-state access command:
> >> +     * Set cpuid to 0 for max C-state.
> >> +     * Set cpuid to 1 for max C-sub-state.
> > 
> > This seems pretty nasty. Since the sysctl interface is not fixed can't
> > we just turn the existing get/set_max_cstate fields into structs? Or
> > (less preferably) define C-sub-states to have bit 31 set.
> > 
> > Can we at least get some named constants instead of 0 and 1?
> 
> Any/all of this would be fine with me; what I suggested was merely
> an approach with as little changes as possible for an interface that
> wasn't defined/implemented well in the first place.

If the interface wasn't well defined/implemented in the first place
shouldn't we make it well defined/implemented rather than worry about
minimising the changes?

Ian.


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