History

sched, idle: Fix the idle polling state logic

Mike reported that commit 7d1a9417 ("x86: Use generic idle loop") regressed several workloads and caused excessive reschedule interrupts. The patch in question failed to notice that the x86 code had an inverted sense of the polling state versus the new generic code (x86: default polling, generic: default !polling). Fix the two prominent x86 mwait based idle drivers and introduce a few new generic polling helpers (fixing the wrong smp_mb__after_clear_bit usage). Also switch the idle routines to using tif_need_resched() which is an immediate TIF_NEED_RESCHED test as opposed to need_resched which will end up being slightly different.

Peter Zijlstra committed 13 years ago

Commit ea811741 parent 31503986 files · +91 −52Browse files

Unified · Split

6 files
  1. arch/x86/kernel/process.c +3 −3
  2. drivers/acpi/processor_idle.c +10 −36
  3. drivers/idle/intel_idle.c +1 −1
  4. include/linux/sched.h +71 −7
  5. include/linux/thread_info.h +2 −0
  6. kernel/cpu/idle.c +4 −5
arch/x86/kernel/process.c+3−3
@@ -391,9 +391,9 @@ static void amd_e400_idle(void) · 390 unmodified lines
391391 * The switch back from broadcast mode needs to be
392392 * called with interrupts disabled.
393393 */
394− local_irq_disable();
395− clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &cpu);
396− local_irq_enable();
394+ local_irq_disable();
395+ clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &cpu);
396+ local_irq_enable();
397397 } else
398398 default_idle();
399399 }
drivers/acpi/processor_idle.c+10−36
@@ -119,17 +119,10 @@ static struct dmi_system_id processor_power_dmi_table[] = { · 118 unmodified lines
119119 */
120120 static void acpi_safe_halt(void)
121121 {
122− current_thread_info()->status &= ~TS_POLLING;
123− /*
124− * TS_POLLING-cleared state must be visible before we
125− * test NEED_RESCHED:
126− */
127− smp_mb();
128− if (!need_resched()) {
122+ if (!tif_need_resched()) {
129123 safe_halt();
130124 local_irq_disable();
131125 }
132− current_thread_info()->status |= TS_POLLING;
133126 }
134127
135128 #ifdef ARCH_APICTIMER_STOPS_ON_C3
@@ -737,6 +730,11 @@ static int acpi_idle_enter_c1(struct cpuidle_device *dev, · 601 unmodified lines
737730 if (unlikely(!pr))
738731 return -EINVAL;
739732
733+ if (cx->entry_method == ACPI_CSTATE_FFH) {
734+ if (current_set_polling_and_test())
735+ return -EINVAL;
736+ }
737+
740738 lapic_timer_state_broadcast(pr, cx, 1);
741739 acpi_idle_do_entry(cx);
742740
@@ -790,18 +788,9 @@ static int acpi_idle_enter_simple(struct cpuidle_device *dev, · 47 unmodified lines
790788 if (unlikely(!pr))
791789 return -EINVAL;
792790
793− if (cx->entry_method != ACPI_CSTATE_FFH) {
794− current_thread_info()->status &= ~TS_POLLING;
795− /*
796− * TS_POLLING-cleared state must be visible before we test
797− * NEED_RESCHED:
798− */
799− smp_mb();
800−
801− if (unlikely(need_resched())) {
802− current_thread_info()->status |= TS_POLLING;
791+ if (cx->entry_method == ACPI_CSTATE_FFH) {
792+ if (current_set_polling_and_test())
803793 return -EINVAL;
804− }
805794 }
806795
807796 /*
@@ -819,9 +808,6 @@ static int acpi_idle_enter_simple(struct cpuidle_device *dev, · 11 unmodified lines
819808
820809 sched_clock_idle_wakeup_event(0);
821810
822− if (cx->entry_method != ACPI_CSTATE_FFH)
823− current_thread_info()->status |= TS_POLLING;
824−
825811 lapic_timer_state_broadcast(pr, cx, 0);
826812 return index;
827813 }
@@ -858,18 +844,9 @@ static int acpi_idle_enter_bm(struct cpuidle_device *dev, · 30 unmodified lines
858844 }
859845 }
860846
861− if (cx->entry_method != ACPI_CSTATE_FFH) {
862− current_thread_info()->status &= ~TS_POLLING;
863− /*
864− * TS_POLLING-cleared state must be visible before we test
865− * NEED_RESCHED:
866− */
867− smp_mb();
868−
869− if (unlikely(need_resched())) {
870− current_thread_info()->status |= TS_POLLING;
847+ if (cx->entry_method == ACPI_CSTATE_FFH) {
848+ if (current_set_polling_and_test())
871849 return -EINVAL;
872− }
873850 }
874851
875852 acpi_unlazy_tlb(smp_processor_id());
@@ -915,9 +892,6 @@ static int acpi_idle_enter_bm(struct cpuidle_device *dev, · 39 unmodified lines
915892
916893 sched_clock_idle_wakeup_event(0);
917894
918− if (cx->entry_method != ACPI_CSTATE_FFH)
919− current_thread_info()->status |= TS_POLLING;
920−
921895 lapic_timer_state_broadcast(pr, cx, 0);
922896 return index;
923897 }
drivers/idle/intel_idle.c+1−1
@@ -359,7 +359,7 @@ static int intel_idle(struct cpuidle_device *dev, · 358 unmodified lines
359359 if (!(lapic_timer_reliable_states & (1 << (cstate))))
360360 clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &cpu);
361361
362− if (!need_resched()) {
362+ if (!current_set_polling_and_test()) {
363363
364364 __monitor((void *)&current_thread_info()->flags, 0, 0);
365365 smp_mb();
include/linux/sched.h+71−7
@@ -2479,34 +2479,98 @@ static inline int tsk_is_polling(struct task_struct *p) · 2478 unmodified lines
24792479 {
24802480 return task_thread_info(p)->status & TS_POLLING;
24812481 }
2482−static inline void current_set_polling(void)
2482+static inline void __current_set_polling(void)
24832483 {
24842484 current_thread_info()->status |= TS_POLLING;
24852485 }
24862486
2487−static inline void current_clr_polling(void)
2487+static inline bool __must_check current_set_polling_and_test(void)
2488+{
2489+ __current_set_polling();
2490+
2491+ /*
2492+ * Polling state must be visible before we test NEED_RESCHED,
2493+ * paired by resched_task()
2494+ */
2495+ smp_mb();
2496+
2497+ return unlikely(tif_need_resched());
2498+}
2499+
2500+static inline void __current_clr_polling(void)
24882501 {
24892502 current_thread_info()->status &= ~TS_POLLING;
2490− smp_mb__after_clear_bit();
2503+}
2504+
2505+static inline bool __must_check current_clr_polling_and_test(void)
2506+{
2507+ __current_clr_polling();
2508+
2509+ /*
2510+ * Polling state must be visible before we test NEED_RESCHED,
2511+ * paired by resched_task()
2512+ */
2513+ smp_mb();
2514+
2515+ return unlikely(tif_need_resched());
24912516 }
24922517 #elif defined(TIF_POLLING_NRFLAG)
24932518 static inline int tsk_is_polling(struct task_struct *p)
24942519 {
24952520 return test_tsk_thread_flag(p, TIF_POLLING_NRFLAG);
24962521 }
2497−static inline void current_set_polling(void)
2522+
2523+static inline void __current_set_polling(void)
24982524 {
24992525 set_thread_flag(TIF_POLLING_NRFLAG);
25002526 }
25012527
2502−static inline void current_clr_polling(void)
2528+static inline bool __must_check current_set_polling_and_test(void)
2529+{
2530+ __current_set_polling();
2531+
2532+ /*
2533+ * Polling state must be visible before we test NEED_RESCHED,
2534+ * paired by resched_task()
2535+ *
2536+ * XXX: assumes set/clear bit are identical barrier wise.
2537+ */
2538+ smp_mb__after_clear_bit();
2539+
2540+ return unlikely(tif_need_resched());
2541+}
2542+
2543+static inline void __current_clr_polling(void)
25032544 {
25042545 clear_thread_flag(TIF_POLLING_NRFLAG);
25052546 }
2547+
2548+static inline bool __must_check current_clr_polling_and_test(void)
2549+{
2550+ __current_clr_polling();
2551+
2552+ /*
2553+ * Polling state must be visible before we test NEED_RESCHED,
2554+ * paired by resched_task()
2555+ */
2556+ smp_mb__after_clear_bit();
2557+
2558+ return unlikely(tif_need_resched());
2559+}
2560+
25062561 #else
25072562 static inline int tsk_is_polling(struct task_struct *p) { return 0; }
2508−static inline void current_set_polling(void) { }
2509−static inline void current_clr_polling(void) { }
2563+static inline void __current_set_polling(void) { }
2564+static inline void __current_clr_polling(void) { }
2565+
2566+static inline bool __must_check current_set_polling_and_test(void)
2567+{
2568+ return unlikely(tif_need_resched());
2569+}
2570+static inline bool __must_check current_clr_polling_and_test(void)
2571+{
2572+ return unlikely(tif_need_resched());
2573+}
25102574 #endif
25112575
25122576 /*
include/linux/thread_info.h+2−0
@@ -118,6 +118,8 @@ static inline __deprecated void set_need_resched(void) · 117 unmodified lines
118118 */
119119 }
120120
121+#define tif_need_resched() test_thread_flag(TIF_NEED_RESCHED)
122+
121123 #if defined TIF_RESTORE_SIGMASK && !defined HAVE_SET_RESTORE_SIGMASK
122124 /*
123125 * An arch can define its own version of set_restore_sigmask() to get the
kernel/cpu/idle.c+4−5
@@ -44,7 +44,7 @@ static inline int cpu_idle_poll(void) · 43 unmodified lines
4444 rcu_idle_enter();
4545 trace_cpu_idle_rcuidle(0, smp_processor_id());
4646 local_irq_enable();
47− while (!need_resched())
47+ while (!tif_need_resched())
4848 cpu_relax();
4949 trace_cpu_idle_rcuidle(PWR_EVENT_EXIT, smp_processor_id());
5050 rcu_idle_exit();
@@ -92,8 +92,7 @@ static void cpu_idle_loop(void) · 41 unmodified lines
9292 if (cpu_idle_force_poll || tick_check_broadcast_expired()) {
9393 cpu_idle_poll();
9494 } else {
95− current_clr_polling();
96− if (!need_resched()) {
95+ if (!current_clr_polling_and_test()) {
9796 stop_critical_timings();
9897 rcu_idle_enter();
9998 arch_cpu_idle();
@@ -103,7 +102,7 @@ static void cpu_idle_loop(void) · 3 unmodified lines
103102 } else {
104103 local_irq_enable();
105104 }
106− current_set_polling();
105+ __current_set_polling();
107106 }
108107 arch_cpu_idle_exit();
109108 }
@@ -129,7 +128,7 @@ void cpu_startup_entry(enum cpuhp_state state) · 19 unmodified lines
129128 */
130129 boot_init_stack_canary();
131130 #endif
132− current_set_polling();
131+ __current_set_polling();
133132 arch_cpu_idle_prepare();
134133 cpu_idle_loop();
135134 }

Keyboard shortcuts

General

Show keyboard shortcuts
?
Focus the current search
/

Issues and pull requests

Open filters
f

Issues, pull requests and Inbox

Select next item
j↓
Select previous item
k↑
Open selected item
Enter
Dismiss filters or clear selection
Esc

Repository code

Go to file
t