Re: [Xen-devel] [PATCH] xen: credit2: fix spinlock irq-safety violation

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On 09/19/2017 03:11 AM, Dario Faggioli wrote:
> In commit ad4b3e1e9df34 ("xen: credit2: implement
> utilization cap") xfree() was being called (for
> deallocating the budget replenishment timer, during
> domain destruction) inside an IRQ disabled critical
> section.
> 
> That must not happen, as it uses the mem-pool's lock,
> which needs to be taken with IRQ enabled. And, in fact,
> we crash (in debug builds):
> 
> (XEN) ****************************************
> (XEN) Panic on CPU 0:
> (XEN) Xen BUG at spinlock.c:47
> (XEN) ****************************************
> 
> Let's, therefore, kill and deallocate the timer outside of
> the critical sections, when IRQs are enabled.
> 
> Signed-off-by: Dario Faggioli <dario.faggioli@xxxxxxxxxx>
> ---
> Cc: osstest service owner <osstest-admin@xxxxxxxxxxxxxx>
> Cc: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
> Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
> ---
> This was spotted by OSSTest's flight 113562:
> 
>  http://logs.test-lab.xenproject.org/osstest/logs/113562/
>  
> http://logs.test-lab.xenproject.org/osstest/logs/113562/test-amd64-amd64-xl-credit2/serial-godello0.log
> ---
>  xen/common/sched_credit2.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
> index 32234ac..7a550db 100644
> --- a/xen/common/sched_credit2.c
> +++ b/xen/common/sched_credit2.c
> @@ -2923,13 +2923,13 @@ csched2_free_domdata(const struct scheduler *ops, 
> void *data)
>  
>      write_lock_irqsave(&prv->lock, flags);
>  
> -    kill_timer(sdom->repl_timer);
> -    xfree(sdom->repl_timer);
> -
>      list_del_init(&sdom->sdom_elem);
>  
>      write_unlock_irqrestore(&prv->lock, flags);
>  
> +    kill_timer(sdom->repl_timer);
> +    xfree(sdom->repl_timer);

Any particular reason for moving the kill_timer() as well as the xfree
outside the lock?  What happens if the timer goes off after the
irqrestore but before the kill_timer?

 -George

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