ACPI: thermal: Do not call acpi_thermal_check() directly

Calling acpi_thermal_check() from acpi_thermal_notify() directly
is problematic if _TMP triggers Notify () on the thermal zone for
which it has been evaluated (which happens on some systems), because
it causes a new acpi_thermal_notify() invocation to be queued up
every time and if that takes place too often, an indefinite number of
pending work items may accumulate in kacpi_notify_wq over time.

Besides, it is not really useful to queue up a new invocation of
acpi_thermal_check() if one of them is pending already.

For these reasons, rework acpi_thermal_notify() to queue up a thermal
check instead of calling acpi_thermal_check() directly and only allow
one thermal check to be pending at a time.  Moreover, only allow one
acpi_thermal_check_fn() instance at a time to run
thermal_zone_device_update() for one thermal zone and make it return
early if it sees other instances running for the same thermal zone.

While at it, fold acpi_thermal_check() into acpi_thermal_check_fn(),
as it is only called from there after the other changes made here.

[This issue appears to have been exposed by commit 6d25be5782
 ("sched/core, workqueues: Distangle worker accounting from rq
 lock"), but it is unclear why it was not visible earlier.]

BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=208877
Reported-by: Stephen Berman <stephen.berman@gmx.net>
Diagnosed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Tested-by: Stephen Berman <stephen.berman@gmx.net>
Cc: All applicable <stable@vger.kernel.org>
This commit is contained in:
Rafael J. Wysocki 2021-01-14 19:34:22 +01:00
parent 6ee1d745b7
commit 81b704d3e4
1 changed files with 33 additions and 13 deletions

View File

@ -174,6 +174,8 @@ struct acpi_thermal {
struct thermal_zone_device *thermal_zone; struct thermal_zone_device *thermal_zone;
int kelvin_offset; /* in millidegrees */ int kelvin_offset; /* in millidegrees */
struct work_struct thermal_check_work; struct work_struct thermal_check_work;
struct mutex thermal_check_lock;
refcount_t thermal_check_count;
}; };
/* -------------------------------------------------------------------------- /* --------------------------------------------------------------------------
@ -495,14 +497,6 @@ static int acpi_thermal_get_trip_points(struct acpi_thermal *tz)
return 0; return 0;
} }
static void acpi_thermal_check(void *data)
{
struct acpi_thermal *tz = data;
thermal_zone_device_update(tz->thermal_zone,
THERMAL_EVENT_UNSPECIFIED);
}
/* sys I/F for generic thermal sysfs support */ /* sys I/F for generic thermal sysfs support */
static int thermal_get_temp(struct thermal_zone_device *thermal, int *temp) static int thermal_get_temp(struct thermal_zone_device *thermal, int *temp)
@ -900,6 +894,12 @@ static void acpi_thermal_unregister_thermal_zone(struct acpi_thermal *tz)
Driver Interface Driver Interface
-------------------------------------------------------------------------- */ -------------------------------------------------------------------------- */
static void acpi_queue_thermal_check(struct acpi_thermal *tz)
{
if (!work_pending(&tz->thermal_check_work))
queue_work(acpi_thermal_pm_queue, &tz->thermal_check_work);
}
static void acpi_thermal_notify(struct acpi_device *device, u32 event) static void acpi_thermal_notify(struct acpi_device *device, u32 event)
{ {
struct acpi_thermal *tz = acpi_driver_data(device); struct acpi_thermal *tz = acpi_driver_data(device);
@ -910,17 +910,17 @@ static void acpi_thermal_notify(struct acpi_device *device, u32 event)
switch (event) { switch (event) {
case ACPI_THERMAL_NOTIFY_TEMPERATURE: case ACPI_THERMAL_NOTIFY_TEMPERATURE:
acpi_thermal_check(tz); acpi_queue_thermal_check(tz);
break; break;
case ACPI_THERMAL_NOTIFY_THRESHOLDS: case ACPI_THERMAL_NOTIFY_THRESHOLDS:
acpi_thermal_trips_update(tz, ACPI_TRIPS_REFRESH_THRESHOLDS); acpi_thermal_trips_update(tz, ACPI_TRIPS_REFRESH_THRESHOLDS);
acpi_thermal_check(tz); acpi_queue_thermal_check(tz);
acpi_bus_generate_netlink_event(device->pnp.device_class, acpi_bus_generate_netlink_event(device->pnp.device_class,
dev_name(&device->dev), event, 0); dev_name(&device->dev), event, 0);
break; break;
case ACPI_THERMAL_NOTIFY_DEVICES: case ACPI_THERMAL_NOTIFY_DEVICES:
acpi_thermal_trips_update(tz, ACPI_TRIPS_REFRESH_DEVICES); acpi_thermal_trips_update(tz, ACPI_TRIPS_REFRESH_DEVICES);
acpi_thermal_check(tz); acpi_queue_thermal_check(tz);
acpi_bus_generate_netlink_event(device->pnp.device_class, acpi_bus_generate_netlink_event(device->pnp.device_class,
dev_name(&device->dev), event, 0); dev_name(&device->dev), event, 0);
break; break;
@ -1020,7 +1020,25 @@ static void acpi_thermal_check_fn(struct work_struct *work)
{ {
struct acpi_thermal *tz = container_of(work, struct acpi_thermal, struct acpi_thermal *tz = container_of(work, struct acpi_thermal,
thermal_check_work); thermal_check_work);
acpi_thermal_check(tz);
/*
* In general, it is not sufficient to check the pending bit, because
* subsequent instances of this function may be queued after one of them
* has started running (e.g. if _TMP sleeps). Avoid bailing out if just
* one of them is running, though, because it may have done the actual
* check some time ago, so allow at least one of them to block on the
* mutex while another one is running the update.
*/
if (!refcount_dec_not_one(&tz->thermal_check_count))
return;
mutex_lock(&tz->thermal_check_lock);
thermal_zone_device_update(tz->thermal_zone, THERMAL_EVENT_UNSPECIFIED);
refcount_inc(&tz->thermal_check_count);
mutex_unlock(&tz->thermal_check_lock);
} }
static int acpi_thermal_add(struct acpi_device *device) static int acpi_thermal_add(struct acpi_device *device)
@ -1052,6 +1070,8 @@ static int acpi_thermal_add(struct acpi_device *device)
if (result) if (result)
goto free_memory; goto free_memory;
refcount_set(&tz->thermal_check_count, 3);
mutex_init(&tz->thermal_check_lock);
INIT_WORK(&tz->thermal_check_work, acpi_thermal_check_fn); INIT_WORK(&tz->thermal_check_work, acpi_thermal_check_fn);
pr_info(PREFIX "%s [%s] (%ld C)\n", acpi_device_name(device), pr_info(PREFIX "%s [%s] (%ld C)\n", acpi_device_name(device),
@ -1117,7 +1137,7 @@ static int acpi_thermal_resume(struct device *dev)
tz->state.active |= tz->trips.active[i].flags.enabled; tz->state.active |= tz->trips.active[i].flags.enabled;
} }
queue_work(acpi_thermal_pm_queue, &tz->thermal_check_work); acpi_queue_thermal_check(tz);
return AE_OK; return AE_OK;
} }