Re: [Xen-devel] [PATCH v2] xen/sched: let credit scheduler control its timer all alone

[Dario Faggioli <dfaggioli@xxxxxxxx>]

On Mon, 2019-10-07 at 08:35 +0200, 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.
> 
Which is great! :-0

> Suggested-by: George Dunlap <george.dunlap@xxxxxxxxxx>
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
>
Well, unless I'm missing something, I guess that, at this point:

> --- a/xen/common/schedule.c
> +++ b/xen/common/schedule.c
> @@ -3082,32 +3078,14 @@ void schedule_dump(struct cpupool *c)
>  
>  void sched_tick_suspend(void)
>  {
> -    struct scheduler *sched;
> -    unsigned int cpu = smp_processor_id();
> -
> -    rcu_read_lock(&sched_res_rculock);
> -
> -    sched = get_sched_res(cpu)->scheduler;
> -    sched_do_tick_suspend(sched, cpu);
> -    rcu_idle_enter(cpu);
> +    rcu_idle_enter(smp_processor_id());
>      rcu_idle_timer_start();
> -
> -    rcu_read_unlock(&sched_res_rculock);
>  }
> 
sched_tick_suspend() could go away and rcu_idle_enter() be called
directly (with rcu_idle_timer_start() becoming static, and called
directly by rcu_idle_timer_enter() itself)

And the same for sched_tick_resume(), rcu_idle_timer_stop() and
rcu_idle_exit().

I'll give my:

Reviewed-by: Dario Faggioli <dfaggioli@xxxxxxxx>

To this patch, though, as I appreciate we want it in to be able to
continue testing core-scheduling during 4.13 rc-phase.

It'd be cool if the adjustments described above (if agreed upon), could
come as a follow-up.

Regards
-- 
Dario Faggioli, Ph.D
http://about.me/dario.faggioli
Virtualization Software Engineer
SUSE Labs, SUSE https://www.suse.com/
-------------------------------------------------------------------
<<This happens because _I_ choose it to happen!>> (Raistlin Majere)

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

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel