Re: [PATCH for-4.22? 1/9] sched: introduce specialization of "running only" vcpu_runstate_get()

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Mon, Jun 15, 2026 at 04:12:11PM +0200, Jan Beulich wrote:
> About half the callers of vcpu_runstate_get() are solely after the
> "running" time of a vCPU. Introduce a specialization with a smaller
> read critical section and thus reduced risk of a need for retries.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

Acked-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

> ---
> The function name was chosen such that grep-ing for "vcpu_runstate_get"
> would still turn up all uses. If that was deemed largely irrelevant, a
> better name might be e.g. vcpu_get_running_time().

FWIW, I think the fact that you can find call sites by using
vcpu_runstate_get is likely to become irrelevant in the long run.

> 
> --- a/xen/common/domctl.c
> +++ b/xen/common/domctl.c
> @@ -56,7 +56,6 @@ void getdomaininfo(struct domain *d, str
>      struct vcpu *v;
>      u64 cpu_time = 0;
>      int flags = XEN_DOMINF_blocked;
> -    struct vcpu_runstate_info runstate;
>  
>      memset(info, 0, sizeof(*info));
>  
> @@ -69,8 +68,7 @@ void getdomaininfo(struct domain *d, str
>       */
>      for_each_vcpu ( d, v )
>      {
> -        vcpu_runstate_get(v, &runstate);
> -        cpu_time += runstate.time[RUNSTATE_running];
> +        cpu_time += vcpu_runstate_get_running(v);
>          info->max_vcpu_id = v->vcpu_id;
>          if ( !(v->pause_flags & VPF_down) )
>          {
> @@ -829,8 +827,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xe
>  
>      case XEN_DOMCTL_getvcpuinfo:
>      {
> -        struct vcpu   *v;
> -        struct vcpu_runstate_info runstate;
> +        const struct vcpu *v;
>  
>          ret = -EINVAL;
>          if ( op->u.getvcpuinfo.vcpu >= d->max_vcpus )
> @@ -840,12 +837,10 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xe
>          if ( (v = d->vcpu[op->u.getvcpuinfo.vcpu]) == NULL )
>              break;
>  
> -        vcpu_runstate_get(v, &runstate);
> -
>          op->u.getvcpuinfo.online   = !(v->pause_flags & VPF_down);
>          op->u.getvcpuinfo.blocked  = !!(v->pause_flags & VPF_blocked);
>          op->u.getvcpuinfo.running  = v->is_running;
> -        op->u.getvcpuinfo.cpu_time = runstate.time[RUNSTATE_running];
> +        op->u.getvcpuinfo.cpu_time = vcpu_runstate_get_running(v);
>          op->u.getvcpuinfo.cpu      = v->processor;
>          ret = 0;
>          copyback = 1;
> --- a/xen/common/sched/core.c
> +++ b/xen/common/sched/core.c
> @@ -325,15 +325,35 @@ void vcpu_runstate_get(const struct vcpu
>      }
>  }
>  
> -uint64_t get_cpu_idle_time(unsigned int cpu)
> +uint64_t vcpu_runstate_get_running(const struct vcpu *v)
>  {
> -    struct vcpu_runstate_info state = { 0 };
> -    const struct vcpu *v = idle_vcpu[cpu];
> +    struct seqcount seq = SEQCNT_ZERO();
> +    const struct seqcount *s = v == current ? &seq : &v->runstate_seq;

Does it make a difference to use a local fake sequence counter or the
real one if the vCPU is the one currently running in this pCPU?  If
it's the running vCPU then it won't get the counters updated, and
hence using the real or a fake counter won't matter, as it will never
be updated while carrying out the read.  IOW: the usage of a local
sequence counter for that specific case just adds more logic without a
real benefit?

Thanks, Roger.