Re: [PATCH 6.18 134/175] hwmon: (acpi_power_meter) Fix deadlocks related to acpi_power_meter_notify()

Wysocki, Rafael J
2026-02-10T16:26:49Z

On 2/10/2026 12:19 PM, Greg Kroah-Hartman wrote:
> On Tue, Feb 10, 2026 at 11:19:12AM +0100, Jaroslav Pulchart wrote:
>>> 6.18-stable review patch. If anyone has any objections, please let me know.
>>>
>>> ------------------
>>>
>>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>>
>>> [ Upstream commit 615901b57b7ef8eb655f71358f7e956e42bcd16b ]
>>>
>>> The acpi_power_meter driver's .notify() callback function,
>>> acpi_power_meter_notify(), calls hwmon_device_unregister() under a lock
>>> that is also acquired by callbacks in sysfs attributes of the device
>>> being unregistered which is prone to deadlocks between sysfs access and
>>> device removal.
>>>
>>> Address this by moving the hwmon device removal in
>>> acpi_power_meter_notify() outside the lock in question, but notice
>>> that doing it alone is not sufficient because two concurrent
>>> METER_NOTIFY_CONFIG notifications may be attempting to remove the
>>> same device at the same time. To prevent that from happening, add a
>>> new lock serializing the execution of the switch () statement in
>>> acpi_power_meter_notify(). For simplicity, it is a static mutex
>>> which should not be a problem from the performance perspective.
>>>
>>> The new lock also allows the hwmon_device_register_with_info()
>>> in acpi_power_meter_notify() to be called outside the inner lock
>>> because it prevents the other notifications handled by that function
>>> from manipulating the "resource" object while the hwmon device based
>>> on it is being registered. The sending of ACPI netlink messages from
>>> acpi_power_meter_notify() is serialized by the new lock too which
>>> generally helps to ensure that the order of handling firmware
>>> notifications is the same as the order of sending netlink messages
>>> related to them.
>>>
>>> In addition, notice that hwmon_device_register_with_info() may fail
>>> in which case resource->hwmon_dev will become an error pointer,
>>> so add checks to avoid attempting to unregister the hwmon device
>>> pointer to by it in that case to acpi_power_meter_notify() and
>>> acpi_power_meter_remove().
>>>
>>> Fixes: 16746ce8adfe ("hwmon: (acpi_power_meter) Replace the deprecated hwmon_device_register")
>>> Closes: https://lore.kernel.org/linux-hwmon/CAK8fFZ58fidGUCHi5WFX0uoTPzveUUDzT=k=AAm4yWo3bAuCFg@mail.gmail.com/
>>> Reported-by: Jaroslav Pulchart <jaroslav.pulchart@gooddata.com>
>>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>>> Signed-off-by: Sasha Levin <sashal@kernel.org>
>>> ---
>>> drivers/hwmon/acpi_power_meter.c | 17 ++++++++++++++---
>>> 1 file changed, 14 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/hwmon/acpi_power_meter.c b/drivers/hwmon/acpi_power_meter.c
>>> index 29ccdc2fb7ff8..de408df0c4d78 100644
>>> --- a/drivers/hwmon/acpi_power_meter.c
>>> +++ b/drivers/hwmon/acpi_power_meter.c
>>> @@ -47,6 +47,8 @@
>>> static int cap_in_hardware;
>>> static bool force_cap_on;
>>>
>>> +static DEFINE_MUTEX(acpi_notify_lock);
>>> +
>>> static int can_cap_in_hardware(void)
>>> {
>>> return force_cap_on || cap_in_hardware;
>>> @@ -823,18 +825,26 @@ static void acpi_power_meter_notify(struct acpi_device *device, u32 event)
>>>
>>> resource = acpi_driver_data(device);
>>>
>>> + guard(mutex)(&acpi_notify_lock);
>>> +
>>> switch (event) {
>>> case METER_NOTIFY_CONFIG:
>>> + if (!IS_ERR(resource->hwmon_dev))
>>> + hwmon_device_unregister(resource->hwmon_dev);
>>> +
>>> mutex_lock(&resource->lock);
>>> +
>>> free_capabilities(resource);
>>> remove_domain_devices(resource);
>>> - hwmon_device_unregister(resource->hwmon_dev);
>>> res = read_capabilities(resource);
>>> if (res)
>>> dev_err_once(&device->dev, "read capabilities failed.\n");
>>> res = read_domain_devices(resource);
>>> if (res && res != -ENODEV)
>>> dev_err_once(&device->dev, "read domain devices failed.\n");
>>> +
>>> + mutex_unlock(&resource->lock);
>>> +
>>> resource->hwmon_dev =
>>> hwmon_device_register_with_info(&device->dev,
>>> ACPI_POWER_METER_NAME,
>>> @@ -843,7 +853,7 @@ static void acpi_power_meter_notify(struct acpi_device *device, u32 event)
>>> power_extra_groups);
>>> if (IS_ERR(resource->hwmon_dev))
>>> dev_err_once(&device->dev, "register hwmon device failed.\n");
>>> - mutex_unlock(&resource->lock);
>>> +
>>> break;
>>> case METER_NOTIFY_TRIP:
>>> sysfs_notify(&device->dev.kobj, NULL, POWER_AVERAGE_NAME);
>>> @@ -953,7 +963,8 @@ static void acpi_power_meter_remove(struct acpi_device *device)
>>> return;
>>>
>>> resource = acpi_driver_data(device);
>>> - hwmon_device_unregister(resource->hwmon_dev);
>>> + if (!IS_ERR(resource->hwmon_dev))
>>> + hwmon_device_unregister(resource->hwmon_dev);
>>>
>>> remove_domain_devices(resource);
>>> free_capabilities(resource);
>>> --
>>> 2.51.0
>>>
>>>
>>>
>> Hello, I tested this patch, but unfortunately it does not resolve the
>> reported issue on our systems the deadlock is still reproducible with
>> the same iDRAC reset reproducer.
> Is 6.19 also a problem here, or does it work properly on that release?

To be precise, the patch does fix a problem, but it is not sufficient to
make the reported observed symptoms go away, so the Closes: tag of it is
inaccurate.

An IPMI fix is needed in addition to it to really close the bug report,