[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 2/3] x86/idle: update to include further package/core residency MSRs

On Wed, 2014-03-05 at 10:37 +0000, Jan Beulich wrote:
> With the number of these growing it becomes increasingly desirable to
> not repeatedly alter the sysctl interface to accommodate them. Replace
> the explicit listing of numbered states by arrays,

I don't have much of an opinion on the hypercall interface, so I'm just
taking that as a given and looking at the tools side accordingly.

> unused fields of which will remain untouched by the hypercall.

Are you supposed to initialise them to some known sentinal or are the
valid entries identified somewhere else (sorry, don't know much about
x86 pm).

> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> --- 2014-02-13.orig/tools/libxc/xc_pm.c       2014-03-04 17:43:06.000000000 
> +0100
> +++ 2014-02-13/tools/libxc/xc_pm.c    2014-03-05 08:54:58.000000000 +0100
> @@ -123,46 +123,90 @@ int xc_pm_get_max_cx(xc_interface *xch, 
>  int xc_pm_get_cxstat(xc_interface *xch, int cpuid, struct xc_cx_stat *cxpt)
> [...]
> +int xc_pm_get_cx_stat(xc_interface *xch, int cpuid, struct xc_cx_stat_v2 
> *cxpt)

That is an incredibly subtle difference in the naming!

If v1 is considered deprecated then lets just get rid of it. There's
only one caller which you update in the next patch. I'd be perfectly
happy to have those collapsed and for the old interface to go away
immediately (or do it in patch 4/3 if you prefer).

> +{
> +    DECLARE_NAMED_HYPERCALL_BOUNCE(triggers, cxpt->triggers,
> +                                   cxpt->nr * sizeof(*cxpt->triggers),
> +                                   XC_HYPERCALL_BUFFER_BOUNCE_OUT);
> +    DECLARE_NAMED_HYPERCALL_BOUNCE(residencies, cxpt->residencies,
> +                                   cxpt->nr * sizeof(*cxpt->residencies),
> +                                   XC_HYPERCALL_BUFFER_BOUNCE_OUT);

The original had these as BOUNCE_BOTH. If that was wrong  and this
change was therefore intentional (which I suspect is the case) please
note it in the commit message.


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.