]> git.dujemihanovic.xyz Git - linux.git/commitdiff
sched/core: Initialize the idle task with preemption disabled
authorValentin Schneider <valentin.schneider@arm.com>
Wed, 12 May 2021 09:46:36 +0000 (10:46 +0100)
committerIngo Molnar <mingo@kernel.org>
Wed, 12 May 2021 11:01:45 +0000 (13:01 +0200)
As pointed out by commit

  de9b8f5dcbd9 ("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
25 files changed:
arch/alpha/kernel/smp.c
arch/arc/kernel/smp.c
arch/arm/kernel/smp.c
arch/arm64/include/asm/preempt.h
arch/arm64/kernel/smp.c
arch/csky/kernel/smp.c
arch/ia64/kernel/smpboot.c
arch/mips/kernel/smp.c
arch/openrisc/kernel/smp.c
arch/parisc/kernel/smp.c
arch/powerpc/kernel/smp.c
arch/riscv/kernel/smpboot.c
arch/s390/include/asm/preempt.h
arch/s390/kernel/smp.c
arch/sh/kernel/smp.c
arch/sparc/kernel/smp_32.c
arch/sparc/kernel/smp_64.c
arch/x86/include/asm/preempt.h
arch/x86/kernel/smpboot.c
arch/xtensa/kernel/smp.c
include/asm-generic/preempt.h
init/main.c
kernel/fork.c
kernel/sched/core.c
kernel/smpboot.c

index f4dd9f3f30010ba5f62d0bfa10ada7876837d3fa..4b2575f936d46e77c82e9c003bd53b98e1f9291b 100644 (file)
@@ -166,7 +166,6 @@ smp_callin(void)
        DBGS(("smp_callin: commencing CPU %d current %p active_mm %p\n",
              cpuid, current, current->active_mm));
 
-       preempt_disable();
        cpu_startup_entry(CPUHP_AP_ONLINE_IDLE);
 }
 
index 52906d3145371b4a5a0bbaba5d1d9239480465da..db0e104d68355b6dd0a35524960983291ccda9c4 100644 (file)
@@ -189,7 +189,6 @@ void start_kernel_secondary(void)
        pr_info("## CPU%u LIVE ##: Executing Code...\n", cpu);
 
        local_irq_enable();
-       preempt_disable();
        cpu_startup_entry(CPUHP_AP_ONLINE_IDLE);
 }
 
index 74679240a9d8e78e3f738d3eb6d79a45b2f491f9..c7bb168b0d97ce26cc62fe562d8a89e8a0443850 100644 (file)
@@ -432,7 +432,6 @@ asmlinkage void secondary_start_kernel(void)
 #endif
        pr_debug("CPU%u: Booted secondary processor\n", cpu);
 
-       preempt_disable();
        trace_hardirqs_off();
 
        /*
index 80e946b2abee276479fde238d3f01e5d7eecacb3..e83f0982b99c1a6a773401e276b90a1f32fefb0a 100644 (file)
@@ -23,7 +23,7 @@ static inline void preempt_count_set(u64 pc)
 } while (0)
 
 #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)
 
 static inline void set_preempt_need_resched(void)
index dcd7041b2b077f0ece865d5edfb4b1b67e04199c..6671000a8b7d7bc9c1275b8824fd77b5e8e5006b 100644 (file)
@@ -224,7 +224,6 @@ asmlinkage notrace void secondary_start_kernel(void)
                init_gic_priority_masking();
 
        rcu_cpu_starting(cpu);
-       preempt_disable();
        trace_hardirqs_off();
 
        /*
index 0f9f5eef9338695b0b43dab09d198c70b2962395..e2993539af8efea577a76ee325c932e6eb060a0b 100644 (file)
@@ -281,7 +281,6 @@ void csky_start_secondary(void)
        pr_info("CPU%u Online: %s...\n", cpu, __func__);
 
        local_irq_enable();
-       preempt_disable();
        cpu_startup_entry(CPUHP_AP_ONLINE_IDLE);
 }
 
index 49b4885809399822ea79c34a51714514ea432fdb..d10f780c13b9eca3d91b55e2249bc5b50da3302e 100644 (file)
@@ -441,7 +441,6 @@ start_secondary (void *unused)
 #endif
        efi_map_pal_code();
        cpu_init();
-       preempt_disable();
        smp_callin();
 
        cpu_startup_entry(CPUHP_AP_ONLINE_IDLE);
index ef86fbad8546074da9edaaec2352e7c5c9589255..d542fb7af3ba2e2221c9e05468928c1775450f41 100644 (file)
@@ -348,7 +348,6 @@ asmlinkage void start_secondary(void)
         */
 
        calibrate_delay();
-       preempt_disable();
        cpu = smp_processor_id();
        cpu_data[cpu].udelay_val = loops_per_jiffy;
 
index 48e1092a64de35c7502a5b3ce91108358d297c05..415e209732a3d085f645fa2dbcdc01f7daec25df 100644 (file)
@@ -145,8 +145,6 @@ asmlinkage __init void secondary_start_kernel(void)
        set_cpu_online(cpu, true);
 
        local_irq_enable();
-
-       preempt_disable();
        /*
         * OK, it's off to the idle thread for us
         */
index 10227f667c8a6ed09f68dc2e9e3e75f6d7a4a171..1405b603b91b6be6909530825ad455ee150ebbf3 100644 (file)
@@ -302,7 +302,6 @@ void __init smp_callin(unsigned long pdce_proc)
 #endif
 
        smp_cpu_init(slave_id);
-       preempt_disable();
 
        flush_cache_all_local(); /* start with known state */
        flush_tlb_all_local(NULL);
index 2e05c783440a33c3ed82bc19486736fb1be615db..6c6e4d934d867c0c6e3257cd8380b9ccc8270f76 100644 (file)
@@ -1547,7 +1547,6 @@ void start_secondary(void *unused)
        smp_store_cpu_info(cpu);
        set_dec(tb_ticks_per_jiffy);
        rcu_cpu_starting(cpu);
-       preempt_disable();
        cpu_callin_map[cpu] = 1;
 
        if (smp_ops->setup_cpu)
index 9a408e2942acfc0ecf11c03ec7a7bc1a6e133226..bd82375db51a62cf1b84414a2a80d9738ca1d36c 100644 (file)
@@ -180,7 +180,6 @@ asmlinkage __visible void smp_callin(void)
         * Disable preemption before enabling interrupts, so we don't try to
         * schedule a CPU that hasn't actually started yet.
         */
-       preempt_disable();
        local_irq_enable();
        cpu_startup_entry(CPUHP_AP_ONLINE_IDLE);
 }
index b49e0492842cccddde08d6497d673ed3022b52e3..23ff51be7e29c19291050bfeefc6468ecb0232b3 100644 (file)
@@ -32,7 +32,7 @@ static inline void preempt_count_set(int pc)
 #define init_task_preempt_count(p)     do { } while (0)
 
 #define init_idle_preempt_count(p, cpu)        do { \
-       S390_lowcore.preempt_count = PREEMPT_ENABLED; \
+       S390_lowcore.preempt_count = PREEMPT_DISABLED; \
 } while (0)
 
 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_idle_preempt_count(p, cpu)        do { \
-       S390_lowcore.preempt_count = PREEMPT_ENABLED; \
+       S390_lowcore.preempt_count = PREEMPT_DISABLED; \
 } while (0)
 
 static inline void set_preempt_need_resched(void)
index 2fec2b80d35d2ee913f02a84e90642f0a2bd5e87..111909aeb8d215401d8ec6490da67c51e472b5d6 100644 (file)
@@ -878,7 +878,6 @@ static void smp_init_secondary(void)
        restore_access_regs(S390_lowcore.access_regs_save_area);
        cpu_init();
        rcu_cpu_starting(cpu);
-       preempt_disable();
        init_cpu_timer();
        vtime_init();
        vdso_getcpu_init();
index 372acdc9033eb35da2b5a610cc6221f53208e573..65924d9ec24598f81e995bee85bf4cb135b1d84b 100644 (file)
@@ -186,8 +186,6 @@ asmlinkage void start_secondary(void)
 
        per_cpu_trap_init();
 
-       preempt_disable();
-
        notify_cpu_starting(cpu);
 
        local_irq_enable();
index 50c127ab46d5bc810f6e874a106188f8029e6af4..22b148e5a5f88c76c46b17b87c53f7df510940c8 100644 (file)
@@ -348,7 +348,6 @@ static void sparc_start_secondary(void *arg)
         */
        arch_cpu_pre_starting(arg);
 
-       preempt_disable();
        cpu = smp_processor_id();
 
        notify_cpu_starting(cpu);
index e38d8bf454e866636e7091d131277e0596baebb3..ae5faa1d989d2ec7e8ef3e0f05f7d91148b25bc3 100644 (file)
@@ -138,9 +138,6 @@ void smp_callin(void)
 
        set_cpu_online(cpuid, true);
 
-       /* idle thread is expected to have preempt disabled */
-       preempt_disable();
-
        local_irq_enable();
 
        cpu_startup_entry(CPUHP_AP_ONLINE_IDLE);
index f8cb8af4de5ce9f0126cac28983cafad0a1cf1e1..fe5efbcba8240c8619ee0ed953cf3a4b01bab9b6 100644 (file)
@@ -44,7 +44,7 @@ static __always_inline void preempt_count_set(int pc)
 #define init_task_preempt_count(p) do { } while (0)
 
 #define init_idle_preempt_count(p, cpu) do { \
-       per_cpu(__preempt_count, (cpu)) = PREEMPT_ENABLED; \
+       per_cpu(__preempt_count, (cpu)) = PREEMPT_DISABLED; \
 } while (0)
 
 /*
index 0ad5214f598a9cc8ee03136472cddbc9520a5959..0936f5ba32229dbdc32a6a694f5396d107a2471c 100644 (file)
@@ -236,7 +236,6 @@ static void notrace start_secondary(void *unused)
        cpu_init();
        rcu_cpu_starting(raw_smp_processor_id());
        x86_cpuinit.early_percpu_clock_init();
-       preempt_disable();
        smp_callin();
 
        enable_start_cpu0 = 0;
index cd85a7a2722ba4f4f101957c8cbc7665deda12ac..1254da07ead1f42712918b90c54b45ebf83cb5bb 100644 (file)
@@ -145,7 +145,6 @@ void secondary_start_kernel(void)
        cpumask_set_cpu(cpu, mm_cpumask(mm));
        enter_lazy_tlb(mm, current);
 
-       preempt_disable();
        trace_hardirqs_off();
 
        calibrate_delay();
index d683f5e6d7913be7746e375908265b827d1b74c2..b4d43a4af5f794e9f7a0094cd002610fa1beb6d0 100644 (file)
@@ -29,7 +29,7 @@ static __always_inline void preempt_count_set(int pc)
 } while (0)
 
 #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)
 
 static __always_inline void set_preempt_need_resched(void)
index eb01e121d2f15ca12f691277d5040eccf5c24476..7b027d9c5c89bc7d9455f0f14538e577f01121aa 100644 (file)
@@ -941,11 +941,7 @@ asmlinkage __visible void __init __no_sanitize_address start_kernel(void)
         * time - but meanwhile we still have a functioning scheduler.
         */
        sched_init();
-       /*
-        * Disable preemption - early bootup scheduling is extremely
-        * fragile until we cpu_idle() for the first time.
-        */
-       preempt_disable();
+
        if (WARN(!irqs_disabled(),
                 "Interrupts were enabled *very* early, fixing it\n"))
                local_irq_disable();
index e7fd928fcafe97bbc714f376f1636cdba43d0aee..ace4631b5b547d17a4a88d1996b3167eae855d8b 100644 (file)
@@ -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 kernel_clone_args args = {
index 55b2d9399e12a03dc1d72cd335487197c9caeac1..9d00f4958bde755cce7e3daec3a8b514683b9586 100644 (file)
@@ -8227,7 +8227,7 @@ void show_state_filter(unsigned long state_filter)
  * NOTE: this function does not set the idle thread's NEED_RESCHED
  * 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);
        unsigned long flags;
index f25208e8df8365e090cedf887638b3e0b00e92bc..e4163042c4d6658d4fcf192bfcac04e8c2ddb5cf 100644 (file)
@@ -33,7 +33,6 @@ struct task_struct *idle_thread_get(unsigned int cpu)
 
        if (!tsk)
                return ERR_PTR(-ENOMEM);
-       init_idle(tsk, cpu);
        return tsk;
 }