Re: [PATCH RESEND 1/4] xen: sched: introduce 'adjust_affinity' hook.

[George Dunlap <george.dunlap@citrix.com>]

On 03/15/2018 06:35 PM, Dario Faggioli wrote:
> From: Dario Faggioli <raistlin@linux.it>
> 
> For now, just as a way to give a scheduler an "heads up",
> about the fact that the affinity changed.
> 
> This enables some optimizations, such as pre-computing
> and storing (e.g., in flags) facts like a vcpu being
> exclusively pinned to a pcpu, or having or not a
> soft affinity. I.e., conditions that, despite the fact
> that they rarely change, are right now checked very
> frequently, even in hot paths.
> 
> Note that, as we expect many scheduler specific
> implementations of the adjust_affinity hook to do
> something with the per-scheduler vCPU private data,
> this commit moves the calls to sched_set_affinity()
> after that is allocated (in sched_init_vcpu()).
> 
> Note also that this, in future, may turn out as a useful
> mean for, e.g., having the schedulers vet, ack or nack
> the changes themselves.
> 
> Signed-off-by: Dario Faggioli <raistlin@linux.it>

Reviewed-by: George Dunlap <george.dunlap@citrix.com>

As I said, I can change all these on check-in.

 -George

> ---
> Cc: George Dunlap <george.dunlap@citrix.com>
> Cc: Anshul Makkar <anshulmakkar@gmail.com>
> ---
> Changes from last posting:
> - rebased on staging.
> 
> Changes from v2:
> - fix sched_set_affinity() not to use always hard_affinity;
> - move calls to sched_set_affinity() below per-scheduler vCPU data allocation.
> ---
>  xen/arch/x86/dom0_build.c  |    7 ++--
>  xen/common/schedule.c      |   75 ++++++++++++++++++++++++++++++++------------
>  xen/include/xen/sched-if.h |    3 ++
>  xen/include/xen/sched.h    |    3 ++
>  4 files changed, 63 insertions(+), 25 deletions(-)
> 
> diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
> index 555660b853..b744791c38 100644
> --- a/xen/arch/x86/dom0_build.c
> +++ b/xen/arch/x86/dom0_build.c
> @@ -140,14 +140,13 @@ struct vcpu *__init dom0_setup_vcpu(struct domain *d,
>      {
>          if ( pv_shim )
>          {
> -            __cpumask_set_cpu(vcpu_id, v->cpu_hard_affinity);
> -            __cpumask_set_cpu(vcpu_id, v->cpu_soft_affinity);
> +            sched_set_affinity(v, cpumask_of(vcpu_id), cpumask_of(vcpu_id));
>          }
>          else
>          {
>              if ( !d->is_pinned && !dom0_affinity_relaxed )
> -                cpumask_copy(v->cpu_hard_affinity, &dom0_cpus);
> -            cpumask_copy(v->cpu_soft_affinity, &dom0_cpus);
> +                sched_set_affinity(v, &dom0_cpus, NULL);
> +            sched_set_affinity(v, NULL, &dom0_cpus);
>          }
>      }
>  
> diff --git a/xen/common/schedule.c b/xen/common/schedule.c
> index 64524f4da7..f43d943765 100644
> --- a/xen/common/schedule.c
> +++ b/xen/common/schedule.c
> @@ -256,18 +256,11 @@ static void sched_spin_unlock_double(spinlock_t *lock1, spinlock_t *lock2,
>  int sched_init_vcpu(struct vcpu *v, unsigned int processor) 
>  {
>      struct domain *d = v->domain;
> +    cpumask_t allcpus;
>  
> -    /*
> -     * Initialize processor and affinity settings. The idler, and potentially
> -     * domain-0 VCPUs, are pinned onto their respective physical CPUs.
> -     */
> -    v->processor = processor;
> -    if ( is_idle_domain(d) || d->is_pinned )
> -        cpumask_copy(v->cpu_hard_affinity, cpumask_of(processor));
> -    else
> -        cpumask_setall(v->cpu_hard_affinity);
> +    cpumask_setall(&allcpus);
>  
> -    cpumask_setall(v->cpu_soft_affinity);
> +    v->processor = processor;
>  
>      /* Initialise the per-vcpu timers. */
>      init_timer(&v->periodic_timer, vcpu_periodic_timer_fn,
> @@ -282,6 +275,15 @@ int sched_init_vcpu(struct vcpu *v, unsigned int processor)
>      if ( v->sched_priv == NULL )
>          return 1;
>  
> +    /*
> +     * Initialize affinity settings. The idler, and potentially
> +     * domain-0 VCPUs, are pinned onto their respective physical CPUs.
> +     */
> +    if ( is_idle_domain(d) || d->is_pinned )
> +        sched_set_affinity(v, cpumask_of(processor), &allcpus);
> +    else
> +        sched_set_affinity(v, &allcpus, &allcpus);
> +
>      /* Idle VCPUs are scheduled immediately, so don't put them in runqueue. */
>      if ( is_idle_domain(d) )
>      {
> @@ -359,6 +361,7 @@ int sched_move_domain(struct domain *d, struct cpupool *c)
>      for_each_vcpu ( d, v )
>      {
>          spinlock_t *lock;
> +        cpumask_t allcpus;
>  
>          vcpudata = v->sched_priv;
>  
> @@ -366,10 +369,12 @@ int sched_move_domain(struct domain *d, struct cpupool *c)
>          migrate_timer(&v->singleshot_timer, new_p);
>          migrate_timer(&v->poll_timer, new_p);
>  
> -        cpumask_setall(v->cpu_hard_affinity);
> -        cpumask_setall(v->cpu_soft_affinity);
> +        cpumask_setall(&allcpus);
>  
>          lock = vcpu_schedule_lock_irq(v);
> +
> +        sched_set_affinity(v, &allcpus, &allcpus);
> +
>          v->processor = new_p;
>          /*
>           * With v->processor modified we must not
> @@ -694,7 +699,7 @@ void restore_vcpu_affinity(struct domain *d)
>  
>          if ( v->affinity_broken )
>          {
> -            cpumask_copy(v->cpu_hard_affinity, v->cpu_hard_affinity_saved);
> +            sched_set_affinity(v, v->cpu_hard_affinity_saved, NULL);
>              v->affinity_broken = 0;
>  
>          }
> @@ -758,6 +763,8 @@ int cpu_disable_scheduler(unsigned int cpu)
>              if ( cpumask_empty(&online_affinity) &&
>                   cpumask_test_cpu(cpu, v->cpu_hard_affinity) )
>              {
> +                cpumask_t allcpus;
> +
>                  if ( v->affinity_broken )
>                  {
>                      /* The vcpu is temporarily pinned, can't move it. */
> @@ -775,7 +782,8 @@ int cpu_disable_scheduler(unsigned int cpu)
>                  else
>                      printk(XENLOG_DEBUG "Breaking affinity for %pv\n", v);
>  
> -                cpumask_setall(v->cpu_hard_affinity);
> +                cpumask_setall(&allcpus);
> +                sched_set_affinity(v, &allcpus, NULL);
>              }
>  
>              if ( v->processor != cpu )
> @@ -845,8 +853,26 @@ int cpu_disable_scheduler(unsigned int cpu)
>      return ret;
>  }
>  
> +/*
> + * In general, this must be called with the scheduler lock held, because the
> + * adjust_affinity hook may want to modify the vCPU state. However, when the
> + * vCPU is being initialized (either for dom0 or domU) there is no risk of
> + * races, and it's fine to not take the look (we're talking about
> + * dom0_setup_vcpu() an sched_init_vcpu()).
> + */
> +void sched_set_affinity(
> +    struct vcpu *v, const cpumask_t *hard, const cpumask_t *soft)
> +{
> +    SCHED_OP(dom_scheduler(v->domain), adjust_affinity, v, hard, soft);
> +
> +    if ( hard )
> +        cpumask_copy(v->cpu_hard_affinity, hard);
> +    if ( soft )
> +        cpumask_copy(v->cpu_soft_affinity, soft);
> +}
> +
>  static int vcpu_set_affinity(
> -    struct vcpu *v, const cpumask_t *affinity, cpumask_t *which)
> +    struct vcpu *v, const cpumask_t *affinity, const cpumask_t *which)
>  {
>      spinlock_t *lock;
>      int ret = 0;
> @@ -857,12 +883,19 @@ static int vcpu_set_affinity(
>          ret = -EBUSY;
>      else
>      {
> -        cpumask_copy(which, affinity);
> -
>          /*
> -         * Always ask the scheduler to re-evaluate placement
> -         * when changing the affinity.
> +         * Tell the scheduler we changes something about affinity,
> +         * and ask to re-evaluate vcpu placement.
>           */
> +        if ( which == v->cpu_hard_affinity )
> +        {
> +            sched_set_affinity(v, affinity, NULL);
> +        }
> +        else
> +        {
> +            ASSERT(which == v->cpu_soft_affinity);
> +            sched_set_affinity(v, NULL, affinity);
> +        }
>          set_bit(_VPF_migrating, &v->pause_flags);
>      }
>  
> @@ -1100,7 +1133,7 @@ int vcpu_pin_override(struct vcpu *v, int cpu)
>      {
>          if ( v->affinity_broken )
>          {
> -            cpumask_copy(v->cpu_hard_affinity, v->cpu_hard_affinity_saved);
> +            sched_set_affinity(v, v->cpu_hard_affinity_saved, NULL);
>              v->affinity_broken = 0;
>              set_bit(_VPF_migrating, &v->pause_flags);
>              ret = 0;
> @@ -1114,7 +1147,7 @@ int vcpu_pin_override(struct vcpu *v, int cpu)
>          {
>              cpumask_copy(v->cpu_hard_affinity_saved, v->cpu_hard_affinity);
>              v->affinity_broken = 1;
> -            cpumask_copy(v->cpu_hard_affinity, cpumask_of(cpu));
> +            sched_set_affinity(v, cpumask_of(cpu), NULL);
>              set_bit(_VPF_migrating, &v->pause_flags);
>              ret = 0;
>          }
> diff --git a/xen/include/xen/sched-if.h b/xen/include/xen/sched-if.h
> index c5dd43ed9c..926d063ccf 100644
> --- a/xen/include/xen/sched-if.h
> +++ b/xen/include/xen/sched-if.h
> @@ -173,6 +173,9 @@ struct scheduler {
>                                      unsigned int);
>      int          (*adjust)         (const struct scheduler *, struct domain *,
>                                      struct xen_domctl_scheduler_op *);
> +    void         (*adjust_affinity)(const struct scheduler *, struct vcpu *,
> +                                    const struct cpumask *,
> +                                    const struct cpumask *);
>      int          (*adjust_global)  (const struct scheduler *,
>                                      struct xen_sysctl_scheduler_op *);
>      void         (*dump_settings)  (const struct scheduler *);
> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> index 39f938644a..ade4d7b9aa 100644
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -846,6 +846,9 @@ void scheduler_free(struct scheduler *sched);
>  int schedule_cpu_switch(unsigned int cpu, struct cpupool *c);
>  void vcpu_force_reschedule(struct vcpu *v);
>  int cpu_disable_scheduler(unsigned int cpu);
> +/* We need it in dom0_setup_vcpu */
> +void sched_set_affinity(struct vcpu *v, const cpumask_t *hard,
> +                        const cpumask_t *soft);
>  int vcpu_set_hard_affinity(struct vcpu *v, const cpumask_t *affinity);
>  int vcpu_set_soft_affinity(struct vcpu *v, const cpumask_t *affinity);
>  void restore_vcpu_affinity(struct domain *d);
> 


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