Re: [PATCH 2/2] sched/core: Update vcpu_runstate_get() to return nonaffine time

[Jan Beulich <jbeulich@xxxxxxxx>]

On 21.07.2025 11:49, Bernhard Kaindl wrote:
> --- a/xen/include/public/vcpu.h
> +++ b/xen/include/public/vcpu.h
> @@ -79,8 +79,18 @@ struct vcpu_runstate_info {
>      uint64_t time[4];
>  };
>  typedef struct vcpu_runstate_info vcpu_runstate_info_t;
> +/* vcpu_runstate_info_t is in the Guest shared memory area (frozen ABI) */
>  DEFINE_XEN_GUEST_HANDLE(vcpu_runstate_info_t);

Personally I'm opposed to this kind of comment.

> +/*
> + * Extra information returned from vcpu_runstate_get that is not part
> + * of the Guest shared memory area (not part of the frozen Guest ABI)
> + */
> +struct vcpu_runstate_extra {
> +    uint64_t nonaffine_time; /* Time running outside soft_affinity mask */
> +};
> +typedef struct vcpu_runstate_extra vcpu_runstate_extra_t;

And the next time we need some further piece of statistics, we'll have
to add yet another new sub-hypercall? Or wait, there is no new sub-
hypercall here. How's this piece of data going to make it out to guest
space then?

> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -1110,8 +1110,8 @@ int vcpu_set_hard_affinity(struct vcpu *v, const 
> cpumask_t *affinity);
>  int vcpu_affinity_domctl(struct domain *d, uint32_t cmd,
>                           struct xen_domctl_vcpuaffinity *vcpuaff);
>  
> -void vcpu_runstate_get(const struct vcpu *v,
> -                       struct vcpu_runstate_info *runstate);
> +vcpu_runstate_extra_t vcpu_runstate_get(const struct vcpu *v,
> +                                        struct vcpu_runstate_info *runstate);

As long as it's only a single 64-bit field in the struct, returning this
by value may be okay(ish). Yet I'd still recommend against doing so.

Also, having reached the end of the patch: Where's the caller making use
of this new return value?

Jan