Re: [PATCH] xen/sched: refactor urgent count update

[Jan Beulich <jbeulich@xxxxxxxx>]

On 25.09.2026 23:28, Ruslan Ruslichenko wrote:
> From: Ruslan Ruslichenko <Ruslan_Ruslichenko@xxxxxxxx>
> 
> With current implementation whenever vCPU blocks on
> waiting for event channel via SCHEDOP_poll hypercall,
> scheduler sets 'is_urgent' flag. The flag is cleared
> on vCPU runstate change, if it is no longer in
> 'VPF_blocked' state or its vcpu_id is not in poll_mask.
> 
> However, the only way for vCPU runstate to change in
> polled state is when it's going to wakeup.
> 
> Thus when 'is_urgent' is set, inner condition will always be:
> 
> if ( unlikely(v->is_urgent) )
> {
>     if ( !(v->pause_flags & VPF_blocked) ||            // Always True
>          !test_bit(v->vcpu_id, v->domain->poll_mask) ) // Never evaluated

Maybe this invariant indeed applies, but then there must be more to it.
Simply from taking the "else" branch here

        if ( unlikely(v->pause_flags & VPF_blocked) &&
             unlikely(test_bit(v->vcpu_id, v->domain->poll_mask)) )
        {
            v->is_urgent = 1;
            atomic_inc(&per_cpu(sched_urgent_count, v->processor));
        }

clearly immediately afterwards the invariant you name does not hold.

> --- a/xen/common/sched/core.c
> +++ b/xen/common/sched/core.c
> @@ -241,26 +241,22 @@ static inline void trace_continue_running(const struct 
> vcpu *v)
>  
>  static inline void vcpu_urgent_count_update(struct vcpu *v)
>  {
> +    bool cur_urgent;
> +
>      if ( is_idle_vcpu(v) )
>          return;
>  
> -    if ( unlikely(v->is_urgent) )
> -    {
> -        if ( !(v->pause_flags & VPF_blocked) ||
> -             !test_bit(v->vcpu_id, v->domain->poll_mask) )
> -        {
> -            v->is_urgent = 0;
> -            atomic_dec(&per_cpu(sched_urgent_count, v->processor));
> -        }
> -    }
> -    else
> +    cur_urgent = (v->pause_flags & VPF_blocked) &&
> +                 test_bit(v->vcpu_id, v->domain->poll_mask);
> +
> +    if ( unlikely(v->is_urgent != cur_urgent) )
>      {
> -        if ( unlikely(v->pause_flags & VPF_blocked) &&
> -             unlikely(test_bit(v->vcpu_id, v->domain->poll_mask)) )
> -        {
> -            v->is_urgent = 1;
> +        v->is_urgent = cur_urgent;
> +
> +        if ( cur_urgent )
>              atomic_inc(&per_cpu(sched_urgent_count, v->processor));
> -        }
> +        else
> +            atomic_dec(&per_cpu(sched_urgent_count, v->processor));

Assuming the transformation is valid to make (scheduler maintainers will
need to judge), may I suggest to fold these two into a single atomic_add(),
passing in either +1 or -1?

Jan