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

Greg Kroah-Hartman
2026-02-10T11:19:30Z

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?

thanks,

greg k-h