From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
The flushing of pending work in the EC driver uses drain_workqueue()
to flush the event handling work that can requeue itself via
advance_transaction(), but this is problematic, because that
work may also be requeued from the query workqueue.
Namely, if an EC transaction is carried out during the execution of
a query handler, it involves calling advance_transaction() which
may queue up the event handling work again. This causes the kernel
to complain about attempts to add a work item to the EC event
workqueue while it is being drained and worst-case it may cause a
valid event to be skipped.
To avoid this problem, introduce two new counters, events_in_progress
and queries_in_progress, incremented when a work item is queued on
the event workqueue or the query workqueue, respectively, and
decremented at the end of the corresponding work function, and make
acpi_ec_dispatch_gpe() the workqueues in a loop until the both of
these counters are zero (or system wakeup is pending) instead of
calling acpi_ec_flush_work().
At the same time, change __acpi_ec_flush_work() to call
flush_workqueue() instead of drain_workqueue() to flush the event
workqueue.
While at it, use the observation that the work item queued in
acpi_ec_query() cannot be pending at that time, because it is used
only once, to simplify the code in there.
Additionally, clean up a comment in acpi_ec_query() and adjust white
space in acpi_ec_event_processor().
Fixes: f0ac20c3f613 ("ACPI: EC: Fix flushing of pending work")
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
---
drivers/acpi/ec.c | 57 ++++++++++++++++++++++++++++++++++++------------
drivers/acpi/internal.h | 2 +
2 files changed, 45 insertions(+), 14 deletions(-)
Index: linux-pm/drivers/acpi/ec.c
===================================================================
--- linux-pm.orig/drivers/acpi/ec.c
+++ linux-pm/drivers/acpi/ec.c
@@ -166,6 +166,7 @@ struct acpi_ec_query {
struct transaction transaction;
struct work_struct work;
struct acpi_ec_query_handler *handler;
+ struct acpi_ec *ec;
};
static int acpi_ec_query(struct acpi_ec *ec, u8 *data);
@@ -452,6 +453,7 @@ static void acpi_ec_submit_query(struct
ec_dbg_evt("Command(%s) submitted/blocked",
acpi_ec_cmd_string(ACPI_EC_COMMAND_QUERY));
ec->nr_pending_queries++;
+ ec->events_in_progress++;
queue_work(ec_wq, &ec->work);
}
}
@@ -518,7 +520,7 @@ static void acpi_ec_enable_event(struct
#ifdef CONFIG_PM_SLEEP
static void __acpi_ec_flush_work(void)
{
- drain_workqueue(ec_wq); /* flush ec->work */
+ flush_workqueue(ec_wq); /* flush ec->work */
flush_workqueue(ec_query_wq); /* flush queries */
}
@@ -1103,7 +1105,7 @@ void acpi_ec_remove_query_handler(struct
}
EXPORT_SYMBOL_GPL(acpi_ec_remove_query_handler);
-static struct acpi_ec_query *acpi_ec_create_query(u8 *pval)
+static struct acpi_ec_query *acpi_ec_create_query(struct acpi_ec *ec, u8 *pval)
{
struct acpi_ec_query *q;
struct transaction *t;
@@ -1111,11 +1113,13 @@ static struct acpi_ec_query *acpi_ec_cre
q = kzalloc(sizeof (struct acpi_ec_query), GFP_KERNEL);
if (!q)
return NULL;
+
INIT_WORK(&q->work, acpi_ec_event_processor);
t = &q->transaction;
t->command = ACPI_EC_COMMAND_QUERY;
t->rdata = pval;
t->rlen = 1;
+ q->ec = ec;
return q;
}
@@ -1132,13 +1136,21 @@ static void acpi_ec_event_processor(stru
{
struct acpi_ec_query *q = container_of(work, struct acpi_ec_query, work);
struct acpi_ec_query_handler *handler = q->handler;
+ struct acpi_ec *ec = q->ec;
ec_dbg_evt("Query(0x%02x) started", handler->query_bit);
+
if (handler->func)
handler->func(handler->data);
else if (handler->handle)
acpi_evaluate_object(handler->handle, NULL, NULL, NULL);
+
ec_dbg_evt("Query(0x%02x) stopped", handler->query_bit);
+
+ spin_lock_irq(&ec->lock);
+ ec->queries_in_progress--;
+ spin_unlock_irq(&ec->lock);
+
acpi_ec_delete_query(q);
}
@@ -1148,7 +1160,7 @@ static int acpi_ec_query(struct acpi_ec
int result;
struct acpi_ec_query *q;
- q = acpi_ec_create_query(&value);
+ q = acpi_ec_create_query(ec, &value);
if (!q)
return -ENOMEM;
@@ -1170,19 +1182,20 @@ static int acpi_ec_query(struct acpi_ec
}
/*
- * It is reported that _Qxx are evaluated in a parallel way on
- * Windows:
+ * It is reported that _Qxx are evaluated in a parallel way on Windows:
* https://bugzilla.kernel.org/show_bug.cgi?id=94411
*
- * Put this log entry before schedule_work() in order to make
- * it appearing before any other log entries occurred during the
- * work queue execution.
+ * Put this log entry before queue_work() to make it appear in the log
+ * before any other messages emitted during workqueue handling.
*/
ec_dbg_evt("Query(0x%02x) scheduled", value);
- if (!queue_work(ec_query_wq, &q->work)) {
- ec_dbg_evt("Query(0x%02x) overlapped", value);
- result = -EBUSY;
- }
+
+ spin_lock_irq(&ec->lock);
+
+ ec->queries_in_progress++;
+ queue_work(ec_query_wq, &q->work);
+
+ spin_unlock_irq(&ec->lock);
err_exit:
if (result)
@@ -1240,6 +1253,10 @@ static void acpi_ec_event_handler(struct
ec_dbg_evt("Event stopped");
acpi_ec_check_event(ec);
+
+ spin_lock_irqsave(&ec->lock, flags);
+ ec->events_in_progress--;
+ spin_unlock_irqrestore(&ec->lock, flags);
}
static void acpi_ec_handle_interrupt(struct acpi_ec *ec)
@@ -2021,6 +2038,7 @@ void acpi_ec_set_gpe_wake_mask(u8 action
bool acpi_ec_dispatch_gpe(void)
{
+ bool work_in_progress;
u32 ret;
if (!first_ec)
@@ -2041,8 +2059,19 @@ bool acpi_ec_dispatch_gpe(void)
if (ret == ACPI_INTERRUPT_HANDLED)
pm_pr_dbg("ACPI EC GPE dispatched\n");
- /* Flush the event and query workqueues. */
- acpi_ec_flush_work();
+ /* Drain EC work. */
+ do {
+ acpi_ec_flush_work();
+
+ pm_pr_dbg("ACPI EC work flushed\n");
+
+ spin_lock_irq(&first_ec->lock);
+
+ work_in_progress = first_ec->events_in_progress +
+ first_ec->queries_in_progress > 0;
+
+ spin_unlock_irq(&first_ec->lock);
+ } while (work_in_progress && !pm_wakeup_pending());
return false;
}
Index: linux-pm/drivers/acpi/internal.h
===================================================================
--- linux-pm.orig/drivers/acpi/internal.h
+++ linux-pm/drivers/acpi/internal.h
@@ -184,6 +184,8 @@ struct acpi_ec {
struct work_struct work;
unsigned long timestamp;
unsigned long nr_pending_queries;
+ unsigned int events_in_progress;
+ unsigned int queries_in_progress;
bool busy_polling;
unsigned int polling_guard;
};
