From 3ff70551a942b4c1d3c2e96e31a5c6e369a6d0be Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Fri, 3 Apr 2015 01:46:34 +0200 Subject: ACPI/PAD: Remove the local APIC nonsense 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 Signed-off-by: Rafael J. Wysocki Cc: Peter Zijlstra Cc: Borislav Petkov Cc: H. Peter Anvin Cc: Linus Torvalds Cc: Thomas Gleixner Link: http://lkml.kernel.org/r/1887788.RObRuI4tSv@vostro.rjw.lan Signed-off-by: Ingo Molnar --- drivers/acpi/acpi_pad.c | 26 +++++--------------------- 1 file changed, 5 insertions(+), 21 deletions(-) (limited to 'drivers/acpi') 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)) { -- cgit v1.2.3 From 979081e7440056da28b19e57acf20098caf49103 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Fri, 3 Apr 2015 02:01:49 +0200 Subject: ACPI/PAD: Use explicit broadcast control function Replace the clockevents_notify() call with an explicit function call. Signed-off-by: Thomas Gleixner Signed-off-by: Rafael J. Wysocki Cc: Len Brown Cc: Peter Zijlstra Link: http://lkml.kernel.org/r/1521832.mm0ZfkTzTA@vostro.rjw.lan Signed-off-by: Ingo Molnar --- drivers/acpi/acpi_pad.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'drivers/acpi') diff --git a/drivers/acpi/acpi_pad.c b/drivers/acpi/acpi_pad.c index 1686e9f756fd..244dbc9be99a 100644 --- a/drivers/acpi/acpi_pad.c +++ b/drivers/acpi/acpi_pad.c @@ -26,7 +26,7 @@ #include #include #include -#include +#include #include #include #include @@ -172,9 +172,8 @@ static int power_saving_thread(void *data) mark_tsc_unstable("TSC halts in idle"); tsc_marked_unstable = 1; } - clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ON, &cpu); - local_irq_disable(); + tick_broadcast_enable(); cpu = smp_processor_id(); clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &cpu); stop_critical_timings(); -- cgit v1.2.3 From ee41eebf9cef624b2e766a4b8688eeeedb65114c Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Fri, 3 Apr 2015 02:02:00 +0200 Subject: ACPI/processor: Use explicit broadcast control function Replace the clockevents_notify() call with an explicit function call. Signed-off-by: Thomas Gleixner Signed-off-by: Rafael J. Wysocki Cc: Peter Zijlstra Link: http://lkml.kernel.org/r/25071624.dkenaL3SGT@vostro.rjw.lan Signed-off-by: Ingo Molnar --- drivers/acpi/processor_idle.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) (limited to 'drivers/acpi') diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c index c6bb9f1257c9..be5479739287 100644 --- a/drivers/acpi/processor_idle.c +++ b/drivers/acpi/processor_idle.c @@ -32,7 +32,7 @@ #include #include #include /* need_resched() */ -#include +#include #include #include #include @@ -157,12 +157,11 @@ static void lapic_timer_check_state(int state, struct acpi_processor *pr, static void __lapic_timer_propagate_broadcast(void *arg) { struct acpi_processor *pr = (struct acpi_processor *) arg; - unsigned long reason; - reason = pr->power.timer_broadcast_on_state < INT_MAX ? - CLOCK_EVT_NOTIFY_BROADCAST_ON : CLOCK_EVT_NOTIFY_BROADCAST_OFF; - - clockevents_notify(reason, &pr->id); + if (pr->power.timer_broadcast_on_state < INT_MAX) + tick_broadcast_enable(); + else + tick_broadcast_disable(); } static void lapic_timer_propagate_broadcast(struct acpi_processor *pr) -- cgit v1.2.3 From c79521354e71a1fbcee040ee3147cadc0f8e3c97 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Fri, 3 Apr 2015 02:06:37 +0200 Subject: ACPI/PAD: Use explicit broadcast oneshot control function Replace the clockevents_notify() call with an explicit function call. Signed-off-by: Thomas Gleixner Signed-off-by: Rafael J. Wysocki Cc: Len Brown Cc: Peter Zijlstra Link: http://lkml.kernel.org/r/1552509.UntNmyqF5v@vostro.rjw.lan Signed-off-by: Ingo Molnar --- drivers/acpi/acpi_pad.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'drivers/acpi') diff --git a/drivers/acpi/acpi_pad.c b/drivers/acpi/acpi_pad.c index 244dbc9be99a..6bc9cbc01ad6 100644 --- a/drivers/acpi/acpi_pad.c +++ b/drivers/acpi/acpi_pad.c @@ -150,7 +150,6 @@ static int power_saving_thread(void *data) sched_setscheduler(current, SCHED_RR, ¶m); while (!kthread_should_stop()) { - int cpu; unsigned long expire_time; try_to_freeze(); @@ -174,14 +173,13 @@ static int power_saving_thread(void *data) } local_irq_disable(); tick_broadcast_enable(); - cpu = smp_processor_id(); - clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &cpu); + tick_broadcast_enter(); stop_critical_timings(); mwait_idle_with_hints(power_saving_mwait_eax, 1); start_critical_timings(); - clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &cpu); + tick_broadcast_exit(); local_irq_enable(); if (time_before(expire_time, jiffies)) { -- cgit v1.2.3 From 7815701c5cd7276b712d898b3cf49c55e587dbb1 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Fri, 3 Apr 2015 02:12:03 +0200 Subject: ACPI/idle: Use explicit broadcast control function Replace the clockevents_notify() call with an explicit function call. Signed-off-by: Thomas Gleixner Signed-off-by: Rafael J. Wysocki Cc: Peter Zijlstra Link: http://lkml.kernel.org/r/2653377.MSAlfA939I@vostro.rjw.lan Signed-off-by: Ingo Molnar --- drivers/acpi/processor_idle.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) (limited to 'drivers/acpi') diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c index be5479739287..53355196c5c6 100644 --- a/drivers/acpi/processor_idle.c +++ b/drivers/acpi/processor_idle.c @@ -178,11 +178,10 @@ static void lapic_timer_state_broadcast(struct acpi_processor *pr, int state = cx - pr->power.states; if (state >= pr->power.timer_broadcast_on_state) { - unsigned long reason; - - reason = broadcast ? CLOCK_EVT_NOTIFY_BROADCAST_ENTER : - CLOCK_EVT_NOTIFY_BROADCAST_EXIT; - clockevents_notify(reason, &pr->id); + if (broadcast) + tick_broadcast_enter(); + else + tick_broadcast_exit(); } } -- cgit v1.2.3