Re: [Xen-devel] [PATCH 1/6] xen/sched: call cpu_disable_scheduler() via cpu notifier

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 18/03/2019 13:11, Juergen Gross wrote:
> cpu_disable_scheduler() is being called from __cpu_disable() today.
> There is no need to call it on the cpu just being disabled, so use
> the CPU_DEAD case of the cpu notifier chain.
>
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> ---
>  xen/arch/x86/smpboot.c |  3 ---
>  xen/common/schedule.c  | 12 +++++-------
>  2 files changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
> index 7d1226d7bc..b7a0a4a419 100644
> --- a/xen/arch/x86/smpboot.c
> +++ b/xen/arch/x86/smpboot.c
> @@ -1221,9 +1221,6 @@ void __cpu_disable(void)
>      cpumask_clear_cpu(cpu, &cpu_online_map);
>      fixup_irqs(&cpu_online_map, 1);
>      fixup_eoi();
> -
> -    if ( cpu_disable_scheduler(cpu) )
> -        BUG();
>  }

It looks like ARM needs an equivalent adjustment.

>  
>  void __cpu_die(unsigned int cpu)
> diff --git a/xen/common/schedule.c b/xen/common/schedule.c
> index 60755a631e..665747f247 100644
> --- a/xen/common/schedule.c
> +++ b/xen/common/schedule.c
> @@ -773,8 +773,9 @@ void restore_vcpu_affinity(struct domain *d)
>  }
>  
>  /*
> - * This function is used by cpu_hotplug code from stop_machine context
> + * This function is used by cpu_hotplug code via cpu notifier chain
>   * and from cpupools to switch schedulers on a cpu.
> + * Caller must get domlist_read_lock.

s/get/hold/ ?

With at least the ARM side adjusted, Reviewed-by: Andrew Cooper
<andrew.cooper3@xxxxxxxxxx>

>   */
>  int cpu_disable_scheduler(unsigned int cpu)
>  {
> @@ -789,12 +790,6 @@ int cpu_disable_scheduler(unsigned int cpu)
>      if ( c == NULL )
>          return ret;
>  
> -    /*
> -     * We'd need the domain RCU lock, but:
> -     *  - when we are called from cpupool code, it's acquired there already;
> -     *  - when we are called for CPU teardown, we're in stop-machine context,
> -     *    so that's not be a problem.
> -     */
>      for_each_domain_in_cpupool ( d, c )
>      {
>          for_each_vcpu ( d, v )
> @@ -1738,6 +1733,9 @@ static int cpu_schedule_callback(
>          rc = cpu_schedule_up(cpu);
>          break;
>      case CPU_DEAD:
> +        rcu_read_lock(&domlist_read_lock);
> +        cpu_disable_scheduler(cpu);
> +        rcu_read_unlock(&domlist_read_lock);
>          SCHED_OP(sched, deinit_pdata, sd->sched_priv, cpu);
>          /* Fallthrough */
>      case CPU_UP_CANCELED:


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