Re: [Xen-devel] [PATCH] xen: credit1: fix a race when picking initial pCPU for a vCPU

[Dario Faggioli <dario.faggioli@xxxxxxxxxx>]

On Fri, 2016-08-12 at 10:14 +0100, George Dunlap wrote:
> On 12/08/16 05:07, Dario Faggioli wrote:

> Let me know if you want me to check this in as-is or if you think you
> might send a follow-up patch adding an ASSERT.
> 
Done, and it actually explodes like this:

(XEN) [    4.870128] Xen call trace:
(XEN) [    4.870130]    [<ffff82d080131cba>] spinlock.c#check_lock+0x42/0x46
(XEN) [    4.870133]    [<ffff82d080131db2>] _spin_is_locked+0x11/0x4d
(XEN) [    4.870139]    [<ffff82d080126c2b>] 
sched_credit.c#_csched_cpu_pick+0x1a9/0x632
(XEN) [    4.870142]    [<ffff82d08012747f>] 
sched_credit.c#csched_tick+0x1fd/0x385
(XEN) [    4.870146]    [<ffff82d080134a16>] timer.c#execute_timer+0x47/0x62
(XEN) [    4.870148]    [<ffff82d080134b0c>] 
timer.c#timer_softirq_action+0xdb/0x22c
(XEN) [    4.870151]    [<ffff82d080131487>] softirq.c#__do_softirq+0x7f/0x8a
(XEN) [    4.870153]    [<ffff82d0801314dc>] do_softirq+0x13/0x15
(XEN) [    4.870157]    [<ffff82d080243e01>] entry.o#process_softirqs+0x21/0x30
(XEN) [    4.870159] 
(XEN) [    5.619096] 
(XEN) [    5.621085] ****************************************
(XEN) [    5.626536] Panic on CPU 0:
(XEN) [    5.629826] Xen BUG at spinlock.c:48
(XEN) [    5.633895] ****************************************

And if I look at csched_tick(), it indeed is the case that we
call csched_vcpu_acct() **without** holding the runq lock.

It in turns calls things like burn_credits(), accesses current, and
other stuff, which I'm having a little bit of an hard time convincing
myself it is safe... Although it must be, if there have been no issues
after all these years. :-O

csched_runq_sort(), called later, still by csched_tick(), acquires the
lock by itself, and we can't acquire it in csched_tick(), because
__csched_vcpu_acct_start() acquires the private lock, and we'd violate
the nesting rule.

In summary, this is looking more complicated than it seemed, and I'll
have to look at this again on Tuesday (it's public holiday, here, on
Monday).

Gosh, how much I hate this scheduler!! :-/

Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

Attachment: signature.asc
Description: This is a digitally signed message part

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