On Thu, 2018-05-10 at 13:57 +0200, Mirela Simonovic wrote:
> Hi,
> 
> +Dario
> 
Thanks.

> On Wed, May 9, 2018 at 6:32 PM, Julien Grall <julien.grall@arm.com>
> wrote:
> > If there is a bug in the scheduler it should be fixed rather trying
> > to
> > workaround with a panic in the code. If you provide more details,
> > we might
> > be able to help here.
> > 
> 
> This flow seems to have several bugs. Lets start from here:
> 
> Please take a look at function cpu_schedule_callback in schedule.c.
> Within switch, case CPU_DEAD doesn't have a break, causing the bellow
> CPU_UP_CANCELED to execute as well when the CPU goes down. This looks
> wrong to me.
> Dario, could you please confirm that this is a bug? 
>
No, that is entirely on purpose (as you can tell from the
"/* Fallthrough */" comment).

> Otherwise could
> you please confirm the reasoning beyond?
> 
Well, the idea is that the CPU_UP_CANCELLED notifier should _only_
invoke cpu_schedule_down(). On the other hand, the CPU_DEAD notifier
should invoke both:
 SCHED_OP(deinit_pdata)
 cpu_schedule_down()

and the fallthrough is the way we achieve that.

On x86, the shutdown/suspend path is as follows:

    XENPF_enter_acpi_sleep (platform_hypercall.c)
      acpi_enter_sleep()
        continue_hypercall_on_cpu(0, enter_state_helper)
          enter_state()
            system_state = SYS_STATE_suspend
            disable_nonboot_cpus()
              for_each_online_cpu ( cpu ) { 
                cpu_down(cpu) }
                  notifier_call_chain(CPU_DOWN_PREPARE)
                    cpu_callback(CPU_DOWN_PREPARE)
                      cpupool_cpu_remove(cpu)
                      cpufreq_del_cpu(cpu)
                      stop_machine_run(take_cpu_down, cpu)
                        __cpu_disable();
                          notifier_call_chain(CPU_DYING)
                            ...
                          cpumask_clear_cpu(cpu, &cpu_online_map);
                            cpu_disable_scheduler(cpu);
                  __cpu_die(cpu)
                    while ( (seen_state = cpu_state) != CPU_STATE_DEAD ) { ... }
                    notifier_call_chain(CPU_DEAD)
                      cpu_schedule_callback(CPU_DEAD)
                        ...
                        ...
                        cpu_schedule_callback(CPU_DEAD)
                          SCHED_OP(deinit_pdata)
                          cpu_schedule_down(cpu)
                            SCHED_OP(free_pdata)
                            SCHED_OP(free_vdata)


If you see a BUG_ON triggering, please provide details about that, and
we'll try to figure out what's causing it.

Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Software Engineer @ SUSE https://www.suse.com/