| Message ID | 150595383763.32567.13168251884004584759.stgit@Solace.fritz.box (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Sep 21, 2017 at 02:30:37AM +0200, 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@citrix.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Thanks, Roger.
On 09/21/2017 01:30 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@citrix.com> Reviewed-by: George Dunlap <george.dunlap@citrix.com>
On Thu, Sep 21, 2017 at 02:30:37AM +0200, 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@citrix.com> > --- Reviewed-by: Wei Liu <wei.liu2@citrix.com>
diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c index 32234ac..015e45e 100644 --- a/xen/common/sched_credit2.c +++ b/xen/common/sched_credit2.c @@ -2921,15 +2921,15 @@ csched2_free_domdata(const struct scheduler *ops, void *data) struct csched2_dom *sdom = data; struct csched2_private *prv = csched2_priv(ops); - write_lock_irqsave(&prv->lock, flags); - kill_timer(sdom->repl_timer); - xfree(sdom->repl_timer); + + write_lock_irqsave(&prv->lock, flags); list_del_init(&sdom->sdom_elem); write_unlock_irqrestore(&prv->lock, flags); + xfree(sdom->repl_timer); xfree(data); }
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@citrix.com> --- Cc: osstest service owner <osstest-admin@xenproject.org> Cc: George Dunlap <george.dunlap@eu.citrix.com> Cc: Wei Liu <wei.liu2@citrix.com> Cc: Roger Pau Monné <roger.pau@citrix.com> Cc: Jan Beulich <JBeulich@suse.com> --- 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 --- Changes from v2: - kill_timer() stays above the critical section, while xfree() goes below it, to avoid having a domain in the linked list of domains with a NULL timer. Changes from v1: - kill_timer() and xfree() moved above critical section, instead than below. --- xen/common/sched_credit2.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)