@@ -30,6 +30,7 @@ struct acpi_smb_hc {
u8 query_bit;
smbus_alarm_callback callback;
void *context;
+ bool done;
};
static int acpi_smbus_hc_add(struct acpi_device *device);
@@ -100,27 +101,11 @@ static inline int smb_hc_write(struct acpi_smb_hc *hc, u8 address, u8 data)
return ec_write(hc->offset + address, data);
}
-static inline int smb_check_done(struct acpi_smb_hc *hc)
-{
- union acpi_smb_status status = {.raw = 0};
- smb_hc_read(hc, ACPI_SMB_STATUS, &status.raw);
- return status.fields.done && (status.fields.status == SMBUS_OK);
-}
-
static int wait_transaction_complete(struct acpi_smb_hc *hc, int timeout)
{
- if (wait_event_timeout(hc->wait, smb_check_done(hc),
- msecs_to_jiffies(timeout)))
+ if (wait_event_timeout(hc->wait, hc->done, msecs_to_jiffies(timeout)))
return 0;
- /*
- * After the timeout happens, OS will try to check the status of SMbus.
- * If the status is what OS expected, it will be regarded as the bogus
- * timeout.
- */
- if (smb_check_done(hc))
- return 0;
- else
- return -ETIME;
+ return -ETIME;
}
static int acpi_smbus_transaction(struct acpi_smb_hc *hc, u8 protocol,
@@ -135,6 +120,7 @@ static int acpi_smbus_transaction(struct acpi_smb_hc *hc, u8 protocol,
}
mutex_lock(&hc->lock);
+ hc->done = false;
if (macbook)
udelay(5);
if (smb_hc_read(hc, ACPI_SMB_PROTOCOL, &temp))
@@ -235,8 +221,10 @@ static int smbus_alarm(void *context)
if (smb_hc_read(hc, ACPI_SMB_STATUS, &status.raw))
return 0;
/* Check if it is only a completion notify */
- if (status.fields.done)
+ if (status.fields.done && status.fields.status == SMBUS_OK) {
+ hc->done = true;
wake_up(&hc->wait);
+ }
if (!status.fields.alarm)
return 0;
mutex_lock(&hc->lock);
In the SBS initialisation, a reentrant call to wait_event_timeout() causes an intermittent boot stall of several minutes usually following the "Switching to clocksource tsc" message. Another symptom of this bug is high CPU usage from programs (Firefox, upowerd) querying the battery state. This is caused by: 1. drivers/acpi/sbshc.c wait_transaction_complete() calls wait_event_timeout(): if (wait_event_timeout(hc->wait, smb_check_done(hc), msecs_to_jiffies(timeout))) 2. ___wait_event sets task state to uninterruptible 3. ___wait_event calls the "condition" smb_check_done() 4. smb_check_done (sbshc.c) calls through to ec_read() in drivers/acpi/ec.c 5. ec_guard() is reached which calls wait_event_timeout() if (wait_event_timeout(ec->wait, ec_transaction_completed(ec), guard)) ie. wait_event_timeout() is being called again inside evaluation of the previous wait_event_timeout() condition 5. The EC IRQ handler calls wake_up() and wakes up the sleeping task in ec_guard() 6. The task is now in state running even though the wait "condition" is still being evaluated 7. The "condition" check returns false so ___wait_event calls schedule_timeout() 8. Since the task state is running, the scheduler immediately schedules it again 9. This loop usually repeats for around 250 seconds even though the original wait_event_timeout was only 1000ms. The timeout is incorrect because each call to schedule_timeout() usually returns immediately, taking less than 1ms, so the jiffies timeout counter is not decremented. The task is now stuck in a running state, and so is highly likely to be immediately rescheduled, which takes less than a jiffy. The loop will never exit if all schedule_timeout() calls take less than a jiffy. Fix this by replacing SMBus reads in the wait_event_timeout condition with checks of a boolean value that is updated by the EC query handler. Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=107191 Link: https://lkml.org/lkml/2015/11/6/776 Signed-off-by: Chris Bainbridge <chris.bainbridge@gmail.com> --- drivers/acpi/sbshc.c | 26 +++++++------------------- 1 file changed, 7 insertions(+), 19 deletions(-)