public inbox for stable@vger.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Mike Galbraith <bitbucket@online.de>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@kernel.org>,
	lenb@kernel.org, tglx@linutronix.de
Subject: [PATCH 3.11 18/36] sched, idle: Fix the idle polling state logic
Date: Tue, 26 Nov 2013 10:12:28 -0800	[thread overview]
Message-ID: <20131126181028.831748517@linuxfoundation.org> (raw)
In-Reply-To: <20131126181025.029404973@linuxfoundation.org>

3.11-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Peter Zijlstra <peterz@infradead.org>

commit ea8117478918a4734586d35ff530721b682425be upstream.

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.

Reported-by: Mike Galbraith <bitbucket@online.de>
Signed-off-by: Peter Zijlstra <peterz@infradead.org>
Cc: lenb@kernel.org
Cc: tglx@linutronix.de
Link: http://lkml.kernel.org/n/tip-nc03imb0etuefmzybzj7sprf@git.kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 arch/x86/kernel/process.c     |    6 +--
 drivers/acpi/processor_idle.c |   46 +++++-------------------
 drivers/idle/intel_idle.c     |    2 -
 include/linux/sched.h         |   78 ++++++++++++++++++++++++++++++++++++++----
 include/linux/thread_info.h   |    2 +
 kernel/cpu/idle.c             |    9 ++--
 6 files changed, 91 insertions(+), 52 deletions(-)

--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -391,9 +391,9 @@ static void amd_e400_idle(void)
 		 * The switch back from broadcast mode needs to be
 		 * called with interrupts disabled.
 		 */
-		 local_irq_disable();
-		 clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &cpu);
-		 local_irq_enable();
+		local_irq_disable();
+		clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &cpu);
+		local_irq_enable();
 	} else
 		default_idle();
 }
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -119,17 +119,10 @@ static struct dmi_system_id processor_po
  */
 static void acpi_safe_halt(void)
 {
-	current_thread_info()->status &= ~TS_POLLING;
-	/*
-	 * TS_POLLING-cleared state must be visible before we
-	 * test NEED_RESCHED:
-	 */
-	smp_mb();
-	if (!need_resched()) {
+	if (!tif_need_resched()) {
 		safe_halt();
 		local_irq_disable();
 	}
-	current_thread_info()->status |= TS_POLLING;
 }
 
 #ifdef ARCH_APICTIMER_STOPS_ON_C3
@@ -737,6 +730,11 @@ static int acpi_idle_enter_c1(struct cpu
 	if (unlikely(!pr))
 		return -EINVAL;
 
+	if (cx->entry_method == ACPI_CSTATE_FFH) {
+		if (current_set_polling_and_test())
+			return -EINVAL;
+	}
+
 	lapic_timer_state_broadcast(pr, cx, 1);
 	acpi_idle_do_entry(cx);
 
@@ -790,18 +788,9 @@ static int acpi_idle_enter_simple(struct
 	if (unlikely(!pr))
 		return -EINVAL;
 
-	if (cx->entry_method != ACPI_CSTATE_FFH) {
-		current_thread_info()->status &= ~TS_POLLING;
-		/*
-		 * TS_POLLING-cleared state must be visible before we test
-		 * NEED_RESCHED:
-		 */
-		smp_mb();
-
-		if (unlikely(need_resched())) {
-			current_thread_info()->status |= TS_POLLING;
+	if (cx->entry_method == ACPI_CSTATE_FFH) {
+		if (current_set_polling_and_test())
 			return -EINVAL;
-		}
 	}
 
 	/*
@@ -819,9 +808,6 @@ static int acpi_idle_enter_simple(struct
 
 	sched_clock_idle_wakeup_event(0);
 
-	if (cx->entry_method != ACPI_CSTATE_FFH)
-		current_thread_info()->status |= TS_POLLING;
-
 	lapic_timer_state_broadcast(pr, cx, 0);
 	return index;
 }
@@ -858,18 +844,9 @@ static int acpi_idle_enter_bm(struct cpu
 		}
 	}
 
-	if (cx->entry_method != ACPI_CSTATE_FFH) {
-		current_thread_info()->status &= ~TS_POLLING;
-		/*
-		 * TS_POLLING-cleared state must be visible before we test
-		 * NEED_RESCHED:
-		 */
-		smp_mb();
-
-		if (unlikely(need_resched())) {
-			current_thread_info()->status |= TS_POLLING;
+	if (cx->entry_method == ACPI_CSTATE_FFH) {
+		if (current_set_polling_and_test())
 			return -EINVAL;
-		}
 	}
 
 	acpi_unlazy_tlb(smp_processor_id());
@@ -915,9 +892,6 @@ static int acpi_idle_enter_bm(struct cpu
 
 	sched_clock_idle_wakeup_event(0);
 
-	if (cx->entry_method != ACPI_CSTATE_FFH)
-		current_thread_info()->status |= TS_POLLING;
-
 	lapic_timer_state_broadcast(pr, cx, 0);
 	return index;
 }
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -359,7 +359,7 @@ static int intel_idle(struct cpuidle_dev
 	if (!(lapic_timer_reliable_states & (1 << (cstate))))
 		clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &cpu);
 
-	if (!need_resched()) {
+	if (!current_set_polling_and_test()) {
 
 		__monitor((void *)&current_thread_info()->flags, 0, 0);
 		smp_mb();
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -2473,34 +2473,98 @@ static inline int tsk_is_polling(struct
 {
 	return task_thread_info(p)->status & TS_POLLING;
 }
-static inline void current_set_polling(void)
+static inline void __current_set_polling(void)
 {
 	current_thread_info()->status |= TS_POLLING;
 }
 
-static inline void current_clr_polling(void)
+static inline bool __must_check current_set_polling_and_test(void)
+{
+	__current_set_polling();
+
+	/*
+	 * Polling state must be visible before we test NEED_RESCHED,
+	 * paired by resched_task()
+	 */
+	smp_mb();
+
+	return unlikely(tif_need_resched());
+}
+
+static inline void __current_clr_polling(void)
 {
 	current_thread_info()->status &= ~TS_POLLING;
-	smp_mb__after_clear_bit();
+}
+
+static inline bool __must_check current_clr_polling_and_test(void)
+{
+	__current_clr_polling();
+
+	/*
+	 * Polling state must be visible before we test NEED_RESCHED,
+	 * paired by resched_task()
+	 */
+	smp_mb();
+
+	return unlikely(tif_need_resched());
 }
 #elif defined(TIF_POLLING_NRFLAG)
 static inline int tsk_is_polling(struct task_struct *p)
 {
 	return test_tsk_thread_flag(p, TIF_POLLING_NRFLAG);
 }
-static inline void current_set_polling(void)
+
+static inline void __current_set_polling(void)
 {
 	set_thread_flag(TIF_POLLING_NRFLAG);
 }
 
-static inline void current_clr_polling(void)
+static inline bool __must_check current_set_polling_and_test(void)
+{
+	__current_set_polling();
+
+	/*
+	 * Polling state must be visible before we test NEED_RESCHED,
+	 * paired by resched_task()
+	 *
+	 * XXX: assumes set/clear bit are identical barrier wise.
+	 */
+	smp_mb__after_clear_bit();
+
+	return unlikely(tif_need_resched());
+}
+
+static inline void __current_clr_polling(void)
 {
 	clear_thread_flag(TIF_POLLING_NRFLAG);
 }
+
+static inline bool __must_check current_clr_polling_and_test(void)
+{
+	__current_clr_polling();
+
+	/*
+	 * Polling state must be visible before we test NEED_RESCHED,
+	 * paired by resched_task()
+	 */
+	smp_mb__after_clear_bit();
+
+	return unlikely(tif_need_resched());
+}
+
 #else
 static inline int tsk_is_polling(struct task_struct *p) { return 0; }
-static inline void current_set_polling(void) { }
-static inline void current_clr_polling(void) { }
+static inline void __current_set_polling(void) { }
+static inline void __current_clr_polling(void) { }
+
+static inline bool __must_check current_set_polling_and_test(void)
+{
+	return unlikely(tif_need_resched());
+}
+static inline bool __must_check current_clr_polling_and_test(void)
+{
+	return unlikely(tif_need_resched());
+}
 #endif
 
 /*
--- a/include/linux/thread_info.h
+++ b/include/linux/thread_info.h
@@ -107,6 +107,8 @@ static inline int test_ti_thread_flag(st
 #define set_need_resched()	set_thread_flag(TIF_NEED_RESCHED)
 #define clear_need_resched()	clear_thread_flag(TIF_NEED_RESCHED)
 
+#define tif_need_resched() test_thread_flag(TIF_NEED_RESCHED)
+
 #if defined TIF_RESTORE_SIGMASK && !defined HAVE_SET_RESTORE_SIGMASK
 /*
  * An arch can define its own version of set_restore_sigmask() to get the
--- a/kernel/cpu/idle.c
+++ b/kernel/cpu/idle.c
@@ -44,7 +44,7 @@ static inline int cpu_idle_poll(void)
 	rcu_idle_enter();
 	trace_cpu_idle_rcuidle(0, smp_processor_id());
 	local_irq_enable();
-	while (!need_resched())
+	while (!tif_need_resched())
 		cpu_relax();
 	trace_cpu_idle_rcuidle(PWR_EVENT_EXIT, smp_processor_id());
 	rcu_idle_exit();
@@ -92,8 +92,7 @@ static void cpu_idle_loop(void)
 			if (cpu_idle_force_poll || tick_check_broadcast_expired()) {
 				cpu_idle_poll();
 			} else {
-				current_clr_polling();
-				if (!need_resched()) {
+				if (!current_clr_polling_and_test()) {
 					stop_critical_timings();
 					rcu_idle_enter();
 					arch_cpu_idle();
@@ -103,7 +102,7 @@ static void cpu_idle_loop(void)
 				} else {
 					local_irq_enable();
 				}
-				current_set_polling();
+				__current_set_polling();
 			}
 			arch_cpu_idle_exit();
 		}
@@ -129,7 +128,7 @@ void cpu_startup_entry(enum cpuhp_state
 	 */
 	boot_init_stack_canary();
 #endif
-	current_set_polling();
+	__current_set_polling();
 	arch_cpu_idle_prepare();
 	cpu_idle_loop();
 }



  parent reply	other threads:[~2013-11-26 18:12 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-26 18:12 [PATCH 3.11 00/36] 3.11.10-stable review Greg Kroah-Hartman
2013-11-26 18:12 ` [PATCH 3.11 01/36] ACPICA: DeRefOf operator: Update to fully resolve FieldUnit and BufferField refs Greg Kroah-Hartman
2013-11-26 18:12 ` [PATCH 3.11 02/36] libertas: potential oops in debugfs Greg Kroah-Hartman
2013-11-26 18:12 ` [PATCH 3.11 03/36] aacraid: prevent invalid pointer dereference Greg Kroah-Hartman
2013-11-26 18:12 ` [PATCH 3.11 04/36] ACPICA: Return error if DerefOf resolves to a null package element Greg Kroah-Hartman
2013-11-26 18:12 ` [PATCH 3.11 05/36] ACPICA: Fix for a Store->ArgX when ArgX contains a reference to a field Greg Kroah-Hartman
2013-11-26 18:12 ` [PATCH 3.11 06/36] USB: mos7840: fix tiocmget error handling Greg Kroah-Hartman
2013-11-26 18:12 ` [PATCH 3.11 07/36] can: kvaser_usb: fix usb endpoints detection Greg Kroah-Hartman
2013-11-26 18:12 ` [PATCH 3.11 08/36] Btrfs: relocate csums properly with prealloc extents Greg Kroah-Hartman
2013-11-26 18:12 ` [PATCH 3.11 09/36] crypto: ansi_cprng - Fix off by one error in non-block size request Greg Kroah-Hartman
2013-11-26 18:12 ` [PATCH 3.11 10/36] crypto: s390 - Fix aes-cbc IV corruption Greg Kroah-Hartman
2013-11-26 18:12 ` [PATCH 3.11 11/36] can: c_can: Fix RX message handling, handle lost message before EOB Greg Kroah-Hartman
2013-11-26 18:12 ` [PATCH 3.11 12/36] alx: Reset phy speed after resume Greg Kroah-Hartman
2013-11-26 18:12 ` [PATCH 3.11 13/36] ipc,shm: correct error return value in shmctl (SHM_UNLOCK) Greg Kroah-Hartman
2013-11-26 18:12 ` [PATCH 3.11 14/36] ipc,shm: fix shm_file deletion races Greg Kroah-Hartman
2013-11-26 18:12 ` [PATCH 3.11 15/36] drm/nvc0-/gr: fix a number of missing explicit array terminators Greg Kroah-Hartman
2013-11-26 18:12 ` [PATCH 3.11 16/36] thinkpad_acpi: Fix build error when CONFIG_SND_MAX_CARDS > 32 Greg Kroah-Hartman
2013-11-26 18:12 ` [PATCH 3.11 17/36] SUNRPC: dont map EKEYEXPIRED to EACCES in call_refreshresult Greg Kroah-Hartman
2013-11-26 18:12 ` Greg Kroah-Hartman [this message]
2013-11-26 18:12 ` [PATCH 3.11 19/36] PCI: Allow PCIe Capability link-related register access for switches Greg Kroah-Hartman
2013-11-26 18:12 ` [PATCH 3.11 20/36] PCI: Remove PCIe Capability version checks Greg Kroah-Hartman
2013-11-26 18:12 ` [PATCH 3.11 21/36] PCI: Support PCIe Capability Slot registers only for ports with slots Greg Kroah-Hartman
2013-11-26 18:12 ` [PATCH 3.11 22/36] perf/ftrace: Fix paranoid level for enabling function tracer Greg Kroah-Hartman
2013-11-26 18:12 ` [PATCH 3.11 23/36] ACPI / EC: Ensure lock is acquired before accessing ec struct members Greg Kroah-Hartman
2013-11-26 18:12 ` [PATCH 3.11 24/36] ACPI / video: Quirk initial backlight level 0 Greg Kroah-Hartman
2013-11-26 18:12 ` [PATCH 3.11 25/36] ACPI / hotplug: Fix handle_root_bridge_removal() Greg Kroah-Hartman
2013-11-26 18:12 ` [PATCH 3.11 26/36] ACPI / hotplug: Do not execute "insert in progress" _OST Greg Kroah-Hartman
2013-11-26 18:12 ` [PATCH 3.11 27/36] Staging: zram: Fix access of NULL pointer Greg Kroah-Hartman
2013-11-26 18:12 ` [PATCH 3.11 28/36] staging: comedi: avoid memleak for subdevice private Greg Kroah-Hartman
2013-11-26 18:12 ` [PATCH 3.11 29/36] Drivers: hv: vmbus: Fix a bug in channel rescind code Greg Kroah-Hartman
2013-11-26 18:12 ` [PATCH 3.11 30/36] rt2x00: fix a crash bug in the HT descriptor handling fix Greg Kroah-Hartman
2013-11-26 18:12 ` [PATCH 3.11 31/36] rt2x00: check if device is still available on rt2x00mac_flush() Greg Kroah-Hartman
2013-11-26 18:12 ` [PATCH 3.11 32/36] rt2x00: rt2800lib: fix VGC adjustment for RT5592 Greg Kroah-Hartman
2013-11-26 18:12 ` [PATCH 3.11 33/36] rt2x00: fix HT TX descriptor settings regression Greg Kroah-Hartman
2013-11-26 18:12 ` [PATCH 3.11 34/36] Bluetooth: revert: "Bluetooth: Add missing reset_resume dev_pm_ops" Greg Kroah-Hartman
2013-11-26 18:12 ` [PATCH 3.11 35/36] Revert "ima: policy for RAMFS" Greg Kroah-Hartman
2013-11-26 18:12 ` [PATCH 3.11 36/36] exec/ptrace: fix get_dumpable() incorrect tests Greg Kroah-Hartman
2013-11-27  4:12 ` [PATCH 3.11 00/36] 3.11.10-stable review Guenter Roeck
2013-11-27 22:30 ` Shuah Khan
2013-11-28 10:56 ` Satoru Takeuchi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20131126181028.831748517@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=bitbucket@online.de \
    --cc=lenb@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=stable@vger.kernel.org \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox