On 10/7/19 7:35 AM, Juergen Gross wrote: > The credit scheduler is the only scheduler with tick_suspend and > tick_resume callbacks. Today those callbacks are invoked without being > guarded by the scheduler lock which is critical when at the same the > cpu those callbacks are active is being moved to or from a cpupool. > > Crashes like the following are possible due to that race: > > (XEN) ----[ Xen-4.13.0-8.0.12-d x86_64 debug=y Not tainted ]---- > (XEN) CPU: 79 > (XEN) RIP: e008:[<ffff82d0802467dc>] set_timer+0x39/0x1f7 > (XEN) RFLAGS: 0000000000010002 CONTEXT: hypervisor > <snip> > (XEN) Xen call trace: > (XEN) [<ffff82d0802467dc>] set_timer+0x39/0x1f7 > (XEN) [<ffff82d08022c1f4>] > sched_credit.c#csched_tick_resume+0x54/0x59 > (XEN) [<ffff82d080241dfe>] sched_tick_resume+0x67/0x86 > (XEN) [<ffff82d0802eda52>] mwait-idle.c#mwait_idle+0x32b/0x357 > (XEN) [<ffff82d08027939e>] domain.c#idle_loop+0xa6/0xc2 > (XEN) > (XEN) Pagetable walk from 0000000000000048: > (XEN) L4[0x000] = 00000082cfb9c063 ffffffffffffffff > (XEN) L3[0x000] = 00000082cfb9b063 ffffffffffffffff > (XEN) L2[0x000] = 00000082cfb9a063 ffffffffffffffff > (XEN) L1[0x000] = 0000000000000000 ffffffffffffffff > (XEN) > (XEN) **************************************** > (XEN) Panic on CPU 79: > (XEN) FATAL PAGE FAULT > (XEN) [error_code=0000] > (XEN) Faulting linear address: 0000000000000048 > (XEN) **************************************** > > The callbacks are used when the cpu is going to or coming from idle in > order to allow higher C-states. > > The credit scheduler knows when it is going to schedule an idle > scheduling unit or another one after idle, so it can easily stop or > resume the timer itself removing the need to do so via the callback. > As this timer handling is done in the main scheduling function the > scheduler lock is still held, so the race with cpupool operations can > no longer occur. Note that calling the callbacks from schedule_cpu_rm() > and schedule_cpu_add() is no longer needed, as the transitions to and > from idle in the cpupool with credit active will automatically occur > and do the right thing. > > With the last user of the callbacks gone those can be removed. > > Suggested-by: George Dunlap <[email protected]> > Signed-off-by: Juergen Gross <[email protected]> > --- > V2: > - instead of locking handle timer in credit only (George Dunlap) > --- > xen/common/sched_arinc653.c | 3 --- > xen/common/sched_credit.c | 34 ++++++++-------------------------- > xen/common/schedule.c | 26 ++------------------------ > xen/include/xen/sched-if.h | 15 --------------- > 4 files changed, 10 insertions(+), 68 deletions(-)
Like those diffstats. :-) Pushed, thanks. -George _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
