Re: [Xen-devel] [PATCH v2 31/48] xen/sched: add support for multiple vcpus per sched unit where missing

[Jan Beulich <jbeulich@xxxxxxxx>]

On 09.08.2019 16:58, Juergen Gross wrote:
> V1:
> - add special handling for idle unit in unit_runnable() and
>   unit_runnable_state()

Why was this done? Isn't vcpu_runnable() going to always return
true for idle vCPU-s?

> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -1255,7 +1255,10 @@ int vcpu_reset(struct vcpu *v)
>      v->async_exception_mask = 0;
>      memset(v->async_exception_state, 0, sizeof(v->async_exception_state));
>  #endif
> -    v->affinity_broken = 0;
> +    if ( v->affinity_broken & VCPU_AFFINITY_OVERRIDE )
> +        vcpu_temporary_affinity(v, NR_CPUS, VCPU_AFFINITY_OVERRIDE);
> +    if ( v->affinity_broken & VCPU_AFFINITY_WAIT )
> +        vcpu_temporary_affinity(v, NR_CPUS, VCPU_AFFINITY_WAIT);

Shouldn't this live in an earlier patch? I guess the same question
is applicable to almost everything here, as also already mentioned
e.g. in the previous patch.

>  static inline void sched_set_res(struct sched_unit *unit,
>                                   struct sched_resource *res)
>  {
> -    unit->vcpu_list->processor = res->processor;
> +    int cpu = cpumask_first(res->cpus);

unsigned int

>  static inline void sched_set_pause_flags_atomic(struct sched_unit *unit,
>                                                  unsigned int bit)
>  {
> -    set_bit(bit, &unit->vcpu_list->pause_flags);
> +    struct vcpu *v;
> +
> +    for_each_sched_unit_vcpu ( unit, v )
> +        set_bit(bit, &v->pause_flags);
>  }
>  
>  static inline void sched_clear_pause_flags_atomic(struct sched_unit *unit,
>                                                    unsigned int bit)
>  {
> -    clear_bit(bit, &unit->vcpu_list->pause_flags);
> +    struct vcpu *v;
> +
> +    for_each_sched_unit_vcpu ( unit, v )
> +        clear_bit(bit, &v->pause_flags);
>  }

The atomicity is (necessarily) limited here - it's atomic for an
individual vCPU, but not atomic for a unit as a whole. If this
is indeed a useful hybrid, then I think it would be nice if there
was a comment next to these functions clarifying under what
conditions use of them is correct without it also being correct
to use their non-atomic counterparts (e.g. due to use of an
external lock).

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel