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
|
@ -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 New Issue