ACPI patches for 2.6.35-rc1

PATCH In review Len Brown 2 participants 7 messages Jun 4, 2010 – Jun 2, 2017

Subsystems (2)

Documentation ACPI

lore

Share & tools

lore

Len Brown

author reviewer

2010-06-04 19:47 UTC #0 lore raw

Here is the contents of the current acpi release branch queued for upstream.

Len Brown

author reviewer

2010-06-04 19:47 UTC #1 lore raw

PATCH 1/5 ACPI / EC / PM: Fix race between EC transactions and system suspend

From: Rafael J. Wysocki <[email protected]>

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 <[email protected]>

Reported-and-tested-by Maxim Levitsky <[email protected]>

Signed-off-by Len Brown <[email protected]>

---
 drivers/acpi/ec.c       |   10 ++++++++
 drivers/acpi/internal.h |    1 +
 drivers/acpi/sleep.c    |   55 ++++++++++++++++++++++++----------------------
 3 files changed, 40 insertions(+), 26 deletions(-)

drivers/acpi/ec.c +10 -0

index f2234db..2c2b73a 100644
@@ -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;

drivers/acpi/internal.h +1 -0

index e284113..0ec48c7 100644
@@ -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

drivers/acpi/sleep.c +29 ~~-26~~

129 lines not on this page · raw message

Signature · 1 line

1.6.0.6

Len Brown

author reviewer

2010-06-04 19:47 UTC #2 lore raw

PATCH 2/5 ACPI / EC / PM: Fix names of functions that block/unblock EC transactions

In reply to Len Brown

From: Rafael J. Wysocki <[email protected]>

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 <[email protected]>

Signed-off-by Len Brown <[email protected]>

---
 drivers/acpi/ec.c       |   16 ++++++++--------
 drivers/acpi/internal.h |    6 +++---
 drivers/acpi/sleep.c    |   12 ++++++------
 3 files changed, 17 insertions(+), 17 deletions(-)

drivers/acpi/ec.c +8 -8

index 2c2b73a..3f01f06 100644
@@ -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)

drivers/acpi/internal.h +3 -3

index 0ec48c7..f8f190e 100644
@@ -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

drivers/acpi/sleep.c +6 -6

index 24741ac..504a55e 100644
@@ -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();
 }

Signature · 1 line

1.6.0.6

Len Brown

author reviewer

2010-06-04 19:47 UTC #3 lore raw

PATCH 3/5 ACPI: update feature-removal.txt to reflect deleted acpi=ht option

In reply to Len Brown

From: Len Brown <[email protected]>

Per plan, acpi=ht was removed in 2.6.35-rc1.

Signed-off-by Len Brown <[email protected]>

---
 Documentation/feature-removal-schedule.txt |    9 ---------
 1 files changed, 0 insertions(+), 9 deletions(-)

Documentation/feature-removal-schedule.txt +0 -9

index 672be01..c268783 100644
@@ -578,15 +578,6 @@ Who:	Avi Kivity <[email protected]>
 
 ----------------------------
 
-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 <[email protected]>
-
-----------------------------
-
 What:	iwlwifi 50XX module parameters
 When:	2.6.40
 Why:	The "..50" modules parameters were used to configure 5000 series and

Signature · 1 line

1.6.0.6

Jike Song

2017-06-02 09:30 UTC #4 lore raw

Re: [PATCH 3/5] ACPI: update feature-removal.txt to reflect deleted acpi=ht option

Hi Len,

Sorry for replying such a old mail written ~7 years ago, but I actually found there is still "acpi=ht" documented in current kernel:

Documentation/x86/x86_64/boot-options.txt

acpi=ht Use ACPI boot table parsing, but don't enable ACPI interpreter

Quoted 36 lines · Len Brown

From: Len Brown <[email protected]>
Per plan, acpi=ht was removed in 2.6.35-rc1.
Signed-off-by Len Brown <[email protected]>
---
 Documentation/feature-removal-schedule.txt |    9 ---------
 1 files changed, 0 insertions(+), 9 deletions(-)
Documentation/feature-removal-schedule.txt +0 ~~-10~~
index 672be01..c268783 100644
@@ -578,15 +578,6 @@ Who:       Avi Kivity <[email protected]>
 
 ----------------------------
 
-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 <[email protected]>
-
-----------------------------
-
 What:  iwlwifi 50XX module parameters
 When:  2.6.40
 Why:   The "..50" modules parameters were used to configure 5000 series and
--
1.6.0.6
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 2 lines

Thanks,
Jike

Len Brown

author reviewer

2010-06-04 19:47 UTC #5 lore raw

PATCH 4/5 ACPI: Fix the incorrect calculation about C-state idle time

In reply to Len Brown

From: Zhao Yakui <[email protected]>

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 <[email protected]>

Reported-by Yu Zhidong <[email protected]>

Tested-by Yu Zhidong <[email protected]>

Acked-by Venkatesh Pallipadi <[email protected]>

Signed-off-by Len Brown <[email protected]>

---
 drivers/acpi/processor_idle.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

drivers/acpi/processor_idle.c +1 -1

index 2e8c27d..6b38a6b 100644
@@ -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);

Signature · 1 line

1.6.0.6

Len Brown

author reviewer

2010-06-04 19:47 UTC #6 lore raw

PATCH 5/5 ACPI: Eliminate us to pm ticks conversion in common path

In reply to Len Brown

From: Venkatesh Pallipadi <[email protected]>

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 <[email protected]>

Signed-off-by Len Brown <[email protected]>

---
 drivers/acpi/processor_idle.c |   15 +++++----------
 1 files changed, 5 insertions(+), 10 deletions(-)

drivers/acpi/processor_idle.c +5 ~~-10~~

index 6b38a6b..b1b3856 100644
@@ -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;
 }

Signature · 1 line

1.6.0.6