On Tue, Nov 7, 2017 at 11:47 PM, Rafael J. Wysocki rafael@kernel.org wrote:

On Tue, Nov 7, 2017 at 10:08 PM, Ville Syrjala ville.syrjala@linux.intel.com wrote:

...

From: Ville Syrjälä ville.syrjala@linux.intel.com

acpi_remove_pm_notifier() ends up calling flush_workqueue() while holding acpi_pm_notifier_lock, and that same lock is taken by by the work via acpi_pm_notify_handler(). This can deadlock.

OK, good catch!

[cut]

...

Cc: stable@vger.kernel.org Cc: Rafael J. Wysocki rafael.j.wysocki@intel.com Cc: Len Brown lenb@kernel.org Cc: Peter Zijlstra peterz@infradead.org Cc: Tejun Heo tj@kernel.org Cc: Ingo Molnar mingo@kernel.org Fixes: c072530f391e ("ACPI / PM: Revork the handling of ACPI device wakeup notifications") Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com

---

drivers/acpi/device_pm.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/acpi/device_pm.c b/drivers/acpi/device_pm.c index fbcc73f7a099..18af71057b44 100644 --- a/drivers/acpi/device_pm.c +++ b/drivers/acpi/device_pm.c @@ -387,6 +387,7 @@ EXPORT_SYMBOL(acpi_bus_power_manageable);

#ifdef CONFIG_PM static DEFINE_MUTEX(acpi_pm_notifier_lock); +static DEFINE_MUTEX(acpi_pm_notifier_install_lock);

void acpi_pm_wakeup_event(struct device *dev) { @@ -443,24 +444,25 @@ acpi_status acpi_add_pm_notifier(struct acpi_device *adev, struct device *dev, if (!dev && !func) return AE_BAD_PARAMETER;

•   mutex_lock(&acpi_pm_notifier_lock);
  

•   mutex_lock(&acpi_pm_notifier_install_lock);
  
    if (adev->wakeup.flags.notifier_present)
            goto out;
  
  

•   adev->wakeup.ws = wakeup_source_register(dev_name(&adev->dev));
  

•   adev->wakeup.context.dev = dev;
  

•   adev->wakeup.context.func = func;
  

But this doesn't look good to me.

notifier_present should be checked under acpi_pm_notifier_lock.

Well, not really, so the above is OK.

However, I still would prefer to avoid adding the outer lock.

Thanks, Rafael