Re: xen/arm: Assertion 'timer->status >= TIMER_STATUS_inactive' failed at timer.c:279

[Julien Grall <julien.grall@arm.com>]

On 26/04/16 18:49, Dario Faggioli wrote:
> On Tue, 2016-04-26 at 15:25 +0100, Julien Grall wrote:
> Hi,

Hi Dario,

[...]

> commit eca4d65fb67a71c0f6563aafbfdd68e566c53c32
> Author: Dario Faggioli <dario.faggioli@citrix.com>
> Date:   Tue Apr 26 17:42:36 2016 +0200
>
>      xen: sched: fix killing an uninitialized timer in free_pdata.
>
>      commit 64269d9365 "sched: implement .init_pdata in Credit,
>      Credit2 and RTDS" helped fixing Credit2 runqueues, and
>      the races we had in switching scheduler for pCPUs, but
>      introduced another issue. In fact, if CPU bringup fails
>      during __cpu_up() (and, more precisely, after CPU_UP_PREPARE,
>      but before CPU_STARTING) the CPU_UP_CANCELED notifier
>      would be executed, which calls the free_pdata hook.
>
>      Such hook does two things: (1) undo the initialization
>      done inside the init_pdata hook; (2) free the memory
>      allocated by the alloc_pdata hook.
>
>      However, in the failure path just described, it is possible
>      that only alloc_pdata has really been called, which is
>      potentially and issue (depending on what actually happens
>      inside the implementation of free_pdata).
>
>      In fact, for Credit1 (the only scheduler that actually
>      allocates per-pCPU data), this result in calling kill_timer()
>      on a timer that had not yet been initialized, which causes
>      the following:
>
>      (XEN) Xen call trace:
>      (XEN)    [<000000000022e304>] timer.c#active_timer+0x8/0x24 (PC)
>      (XEN)    [<000000000022f624>] kill_timer+0x108/0x2e0 (LR)
>      (XEN)    [<00000000002208c0>] sched_credit.c#csched_free_pdata+0xd8/0x114
>      (XEN)    [<0000000000227a18>] schedule.c#cpu_schedule_callback+0xc0/0x12c
>      (XEN)    [<0000000000219944>] notifier_call_chain+0x78/0x9c
>      (XEN)    [<00000000002015fc>] cpu_up+0x104/0x130
>      (XEN)    [<000000000028f7c0>] start_xen+0xaf8/0xce0
>      (XEN)    [<00000000810021d8>] 00000000810021d8
>      (XEN)
>      (XEN)
>      (XEN) ****************************************
>      (XEN) Panic on CPU 0:
>      (XEN) Assertion 'timer->status >= TIMER_STATUS_inactive' failed at timer.c:279
>      (XEN) ****************************************
>
>      Solve this by making the scheduler hooks API symmetric again,
>      i.e., by adding an deinit_pdata hook and making it responsible
>      of undoing what init_pdata did, rather than asking to free_pdata
>      to do everything.
>
>      This is cleaner and, in the case at hand, makes it possible to
>      only call free_pdata, which is the right thing to do, as only
>      allocation and no initialization was performed.
>
>      Reported-by: Julien Grall <julien.grall@arm.com>
>      Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
>      ---
>      Cc: George Dunlap <george.dunlap@citrix.com>
>      Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>      Cc: Varun.Swara@arm.com
>      Cc: Steve Capper <Steve.Capper@arm.com>

Thank you for the patch. It fixes the bug when secondary CPUs fail to 
come online on the Foundation Model:

Tested-by: Julien Grall <julien.grall@arm.com>

Regards,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel