From d5a64513c6a171262082c250592c062e97a2c693 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 9 Apr 2010 01:39:40 +0200 Subject: ACPI / EC / PM: Fix race between EC transactions and system suspend There still is a race that may result in suspending the system in the middle of an EC transaction in progress, which leads to problems (like the kernel thinking that the ACPI global lock is held during resume while in fact it's not). To remove the race condition, modify the ACPI platform suspend and hibernate callbacks so that EC transactions are blocked right after executing the _PTS global control method and are allowed to happen again right after the low-level wakeup. Introduce acpi_pm_freeze() that will disable GPEs, wait until the event queues are empty and block EC transactions. Use it wherever GPEs are disabled in preparation for switching local interrupts off. Introduce acpi_pm_thaw() that will allow EC transactions to happen again and enable runtime GPEs. Use it to balance acpi_pm_freeze() wherever necessary. In addition to that use acpi_ec_resume_transactions_early() to unblock EC transactions as early as reasonably possible during resume. Also unblock EC transactions in acpi_hibernation_finish() and in the analogous suspend routine to make sure that the EC transactions are enabled in all error paths. Fixes https://bugzilla.kernel.org/show_bug.cgi?id=14668 Signed-off-by: Rafael J. Wysocki Reported-and-tested-by: Maxim Levitsky Signed-off-by: Len Brown diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index f2234db..2c2b73a 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -485,6 +485,16 @@ void acpi_ec_resume_transactions(void) mutex_unlock(&ec->lock); } +void acpi_ec_resume_transactions_early(void) +{ + /* + * Allow transactions to happen again (this function is called from + * atomic context during wakeup, so we don't need to acquire the mutex). + */ + if (first_ec) + clear_bit(EC_FLAGS_FROZEN, &first_ec->flags); +} + static int acpi_ec_query_unlocked(struct acpi_ec *ec, u8 * data) { int result; diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h index e284113..0ec48c7 100644 --- a/drivers/acpi/internal.h +++ b/drivers/acpi/internal.h @@ -51,6 +51,7 @@ int acpi_ec_ecdt_probe(void); int acpi_boot_ec_enable(void); void acpi_ec_suspend_transactions(void); void acpi_ec_resume_transactions(void); +void acpi_ec_resume_transactions_early(void); /*-------------------------------------------------------------------------- Suspend/Resume diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c index baa76bb..24741ac 100644 --- a/drivers/acpi/sleep.c +++ b/drivers/acpi/sleep.c @@ -110,11 +110,13 @@ void __init acpi_old_suspend_ordering(void) } /** - * acpi_pm_disable_gpes - Disable the GPEs. + * acpi_pm_freeze - Disable the GPEs and suspend EC transactions. */ -static int acpi_pm_disable_gpes(void) +static int acpi_pm_freeze(void) { acpi_disable_all_gpes(); + acpi_os_wait_events_complete(NULL); + acpi_ec_suspend_transactions(); return 0; } @@ -142,7 +144,8 @@ static int acpi_pm_prepare(void) int error = __acpi_pm_prepare(); if (!error) - acpi_disable_all_gpes(); + acpi_pm_freeze(); + return error; } @@ -275,6 +278,8 @@ static int acpi_suspend_enter(suspend_state_t pm_state) * acpi_leave_sleep_state will reenable specific GPEs later */ acpi_disable_all_gpes(); + /* Allow EC transactions to happen. */ + acpi_ec_resume_transactions_early(); local_irq_restore(flags); printk(KERN_DEBUG "Back to C!\n"); @@ -286,6 +291,12 @@ static int acpi_suspend_enter(suspend_state_t pm_state) return ACPI_SUCCESS(status) ? 0 : -EFAULT; } +static void acpi_suspend_finish(void) +{ + acpi_ec_resume_transactions(); + acpi_pm_finish(); +} + static int acpi_suspend_state_valid(suspend_state_t pm_state) { u32 acpi_state; @@ -307,7 +318,7 @@ static struct platform_suspend_ops acpi_suspend_ops = { .begin = acpi_suspend_begin, .prepare_late = acpi_pm_prepare, .enter = acpi_suspend_enter, - .wake = acpi_pm_finish, + .wake = acpi_suspend_finish, .end = acpi_pm_end, }; @@ -333,9 +344,9 @@ static int acpi_suspend_begin_old(suspend_state_t pm_state) static struct platform_suspend_ops acpi_suspend_ops_old = { .valid = acpi_suspend_state_valid, .begin = acpi_suspend_begin_old, - .prepare_late = acpi_pm_disable_gpes, + .prepare_late = acpi_pm_freeze, .enter = acpi_suspend_enter, - .wake = acpi_pm_finish, + .wake = acpi_suspend_finish, .end = acpi_pm_end, .recover = acpi_pm_finish, }; @@ -586,6 +597,7 @@ static int acpi_hibernation_enter(void) static void acpi_hibernation_finish(void) { hibernate_nvs_free(); + acpi_ec_resume_transactions(); acpi_pm_finish(); } @@ -606,17 +618,11 @@ static void acpi_hibernation_leave(void) } /* Restore the NVS memory area */ hibernate_nvs_restore(); + /* Allow EC transactions to happen. */ + acpi_ec_resume_transactions_early(); } -static int acpi_pm_pre_restore(void) -{ - acpi_disable_all_gpes(); - acpi_os_wait_events_complete(NULL); - acpi_ec_suspend_transactions(); - return 0; -} - -static void acpi_pm_restore_cleanup(void) +static void acpi_pm_thaw(void) { acpi_ec_resume_transactions(); acpi_enable_all_runtime_gpes(); @@ -630,8 +636,8 @@ static struct platform_hibernation_ops acpi_hibernation_ops = { .prepare = acpi_pm_prepare, .enter = acpi_hibernation_enter, .leave = acpi_hibernation_leave, - .pre_restore = acpi_pm_pre_restore, - .restore_cleanup = acpi_pm_restore_cleanup, + .pre_restore = acpi_pm_freeze, + .restore_cleanup = acpi_pm_thaw, }; /** @@ -663,12 +669,9 @@ static int acpi_hibernation_begin_old(void) static int acpi_hibernation_pre_snapshot_old(void) { - int error = acpi_pm_disable_gpes(); - - if (!error) - hibernate_nvs_save(); - - return error; + acpi_pm_freeze(); + hibernate_nvs_save(); + return 0; } /* @@ -680,11 +683,11 @@ static struct platform_hibernation_ops acpi_hibernation_ops_old = { .end = acpi_pm_end, .pre_snapshot = acpi_hibernation_pre_snapshot_old, .finish = acpi_hibernation_finish, - .prepare = acpi_pm_disable_gpes, + .prepare = acpi_pm_freeze, .enter = acpi_hibernation_enter, .leave = acpi_hibernation_leave, - .pre_restore = acpi_pm_pre_restore, - .restore_cleanup = acpi_pm_restore_cleanup, + .pre_restore = acpi_pm_freeze, + .restore_cleanup = acpi_pm_thaw, .recover = acpi_pm_finish, }; #endif /* CONFIG_HIBERNATION */ -- cgit v0.10.2 From fe955682d2153b35dffcf1673dff0491096a3f0a Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 9 Apr 2010 01:40:38 +0200 Subject: ACPI / EC / PM: Fix names of functions that block/unblock EC transactions The names of the functions used for blocking/unblocking EC transactions during suspend/hibernation suggest that the transactions are suspended and resumed by them, while in fact they are disabled and enabled. Rename the functions (and the flag used by them) to better reflect what they really do. Signed-off-by: Rafael J. Wysocki Signed-off-by: Len Brown diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index 2c2b73a..3f01f06 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -79,7 +79,7 @@ enum { EC_FLAGS_GPE_STORM, /* GPE storm detected */ EC_FLAGS_HANDLERS_INSTALLED, /* Handlers for GPE and * OpReg are installed */ - EC_FLAGS_FROZEN, /* Transactions are suspended */ + EC_FLAGS_BLOCKED, /* Transactions are blocked */ }; /* If we find an EC via the ECDT, we need to keep a ptr to its context */ @@ -293,7 +293,7 @@ static int acpi_ec_transaction(struct acpi_ec *ec, struct transaction *t) if (t->rdata) memset(t->rdata, 0, t->rlen); mutex_lock(&ec->lock); - if (test_bit(EC_FLAGS_FROZEN, &ec->flags)) { + if (test_bit(EC_FLAGS_BLOCKED, &ec->flags)) { status = -EINVAL; goto unlock; } @@ -459,7 +459,7 @@ int ec_transaction(u8 command, EXPORT_SYMBOL(ec_transaction); -void acpi_ec_suspend_transactions(void) +void acpi_ec_block_transactions(void) { struct acpi_ec *ec = first_ec; @@ -468,11 +468,11 @@ void acpi_ec_suspend_transactions(void) mutex_lock(&ec->lock); /* Prevent transactions from being carried out */ - set_bit(EC_FLAGS_FROZEN, &ec->flags); + set_bit(EC_FLAGS_BLOCKED, &ec->flags); mutex_unlock(&ec->lock); } -void acpi_ec_resume_transactions(void) +void acpi_ec_unblock_transactions(void) { struct acpi_ec *ec = first_ec; @@ -481,18 +481,18 @@ void acpi_ec_resume_transactions(void) mutex_lock(&ec->lock); /* Allow transactions to be carried out again */ - clear_bit(EC_FLAGS_FROZEN, &ec->flags); + clear_bit(EC_FLAGS_BLOCKED, &ec->flags); mutex_unlock(&ec->lock); } -void acpi_ec_resume_transactions_early(void) +void acpi_ec_unblock_transactions_early(void) { /* * Allow transactions to happen again (this function is called from * atomic context during wakeup, so we don't need to acquire the mutex). */ if (first_ec) - clear_bit(EC_FLAGS_FROZEN, &first_ec->flags); + clear_bit(EC_FLAGS_BLOCKED, &first_ec->flags); } static int acpi_ec_query_unlocked(struct acpi_ec *ec, u8 * data) diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h index 0ec48c7..f8f190e 100644 --- a/drivers/acpi/internal.h +++ b/drivers/acpi/internal.h @@ -49,9 +49,9 @@ void acpi_early_processor_set_pdc(void); int acpi_ec_init(void); int acpi_ec_ecdt_probe(void); int acpi_boot_ec_enable(void); -void acpi_ec_suspend_transactions(void); -void acpi_ec_resume_transactions(void); -void acpi_ec_resume_transactions_early(void); +void acpi_ec_block_transactions(void); +void acpi_ec_unblock_transactions(void); +void acpi_ec_unblock_transactions_early(void); /*-------------------------------------------------------------------------- Suspend/Resume diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c index 24741ac..504a55e 100644 --- a/drivers/acpi/sleep.c +++ b/drivers/acpi/sleep.c @@ -116,7 +116,7 @@ static int acpi_pm_freeze(void) { acpi_disable_all_gpes(); acpi_os_wait_events_complete(NULL); - acpi_ec_suspend_transactions(); + acpi_ec_block_transactions(); return 0; } @@ -279,7 +279,7 @@ static int acpi_suspend_enter(suspend_state_t pm_state) */ acpi_disable_all_gpes(); /* Allow EC transactions to happen. */ - acpi_ec_resume_transactions_early(); + acpi_ec_unblock_transactions_early(); local_irq_restore(flags); printk(KERN_DEBUG "Back to C!\n"); @@ -293,7 +293,7 @@ static int acpi_suspend_enter(suspend_state_t pm_state) static void acpi_suspend_finish(void) { - acpi_ec_resume_transactions(); + acpi_ec_unblock_transactions(); acpi_pm_finish(); } @@ -597,7 +597,7 @@ static int acpi_hibernation_enter(void) static void acpi_hibernation_finish(void) { hibernate_nvs_free(); - acpi_ec_resume_transactions(); + acpi_ec_unblock_transactions(); acpi_pm_finish(); } @@ -619,12 +619,12 @@ static void acpi_hibernation_leave(void) /* Restore the NVS memory area */ hibernate_nvs_restore(); /* Allow EC transactions to happen. */ - acpi_ec_resume_transactions_early(); + acpi_ec_unblock_transactions_early(); } static void acpi_pm_thaw(void) { - acpi_ec_resume_transactions(); + acpi_ec_unblock_transactions(); acpi_enable_all_runtime_gpes(); } -- cgit v0.10.2 From 0a76a34ff0804f1f413807b2e2d12117c2b602ca Mon Sep 17 00:00:00 2001 From: Len Brown Date: Tue, 1 Jun 2010 12:13:23 -0400 Subject: ACPI: update feature-removal.txt to reflect deleted acpi=ht option Per plan, acpi=ht was removed in 2.6.35-rc1. Signed-off-by: Len Brown diff --git a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt index 672be01..c268783 100644 --- a/Documentation/feature-removal-schedule.txt +++ b/Documentation/feature-removal-schedule.txt @@ -578,15 +578,6 @@ Who: Avi Kivity ---------------------------- -What: "acpi=ht" boot option -When: 2.6.35 -Why: Useful in 2003, implementation is a hack. - Generally invoked by accident today. - Seen as doing more harm than good. -Who: Len Brown - ----------------------------- - What: iwlwifi 50XX module parameters When: 2.6.40 Why: The "..50" modules parameters were used to configure 5000 series and -- cgit v0.10.2 From 157317ba3ec3e5a4d9683b8d24ba40b4f8f3296b Mon Sep 17 00:00:00 2001 From: Zhao Yakui Date: Wed, 2 Jun 2010 11:04:09 +0800 Subject: ACPI: Fix the incorrect calculation about C-state idle time The C-state idle time is not calculated correctly, which will return the wrong residency time in C-state. It will have the following effects: 1. The system can't choose the deeper C-state when it is idle next time. Of course the system power is increased. E.g. On one server machine about 40W idle power is increased. 2. The powertop shows that it will stay in C0 running state about 95% time although the system is idle at most time. 2.6.35-rc1 regression caused-by: 2da513f582a96c053aacc2c92873978d2ea7abff (ACPI: Minor cleanup eliminating redundant PMTIMER_TICKS to NS conversion) Signed-off-by: Zhao Yakui Reported-by: Yu Zhidong Tested-by: Yu Zhidong Acked-by: Venkatesh Pallipadi Signed-off-by: Len Brown diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c index 2e8c27d..6b38a6b 100644 --- a/drivers/acpi/processor_idle.c +++ b/drivers/acpi/processor_idle.c @@ -1022,7 +1022,7 @@ static int acpi_idle_enter_bm(struct cpuidle_device *dev, spin_unlock(&c3_lock); } kt2 = ktime_get_real(); - idle_time_ns = ktime_to_us(ktime_sub(kt2, kt1)); + idle_time_ns = ktime_to_ns(ktime_sub(kt2, kt1)); idle_time = idle_time_ns; do_div(idle_time, NSEC_PER_USEC); -- cgit v0.10.2 From bceefad59ab66d1b1a815a1738744ea013da966e Mon Sep 17 00:00:00 2001 From: Venkatesh Pallipadi Date: Wed, 2 Jun 2010 10:01:09 -0700 Subject: ACPI: Eliminate us to pm ticks conversion in common path acpi_enter_[simple|bm] routines does us to pm tick conversion on every idle wakeup and the value is only used in /proc/acpi display. We can store the time in us and convert it into pm ticks before printing instead and avoid the conversion in the common path. Signed-off-by: Venkatesh Pallipadi Signed-off-by: Len Brown diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c index 6b38a6b..b1b3856 100644 --- a/drivers/acpi/processor_idle.c +++ b/drivers/acpi/processor_idle.c @@ -80,7 +80,7 @@ module_param(nocst, uint, 0000); static unsigned int latency_factor __read_mostly = 2; module_param(latency_factor, uint, 0644); -static s64 us_to_pm_timer_ticks(s64 t) +static u64 us_to_pm_timer_ticks(s64 t) { return div64_u64(t * PM_TIMER_FREQUENCY, 1000000); } @@ -731,10 +731,10 @@ static int acpi_processor_power_seq_show(struct seq_file *seq, void *offset) seq_puts(seq, "demotion[--] "); - seq_printf(seq, "latency[%03d] usage[%08d] duration[%020llu]\n", + seq_printf(seq, "latency[%03d] usage[%08d] duration[%020Lu]\n", pr->power.states[i].latency, pr->power.states[i].usage, - (unsigned long long)pr->power.states[i].time); + us_to_pm_timer_ticks(pr->power.states[i].time)); } end: @@ -861,7 +861,6 @@ static int acpi_idle_enter_simple(struct cpuidle_device *dev, ktime_t kt1, kt2; s64 idle_time_ns; s64 idle_time; - s64 sleep_ticks = 0; pr = __get_cpu_var(processors); @@ -906,8 +905,6 @@ static int acpi_idle_enter_simple(struct cpuidle_device *dev, idle_time = idle_time_ns; do_div(idle_time, NSEC_PER_USEC); - sleep_ticks = us_to_pm_timer_ticks(idle_time); - /* Tell the scheduler how much we idled: */ sched_clock_idle_wakeup_event(idle_time_ns); @@ -918,7 +915,7 @@ static int acpi_idle_enter_simple(struct cpuidle_device *dev, cx->usage++; lapic_timer_state_broadcast(pr, cx, 0); - cx->time += sleep_ticks; + cx->time += idle_time; return idle_time; } @@ -940,7 +937,6 @@ static int acpi_idle_enter_bm(struct cpuidle_device *dev, ktime_t kt1, kt2; s64 idle_time_ns; s64 idle_time; - s64 sleep_ticks = 0; pr = __get_cpu_var(processors); @@ -1026,7 +1022,6 @@ static int acpi_idle_enter_bm(struct cpuidle_device *dev, idle_time = idle_time_ns; do_div(idle_time, NSEC_PER_USEC); - sleep_ticks = us_to_pm_timer_ticks(idle_time); /* Tell the scheduler how much we idled: */ sched_clock_idle_wakeup_event(idle_time_ns); @@ -1037,7 +1032,7 @@ static int acpi_idle_enter_bm(struct cpuidle_device *dev, cx->usage++; lapic_timer_state_broadcast(pr, cx, 0); - cx->time += sleep_ticks; + cx->time += idle_time; return idle_time; } -- cgit v0.10.2