linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Rafael J. Wysocki" <rjw@rjwysocki.net>
To: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Linux PM list <linux-pm@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	ACPI Devel Maling List <linux-acpi@vger.kernel.org>
Subject: [v2][PATCH 01/21] ACPI / PAD: Remove the local APIC nonsense
Date: Fri, 03 Apr 2015 01:46:34 +0200	[thread overview]
Message-ID: <1887788.RObRuI4tSv@vostro.rjw.lan> (raw)
In-Reply-To: <3410621.5uMny9y0B7@vostro.rjw.lan>

From: Thomas Gleixner <tglx@linutronix.de>

While looking through the (ab)use of the clockevents_notify() function
I stumbled over the following gem in the acpi_pad code:

  if (lapic_detected_unstable && !lapic_marked_unstable) {
     /* LAPIC could halt in idle, so notify users */
     for_each_online_cpu(i)
       clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ON, &i);
     lapic_marked_unstable = 1;
  }

This code calls on the cpu which detects the lapic unstable condition
first clockevents_notify() to tell the core code that the broadcast
should be enabled on all online cpus. Brilliant stuff that as it
notifies the core code a num_online_cpus() times that the broadcast
should be enabled on the current cpu.

This probably has never been noticed because that code got never
tested with NOHZ=n and HIGHRES_TIMER=n or it just worked by chance
because one of the other mechanisms told the core in the right way
that the local apic timer is wreckaged.

Sigh, this is:

 - The 4th incarnation of idle drivers which has their own mechanism
   to detect and deal with X86_FEATURE_ARAT.

 - The 2nd incarnation of fake idle mechanisms with a different set of
   brainmelting bugs.

 - Has been merged against an explicit NAK of the scheduler
   maintainer with the promise to improve it over time.

 - Another example of featuritis driven trainwreck engineering.

 - Another pointless waste of my time.

Fix this nonsense by removing that lapic detection and notification
logic and simply call into the clockevents code unconditonally. The
ARAT feature is marked in the lapic clockevent already so the core
code will just ignore the requests and return.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
diff --git a/drivers/acpi/acpi_pad.c b/drivers/acpi/acpi_pad.c
index c7b105c0e1d3..1686e9f756fd 100644
--- a/drivers/acpi/acpi_pad.c
+++ b/drivers/acpi/acpi_pad.c
@@ -41,8 +41,6 @@ static unsigned long power_saving_mwait_eax;
 
 static unsigned char tsc_detected_unstable;
 static unsigned char tsc_marked_unstable;
-static unsigned char lapic_detected_unstable;
-static unsigned char lapic_marked_unstable;
 
 static void power_saving_mwait_init(void)
 {
@@ -82,13 +80,10 @@ static void power_saving_mwait_init(void)
 		 */
 		if (!boot_cpu_has(X86_FEATURE_NONSTOP_TSC))
 			tsc_detected_unstable = 1;
-		if (!boot_cpu_has(X86_FEATURE_ARAT))
-			lapic_detected_unstable = 1;
 		break;
 	default:
-		/* TSC & LAPIC could halt in idle */
+		/* TSC could halt in idle */
 		tsc_detected_unstable = 1;
-		lapic_detected_unstable = 1;
 	}
 #endif
 }
@@ -177,28 +172,17 @@ static int power_saving_thread(void *data)
 				mark_tsc_unstable("TSC halts in idle");
 				tsc_marked_unstable = 1;
 			}
-			if (lapic_detected_unstable && !lapic_marked_unstable) {
-				int i;
-				/* LAPIC could halt in idle, so notify users */
-				for_each_online_cpu(i)
-					clockevents_notify(
-						CLOCK_EVT_NOTIFY_BROADCAST_ON,
-						&i);
-				lapic_marked_unstable = 1;
-			}
+			clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ON, &cpu);
+
 			local_irq_disable();
 			cpu = smp_processor_id();
-			if (lapic_marked_unstable)
-				clockevents_notify(
-					CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &cpu);
+			clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &cpu);
 			stop_critical_timings();
 
 			mwait_idle_with_hints(power_saving_mwait_eax, 1);
 
 			start_critical_timings();
-			if (lapic_marked_unstable)
-				clockevents_notify(
-					CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &cpu);
+			clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &cpu);
 			local_irq_enable();
 
 			if (time_before(expire_time, jiffies)) {


  reply	other threads:[~2015-04-03  0:17 UTC|newest]

Thread overview: 90+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-01 22:02 [PATCH 00/20] clockevents_notify() removal Rafael J. Wysocki
2015-04-01 22:03 ` [PATCH 01/20] clockevents: Provide explicit broadcast control functions Rafael J. Wysocki
2015-04-01 22:04 ` [PATCH 02/20] x86, amd_idle: Use explicit broadcast control function Rafael J. Wysocki
2015-04-01 22:05 ` [PATCH 03/20] ACPI / PAD: " Rafael J. Wysocki
2015-04-01 22:06 ` [PATCH 04/20] ACPI / processor: " Rafael J. Wysocki
2015-04-01 22:07 ` [PATCH 05/20] cpuidle: " Rafael J. Wysocki
2015-04-01 22:09 ` [PATCH 06/20] intel_idle: " Rafael J. Wysocki
2015-04-01 22:10 ` [PATCH 07/20] ARM: OMAP: " Rafael J. Wysocki
2015-04-01 22:11 ` [PATCH 08/20] clockevents: Remove the broadcast control leftovers Rafael J. Wysocki
2015-04-01 22:13 ` [PATCH 09/20] clockevents: Provide explicit broadcast oneshot control functions Rafael J. Wysocki
2015-04-01 22:15 ` [PATCH 10/20] x86, amd_idle: Use " Rafael J. Wysocki
2015-04-01 22:16 ` [PATCH 11/20] ACPI / PAD: Use explicit broadcast oneshot control function Rafael J. Wysocki
2015-04-01 22:17 ` [PATCH 12/20] ACPI / processor: Use explicit broadcast controll function Rafael J. Wysocki
2015-04-01 22:20 ` [PATCH 13/20] intel_idle: Use explicit broadcast oneshot control function Rafael J. Wysocki
2015-04-01 22:20 ` [PATCH 14/20] ARM: OMAP: " Rafael J. Wysocki
2015-04-01 22:21 ` [PATCH 15/20] ARM: tegra: " Rafael J. Wysocki
2015-04-01 22:22 ` [PATCH 16/20] sched/idle: " Rafael J. Wysocki
2015-04-28 10:11   ` Linus Walleij
2015-04-28 10:17     ` Sudeep Holla
2015-04-28 10:34     ` Daniel Lezcano
2015-04-28 10:42       ` Sudeep Holla
2015-04-28 12:19         ` Rafael J. Wysocki
2015-04-28 12:37           ` Linus Walleij
2015-04-28 13:31             ` Rafael J. Wysocki
2015-04-28 13:37               ` Rafael J. Wysocki
2015-04-28 14:14                 ` Rafael J. Wysocki
2015-04-28 13:58                   ` Sudeep Holla
2015-04-29  0:50                     ` Rafael J. Wysocki
2015-04-29  1:04                       ` Rafael J. Wysocki
2015-04-29  7:10                         ` Linus Walleij
2015-04-29  8:57                         ` Peter Zijlstra
2015-04-29  9:44                         ` Daniel Lezcano
2015-04-29  9:50                         ` Sudeep Holla
2015-04-29 14:07                           ` [PATCH][Fix] cpuidle: Run tick_broadcast_exit() with disabled interrupts Rafael J. Wysocki
2015-04-30  3:47                             ` Preeti U Murthy
2015-04-30 20:12                             ` Nicolas Pitre
2015-04-30 22:10                               ` Rafael J. Wysocki
2015-04-30  3:45                         ` [PATCH 16/20] sched/idle: Use explicit broadcast oneshot control function Preeti U Murthy
2015-04-28 13:04           ` Sudeep Holla
2015-04-01 22:23 ` [PATCH 17/20] clockevents: Remove broadcast oneshot control leftovers Rafael J. Wysocki
2015-04-01 22:24 ` [PATCH 18/20] clockevents: Make tick handover explicit Rafael J. Wysocki
2015-04-01 22:25 ` [PATCH 19/20] clockevents: Cleanup dead cpu explicitely Rafael J. Wysocki
2015-04-01 22:26 ` [PATCH 20/20] timekeeping: Get rid of stale comment Rafael J. Wysocki
2015-04-01 23:57   ` John Stultz
2015-04-02 12:39 ` [PATCH 00/20] clockevents_notify() removal Ingo Molnar
2015-04-02 22:19   ` Rafael J. Wysocki
2015-04-02 23:45     ` [v2][PATCH 00/21] " Rafael J. Wysocki
2015-04-02 23:46       ` Rafael J. Wysocki [this message]
2015-04-03  8:21         ` [tip:timers/core] ACPI/PAD: Remove the local APIC nonsense tip-bot for Thomas Gleixner
2015-04-03  0:01       ` [v2][PATCH 02/21] clockevents: Provide explicit broadcast control functions Rafael J. Wysocki
2015-04-03  8:21         ` [tip:timers/core] " tip-bot for Thomas Gleixner
2015-04-03  0:01       ` [v2][PATCH 03/21] x86, amd_idle: Use explicit broadcast control function Rafael J. Wysocki
2015-04-03  8:22         ` [tip:timers/core] x86/amd/idle, clockevents: " tip-bot for Thomas Gleixner
2015-04-03  0:01       ` [v2][PATCH 04/21] ACPI / PAD: " Rafael J. Wysocki
2015-04-03  8:22         ` [tip:timers/core] ACPI/PAD: " tip-bot for Thomas Gleixner
2015-04-03  0:02       ` [v2][PATCH 05/21] ACPI / processor: " Rafael J. Wysocki
2015-04-03  8:22         ` [tip:timers/core] ACPI/processor: " tip-bot for Thomas Gleixner
2015-04-03  0:02       ` [v2][PATCH 06/21] cpuidle: " Rafael J. Wysocki
2015-04-03  8:23         ` [tip:timers/core] " tip-bot for Thomas Gleixner
2015-04-03  0:02       ` [v2][PATCH 07/21] intel_idle: " Rafael J. Wysocki
2015-04-03  8:23         ` [tip:timers/core] " tip-bot for Thomas Gleixner
2015-04-03  0:02       ` [v2][PATCH 08/21] ARM: OMAP: " Rafael J. Wysocki
2015-04-03  8:23         ` [tip:timers/core] " tip-bot for Thomas Gleixner
2015-04-03  0:03       ` [v2][PATCH 09/21] clockevents: Remove the broadcast control leftovers Rafael J. Wysocki
2015-04-03  8:23         ` [tip:timers/core] " tip-bot for Thomas Gleixner
2015-04-03  0:05       ` [v2][PATCH 10/21] clockevents: Provide explicit broadcast oneshot control functions Rafael J. Wysocki
2015-04-03  8:24         ` [tip:timers/core] " tip-bot for Thomas Gleixner
2015-04-03  0:05       ` [v2][PATCH 11/21] x86, amd_idle: Use " Rafael J. Wysocki
2015-04-03  8:24         ` [tip:timers/core] x86/amd/idle, clockevents: " tip-bot for Thomas Gleixner
2015-04-03  0:06       ` [v2][PATCH 12/21] ACPI / PAD: Use explicit broadcast oneshot control function Rafael J. Wysocki
2015-04-03  8:24         ` [tip:timers/core] ACPI/PAD: " tip-bot for Thomas Gleixner
2015-04-03  0:12       ` [v2][PATCH 13/21] ACPI / processor: Use explicit broadcast controll function Rafael J. Wysocki
2015-04-03  8:25         ` [tip:timers/core] ACPI/idle: Use explicit broadcast control function tip-bot for Thomas Gleixner
2015-04-03  0:14       ` [v2][PATCH 14/21] intel_idle: Use explicit broadcast oneshot " Rafael J. Wysocki
2015-04-03  8:25         ` [tip:timers/core] " tip-bot for Thomas Gleixner
2015-04-03  0:31       ` [v2][PATCH 15/21] ARM: OMAP: " Rafael J. Wysocki
2015-04-03  8:25         ` [tip:timers/core] " tip-bot for Thomas Gleixner
2015-04-03  0:32       ` [v2][PATCH 16/21] ARM: tegra: " Rafael J. Wysocki
2015-04-03  8:25         ` [tip:timers/core] ARM: Tegra: " tip-bot for Thomas Gleixner
2015-04-03  0:34       ` [v2][PATCH 17/21] sched/idle: " Rafael J. Wysocki
2015-04-03  8:26         ` [tip:timers/core] " tip-bot for Thomas Gleixner
2015-04-03  0:36       ` [v2][PATCH 18/21] clockevents: Remove broadcast oneshot control leftovers Rafael J. Wysocki
2015-04-03  8:26         ` [tip:timers/core] " tip-bot for Rafael J. Wysocki
2015-04-03  0:37       ` [v2][PATCH 19/21] clockevents: Make tick handover explicit Rafael J. Wysocki
2015-04-03  8:26         ` [tip:timers/core] " tip-bot for Thomas Gleixner
2015-04-03  0:38       ` [v2][PATCH 20/21] clockevents: Cleanup dead cpu explicitely Rafael J. Wysocki
2015-04-03  8:26         ` [tip:timers/core] " tip-bot for Thomas Gleixner
2015-04-03  0:39       ` [v2][PATCH 21/21] timekeeping: Get rid of stale comment Rafael J. Wysocki
2015-04-03  8:27         ` [tip:timers/core] " tip-bot for Thomas Gleixner
2015-04-03  6:45       ` [v2][PATCH 00/21] clockevents_notify() removal Ingo Molnar

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=1887788.RObRuI4tSv@vostro.rjw.lan \
    --to=rjw@rjwysocki.net \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --subject='Re: [v2][PATCH 01/21] ACPI / PAD: Remove the local APIC nonsense' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).