[Intel-gfx] [PATCH] ACPI / PM: Fix acpi_pm_notifier_lock vs. flush_workqueue() deadlock

[Rafael J. Wysocki]

On Tue, Nov 7, 2017 at 11:47 PM, Rafael J. Wysocki <rafael at kernel.org> wrote:
> On Tue, Nov 7, 2017 at 10:08 PM, Ville Syrjala
> <ville.syrjala at linux.intel.com> wrote:
>> From: Ville Syrjälä <ville.syrjala at 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 at vger.kernel.org
>> Cc: Rafael J. Wysocki <rafael.j.wysocki at intel.com>
>> Cc: Len Brown <lenb at kernel.org>
>> Cc: Peter Zijlstra <peterz at infradead.org>
>> Cc: Tejun Heo <tj at kernel.org>
>> Cc: Ingo Molnar <mingo at kernel.org>
>> Fixes: c072530f391e ("ACPI / PM: Revork the handling of ACPI device wakeup notifications")
>> Signed-off-by: Ville Syrjälä <ville.syrjala at 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