mirror of
				https://github.com/torvalds/linux.git
				synced 2025-10-31 16:48:26 +02:00 
			
		
		
		
	sched/core: Initialize the idle task with preemption disabled
As pointed out by commit
  de9b8f5dcb ("sched: Fix crash trying to dequeue/enqueue the idle thread")
init_idle() can and will be invoked more than once on the same idle
task. At boot time, it is invoked for the boot CPU thread by
sched_init(). Then smp_init() creates the threads for all the secondary
CPUs and invokes init_idle() on them.
As the hotplug machinery brings the secondaries to life, it will issue
calls to idle_thread_get(), which itself invokes init_idle() yet again.
In this case it's invoked twice more per secondary: at _cpu_up(), and at
bringup_cpu().
Given smp_init() already initializes the idle tasks for all *possible*
CPUs, no further initialization should be required. Now, removing
init_idle() from idle_thread_get() exposes some interesting expectations
with regards to the idle task's preempt_count: the secondary startup always
issues a preempt_disable(), requiring some reset of the preempt count to 0
between hot-unplug and hotplug, which is currently served by
idle_thread_get() -> idle_init().
Given the idle task is supposed to have preemption disabled once and never
see it re-enabled, it seems that what we actually want is to initialize its
preempt_count to PREEMPT_DISABLED and leave it there. Do that, and remove
init_idle() from idle_thread_get().
Secondary startups were patched via coccinelle:
  @begone@
  @@
  -preempt_disable();
  ...
  cpu_startup_entry(CPUHP_AP_ONLINE_IDLE);
Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Acked-by: Peter Zijlstra <peterz@infradead.org>
Link: https://lore.kernel.org/r/20210512094636.2958515-1-valentin.schneider@arm.com
			
			
This commit is contained in:
		
							parent
							
								
									9f26990074
								
							
						
					
					
						commit
						f1a0a376ca
					
				
					 25 changed files with 8 additions and 34 deletions
				
			
		|  | @ -166,7 +166,6 @@ smp_callin(void) | ||||||
| 	DBGS(("smp_callin: commencing CPU %d current %p active_mm %p\n", | 	DBGS(("smp_callin: commencing CPU %d current %p active_mm %p\n", | ||||||
| 	      cpuid, current, current->active_mm)); | 	      cpuid, current, current->active_mm)); | ||||||
| 
 | 
 | ||||||
| 	preempt_disable(); |  | ||||||
| 	cpu_startup_entry(CPUHP_AP_ONLINE_IDLE); | 	cpu_startup_entry(CPUHP_AP_ONLINE_IDLE); | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
|  | @ -189,7 +189,6 @@ void start_kernel_secondary(void) | ||||||
| 	pr_info("## CPU%u LIVE ##: Executing Code...\n", cpu); | 	pr_info("## CPU%u LIVE ##: Executing Code...\n", cpu); | ||||||
| 
 | 
 | ||||||
| 	local_irq_enable(); | 	local_irq_enable(); | ||||||
| 	preempt_disable(); |  | ||||||
| 	cpu_startup_entry(CPUHP_AP_ONLINE_IDLE); | 	cpu_startup_entry(CPUHP_AP_ONLINE_IDLE); | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
|  | @ -432,7 +432,6 @@ asmlinkage void secondary_start_kernel(void) | ||||||
| #endif | #endif | ||||||
| 	pr_debug("CPU%u: Booted secondary processor\n", cpu); | 	pr_debug("CPU%u: Booted secondary processor\n", cpu); | ||||||
| 
 | 
 | ||||||
| 	preempt_disable(); |  | ||||||
| 	trace_hardirqs_off(); | 	trace_hardirqs_off(); | ||||||
| 
 | 
 | ||||||
| 	/*
 | 	/*
 | ||||||
|  |  | ||||||
|  | @ -23,7 +23,7 @@ static inline void preempt_count_set(u64 pc) | ||||||
| } while (0) | } while (0) | ||||||
| 
 | 
 | ||||||
| #define init_idle_preempt_count(p, cpu) do { \ | #define init_idle_preempt_count(p, cpu) do { \ | ||||||
| 	task_thread_info(p)->preempt_count = PREEMPT_ENABLED; \ | 	task_thread_info(p)->preempt_count = PREEMPT_DISABLED; \ | ||||||
| } while (0) | } while (0) | ||||||
| 
 | 
 | ||||||
| static inline void set_preempt_need_resched(void) | static inline void set_preempt_need_resched(void) | ||||||
|  |  | ||||||
|  | @ -224,7 +224,6 @@ asmlinkage notrace void secondary_start_kernel(void) | ||||||
| 		init_gic_priority_masking(); | 		init_gic_priority_masking(); | ||||||
| 
 | 
 | ||||||
| 	rcu_cpu_starting(cpu); | 	rcu_cpu_starting(cpu); | ||||||
| 	preempt_disable(); |  | ||||||
| 	trace_hardirqs_off(); | 	trace_hardirqs_off(); | ||||||
| 
 | 
 | ||||||
| 	/*
 | 	/*
 | ||||||
|  |  | ||||||
|  | @ -281,7 +281,6 @@ void csky_start_secondary(void) | ||||||
| 	pr_info("CPU%u Online: %s...\n", cpu, __func__); | 	pr_info("CPU%u Online: %s...\n", cpu, __func__); | ||||||
| 
 | 
 | ||||||
| 	local_irq_enable(); | 	local_irq_enable(); | ||||||
| 	preempt_disable(); |  | ||||||
| 	cpu_startup_entry(CPUHP_AP_ONLINE_IDLE); | 	cpu_startup_entry(CPUHP_AP_ONLINE_IDLE); | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
|  | @ -441,7 +441,6 @@ start_secondary (void *unused) | ||||||
| #endif | #endif | ||||||
| 	efi_map_pal_code(); | 	efi_map_pal_code(); | ||||||
| 	cpu_init(); | 	cpu_init(); | ||||||
| 	preempt_disable(); |  | ||||||
| 	smp_callin(); | 	smp_callin(); | ||||||
| 
 | 
 | ||||||
| 	cpu_startup_entry(CPUHP_AP_ONLINE_IDLE); | 	cpu_startup_entry(CPUHP_AP_ONLINE_IDLE); | ||||||
|  |  | ||||||
|  | @ -348,7 +348,6 @@ asmlinkage void start_secondary(void) | ||||||
| 	 */ | 	 */ | ||||||
| 
 | 
 | ||||||
| 	calibrate_delay(); | 	calibrate_delay(); | ||||||
| 	preempt_disable(); |  | ||||||
| 	cpu = smp_processor_id(); | 	cpu = smp_processor_id(); | ||||||
| 	cpu_data[cpu].udelay_val = loops_per_jiffy; | 	cpu_data[cpu].udelay_val = loops_per_jiffy; | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
|  | @ -145,8 +145,6 @@ asmlinkage __init void secondary_start_kernel(void) | ||||||
| 	set_cpu_online(cpu, true); | 	set_cpu_online(cpu, true); | ||||||
| 
 | 
 | ||||||
| 	local_irq_enable(); | 	local_irq_enable(); | ||||||
| 
 |  | ||||||
| 	preempt_disable(); |  | ||||||
| 	/*
 | 	/*
 | ||||||
| 	 * OK, it's off to the idle thread for us | 	 * OK, it's off to the idle thread for us | ||||||
| 	 */ | 	 */ | ||||||
|  |  | ||||||
|  | @ -302,7 +302,6 @@ void __init smp_callin(unsigned long pdce_proc) | ||||||
| #endif | #endif | ||||||
| 
 | 
 | ||||||
| 	smp_cpu_init(slave_id); | 	smp_cpu_init(slave_id); | ||||||
| 	preempt_disable(); |  | ||||||
| 
 | 
 | ||||||
| 	flush_cache_all_local(); /* start with known state */ | 	flush_cache_all_local(); /* start with known state */ | ||||||
| 	flush_tlb_all_local(NULL); | 	flush_tlb_all_local(NULL); | ||||||
|  |  | ||||||
|  | @ -1547,7 +1547,6 @@ void start_secondary(void *unused) | ||||||
| 	smp_store_cpu_info(cpu); | 	smp_store_cpu_info(cpu); | ||||||
| 	set_dec(tb_ticks_per_jiffy); | 	set_dec(tb_ticks_per_jiffy); | ||||||
| 	rcu_cpu_starting(cpu); | 	rcu_cpu_starting(cpu); | ||||||
| 	preempt_disable(); |  | ||||||
| 	cpu_callin_map[cpu] = 1; | 	cpu_callin_map[cpu] = 1; | ||||||
| 
 | 
 | ||||||
| 	if (smp_ops->setup_cpu) | 	if (smp_ops->setup_cpu) | ||||||
|  |  | ||||||
|  | @ -180,7 +180,6 @@ asmlinkage __visible void smp_callin(void) | ||||||
| 	 * Disable preemption before enabling interrupts, so we don't try to | 	 * Disable preemption before enabling interrupts, so we don't try to | ||||||
| 	 * schedule a CPU that hasn't actually started yet. | 	 * schedule a CPU that hasn't actually started yet. | ||||||
| 	 */ | 	 */ | ||||||
| 	preempt_disable(); |  | ||||||
| 	local_irq_enable(); | 	local_irq_enable(); | ||||||
| 	cpu_startup_entry(CPUHP_AP_ONLINE_IDLE); | 	cpu_startup_entry(CPUHP_AP_ONLINE_IDLE); | ||||||
| } | } | ||||||
|  |  | ||||||
|  | @ -32,7 +32,7 @@ static inline void preempt_count_set(int pc) | ||||||
| #define init_task_preempt_count(p)	do { } while (0) | #define init_task_preempt_count(p)	do { } while (0) | ||||||
| 
 | 
 | ||||||
| #define init_idle_preempt_count(p, cpu)	do { \ | #define init_idle_preempt_count(p, cpu)	do { \ | ||||||
| 	S390_lowcore.preempt_count = PREEMPT_ENABLED; \ | 	S390_lowcore.preempt_count = PREEMPT_DISABLED; \ | ||||||
| } while (0) | } while (0) | ||||||
| 
 | 
 | ||||||
| static inline void set_preempt_need_resched(void) | static inline void set_preempt_need_resched(void) | ||||||
|  | @ -91,7 +91,7 @@ static inline void preempt_count_set(int pc) | ||||||
| #define init_task_preempt_count(p)	do { } while (0) | #define init_task_preempt_count(p)	do { } while (0) | ||||||
| 
 | 
 | ||||||
| #define init_idle_preempt_count(p, cpu)	do { \ | #define init_idle_preempt_count(p, cpu)	do { \ | ||||||
| 	S390_lowcore.preempt_count = PREEMPT_ENABLED; \ | 	S390_lowcore.preempt_count = PREEMPT_DISABLED; \ | ||||||
| } while (0) | } while (0) | ||||||
| 
 | 
 | ||||||
| static inline void set_preempt_need_resched(void) | static inline void set_preempt_need_resched(void) | ||||||
|  |  | ||||||
|  | @ -878,7 +878,6 @@ static void smp_init_secondary(void) | ||||||
| 	restore_access_regs(S390_lowcore.access_regs_save_area); | 	restore_access_regs(S390_lowcore.access_regs_save_area); | ||||||
| 	cpu_init(); | 	cpu_init(); | ||||||
| 	rcu_cpu_starting(cpu); | 	rcu_cpu_starting(cpu); | ||||||
| 	preempt_disable(); |  | ||||||
| 	init_cpu_timer(); | 	init_cpu_timer(); | ||||||
| 	vtime_init(); | 	vtime_init(); | ||||||
| 	vdso_getcpu_init(); | 	vdso_getcpu_init(); | ||||||
|  |  | ||||||
|  | @ -186,8 +186,6 @@ asmlinkage void start_secondary(void) | ||||||
| 
 | 
 | ||||||
| 	per_cpu_trap_init(); | 	per_cpu_trap_init(); | ||||||
| 
 | 
 | ||||||
| 	preempt_disable(); |  | ||||||
| 
 |  | ||||||
| 	notify_cpu_starting(cpu); | 	notify_cpu_starting(cpu); | ||||||
| 
 | 
 | ||||||
| 	local_irq_enable(); | 	local_irq_enable(); | ||||||
|  |  | ||||||
|  | @ -348,7 +348,6 @@ static void sparc_start_secondary(void *arg) | ||||||
| 	 */ | 	 */ | ||||||
| 	arch_cpu_pre_starting(arg); | 	arch_cpu_pre_starting(arg); | ||||||
| 
 | 
 | ||||||
| 	preempt_disable(); |  | ||||||
| 	cpu = smp_processor_id(); | 	cpu = smp_processor_id(); | ||||||
| 
 | 
 | ||||||
| 	notify_cpu_starting(cpu); | 	notify_cpu_starting(cpu); | ||||||
|  |  | ||||||
|  | @ -138,9 +138,6 @@ void smp_callin(void) | ||||||
| 
 | 
 | ||||||
| 	set_cpu_online(cpuid, true); | 	set_cpu_online(cpuid, true); | ||||||
| 
 | 
 | ||||||
| 	/* idle thread is expected to have preempt disabled */ |  | ||||||
| 	preempt_disable(); |  | ||||||
| 
 |  | ||||||
| 	local_irq_enable(); | 	local_irq_enable(); | ||||||
| 
 | 
 | ||||||
| 	cpu_startup_entry(CPUHP_AP_ONLINE_IDLE); | 	cpu_startup_entry(CPUHP_AP_ONLINE_IDLE); | ||||||
|  |  | ||||||
|  | @ -44,7 +44,7 @@ static __always_inline void preempt_count_set(int pc) | ||||||
| #define init_task_preempt_count(p) do { } while (0) | #define init_task_preempt_count(p) do { } while (0) | ||||||
| 
 | 
 | ||||||
| #define init_idle_preempt_count(p, cpu) do { \ | #define init_idle_preempt_count(p, cpu) do { \ | ||||||
| 	per_cpu(__preempt_count, (cpu)) = PREEMPT_ENABLED; \ | 	per_cpu(__preempt_count, (cpu)) = PREEMPT_DISABLED; \ | ||||||
| } while (0) | } while (0) | ||||||
| 
 | 
 | ||||||
| /*
 | /*
 | ||||||
|  |  | ||||||
|  | @ -236,7 +236,6 @@ static void notrace start_secondary(void *unused) | ||||||
| 	cpu_init(); | 	cpu_init(); | ||||||
| 	rcu_cpu_starting(raw_smp_processor_id()); | 	rcu_cpu_starting(raw_smp_processor_id()); | ||||||
| 	x86_cpuinit.early_percpu_clock_init(); | 	x86_cpuinit.early_percpu_clock_init(); | ||||||
| 	preempt_disable(); |  | ||||||
| 	smp_callin(); | 	smp_callin(); | ||||||
| 
 | 
 | ||||||
| 	enable_start_cpu0 = 0; | 	enable_start_cpu0 = 0; | ||||||
|  |  | ||||||
|  | @ -145,7 +145,6 @@ void secondary_start_kernel(void) | ||||||
| 	cpumask_set_cpu(cpu, mm_cpumask(mm)); | 	cpumask_set_cpu(cpu, mm_cpumask(mm)); | ||||||
| 	enter_lazy_tlb(mm, current); | 	enter_lazy_tlb(mm, current); | ||||||
| 
 | 
 | ||||||
| 	preempt_disable(); |  | ||||||
| 	trace_hardirqs_off(); | 	trace_hardirqs_off(); | ||||||
| 
 | 
 | ||||||
| 	calibrate_delay(); | 	calibrate_delay(); | ||||||
|  |  | ||||||
|  | @ -29,7 +29,7 @@ static __always_inline void preempt_count_set(int pc) | ||||||
| } while (0) | } while (0) | ||||||
| 
 | 
 | ||||||
| #define init_idle_preempt_count(p, cpu) do { \ | #define init_idle_preempt_count(p, cpu) do { \ | ||||||
| 	task_thread_info(p)->preempt_count = PREEMPT_ENABLED; \ | 	task_thread_info(p)->preempt_count = PREEMPT_DISABLED; \ | ||||||
| } while (0) | } while (0) | ||||||
| 
 | 
 | ||||||
| static __always_inline void set_preempt_need_resched(void) | static __always_inline void set_preempt_need_resched(void) | ||||||
|  |  | ||||||
|  | @ -941,11 +941,7 @@ asmlinkage __visible void __init __no_sanitize_address start_kernel(void) | ||||||
| 	 * time - but meanwhile we still have a functioning scheduler. | 	 * time - but meanwhile we still have a functioning scheduler. | ||||||
| 	 */ | 	 */ | ||||||
| 	sched_init(); | 	sched_init(); | ||||||
| 	/*
 | 
 | ||||||
| 	 * Disable preemption - early bootup scheduling is extremely |  | ||||||
| 	 * fragile until we cpu_idle() for the first time. |  | ||||||
| 	 */ |  | ||||||
| 	preempt_disable(); |  | ||||||
| 	if (WARN(!irqs_disabled(), | 	if (WARN(!irqs_disabled(), | ||||||
| 		 "Interrupts were enabled *very* early, fixing it\n")) | 		 "Interrupts were enabled *very* early, fixing it\n")) | ||||||
| 		local_irq_disable(); | 		local_irq_disable(); | ||||||
|  |  | ||||||
|  | @ -2412,7 +2412,7 @@ static inline void init_idle_pids(struct task_struct *idle) | ||||||
| 	} | 	} | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| struct task_struct *fork_idle(int cpu) | struct task_struct * __init fork_idle(int cpu) | ||||||
| { | { | ||||||
| 	struct task_struct *task; | 	struct task_struct *task; | ||||||
| 	struct kernel_clone_args args = { | 	struct kernel_clone_args args = { | ||||||
|  |  | ||||||
|  | @ -8227,7 +8227,7 @@ void show_state_filter(unsigned long state_filter) | ||||||
|  * NOTE: this function does not set the idle thread's NEED_RESCHED |  * NOTE: this function does not set the idle thread's NEED_RESCHED | ||||||
|  * flag, to make booting more robust. |  * flag, to make booting more robust. | ||||||
|  */ |  */ | ||||||
| void init_idle(struct task_struct *idle, int cpu) | void __init init_idle(struct task_struct *idle, int cpu) | ||||||
| { | { | ||||||
| 	struct rq *rq = cpu_rq(cpu); | 	struct rq *rq = cpu_rq(cpu); | ||||||
| 	unsigned long flags; | 	unsigned long flags; | ||||||
|  |  | ||||||
|  | @ -33,7 +33,6 @@ struct task_struct *idle_thread_get(unsigned int cpu) | ||||||
| 
 | 
 | ||||||
| 	if (!tsk) | 	if (!tsk) | ||||||
| 		return ERR_PTR(-ENOMEM); | 		return ERR_PTR(-ENOMEM); | ||||||
| 	init_idle(tsk, cpu); |  | ||||||
| 	return tsk; | 	return tsk; | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
		Loading…
	
		Reference in a new issue
	
	 Valentin Schneider
						Valentin Schneider