On 09/14/2012 06:26 AM, Rafael J. Wysocki wrote:
> On Wednesday, September 12, 2012, Aaron Lu wrote:
>> Commit 0090def("ACPI: Add interface to register/unregister device
>> to/from power resources") used resource_lock to protect the devices list
>> that relies on power resource. It caused a mutex dead lock, as below
>>
>> acpi_power_on ---> lock resource_lock
>> __acpi_power_on
>> acpi_power_on_device
>> acpi_power_get_inferred_state
>> acpi_power_get_list_state ---> lock resource_lock
>>
>> This patch adds a new mutex "devices_lock" to protect the devices list
>> and calls acpi_power_on_device in acpi_power_on, instead of
>> __acpi_power_on, after the resource_lock is released.
>>
>> Signed-off-by: Lin Ming <ming.m.lin@xxxxxxxxx>
>> Signed-off-by: Aaron Lu <aaron.lu@xxxxxxxxx>
>
> Good catch, thanks.
>
> I hope Len won't mind if I take it for v3.6.
>
Yes, that would be good.
And the commit enters v3.4, so a stable tag may need be added.
>> ---
>> v2:
>> If power resource is already on, no need to check if device needs
>> to be resumed.
>> v1:
>> By Lin Ming.
>>
>> drivers/acpi/power.c | 36 ++++++++++++++++++++++++------------
>> 1 file changed, 24 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/acpi/power.c b/drivers/acpi/power.c
>> index 215ecd0..3582a26 100644
>> --- a/drivers/acpi/power.c
>> +++ b/drivers/acpi/power.c
>> @@ -105,6 +105,7 @@ struct acpi_power_resource {
>>
>> /* List of devices relying on this power resource */
>> struct acpi_power_resource_device *devices;
>> + struct mutex devices_lock;
>> };
>>
>> static struct list_head acpi_power_resource_list;
>> @@ -223,7 +224,6 @@ static void acpi_power_on_device(struct acpi_power_managed_device *device)
>>
>> static int __acpi_power_on(struct acpi_power_resource *resource)
>> {
>> - struct acpi_power_resource_device *device_list = resource->devices;
>> acpi_status status = AE_OK;
>>
>> status = acpi_evaluate_object(resource->device->handle, "_ON", NULL, NULL);
>> @@ -236,19 +236,14 @@ static int __acpi_power_on(struct acpi_power_resource *resource)
>> ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Power resource [%s] turned on\n",
>> resource->name));
>>
>> - while (device_list) {
>> - acpi_power_on_device(device_list->device);
>> -
>> - device_list = device_list->next;
>> - }
>> -
>> return 0;
>> }
>>
>> static int acpi_power_on(acpi_handle handle)
>> {
>> - int result = 0;
>> + int result = 0, resume_device = 0;
>
> I'll change the data type of resume_device to bool when applying the patch.
>
Sure, thanks.
-Aaron
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
