ACPICA: Tables: Fix hidden logic related to acpi_tb_install_standard_table()
authorLv Zheng <lv.zheng@intel.com>
Thu, 19 Jan 2017 07:21:34 +0000 (15:21 +0800)
committerRafael J. Wysocki <rafael.j.wysocki@intel.com>
Fri, 20 Jan 2017 02:44:58 +0000 (03:44 +0100)
There is a hidden logic for acpi_tb_install_standard_table() as it can be
invoked from the boot stage and during runtime.

 1. When it is invoked from the OS boot stage, the ACPICA mutex may not have
    been initialized yet and so acpi_ut_acquire_mutex()/acpi_ut_release_mutex()
    are not invoked in these code paths:

   acpi_initialize_tables
     acpi_tb_parse_root_table
       acpi_tb_install_standard_table (4 invocations)
   acpi_install_table
       acpi_tb_install_standard_table

 2. When it is invoked during the runtime, ACPICA mutex is used as
    appropriate:

   acpi_ex_load_op
     acpi_tb_install_and_load_table
       acpi_tb_install_standard_table
   acpi_load_table
     acpi_tb_install_and_load_table
       acpi_tb_install_standard_table

The mutex is now used in acpi_tb_install_and_load_table(), while it actually
should be in acpi_tb_install_standard_table().

This introduces another problem in acpi_tb_install_standard_table() where
acpi_gbl_table_handler is invoked from and the lock contexts are thus not
consistent for the table handlers. This triggers a regression when
acpi_get_table()/acpi_put_table() start to hold table mutex during runtime.

The regression is noticed by LKP as new errors reported by ACPICA mutex
debugging facility.

[    2.043693] ACPI Error: Mutex [ACPI_MTX_Tables] already acquired by this thread [497483776] (20160930/utmutex-254)
[    2.054084] ACPI Error: Mutex [0x2] is not acquired, cannot release (20160930/utmutex-326)

And it triggers a deadlock:

[  247.066214] INFO: task swapper/0:1 blocked for more than 120 seconds.
...
[  247.091271] Call Trace:
...
[  247.121523]  down_timeout+0x47/0x50
[  247.125065]  acpi_os_wait_semaphore+0x47/0x62
[  247.129475]  acpi_ut_acquire_mutex+0x43/0x81
[  247.133798]  acpi_get_table+0x2d/0x84
[  247.137513]  acpi_table_attr_init+0xcd/0x100
[  247.146590]  acpi_sysfs_table_handler+0x5d/0xb8
[  247.151174]  acpi_bus_table_handler+0x23/0x2a
[  247.155583]  acpi_tb_install_standard_table+0xe0/0x213
[  247.164489]  acpi_tb_install_and_load_table+0x3a/0x82
[  247.169592]  acpi_ex_load_op+0x194/0x201
...
[  247.200108]  acpi_ns_evaluate+0x1bb/0x247
[  247.204170]  acpi_evaluate_object+0x178/0x274
[  247.213249]  acpi_processor_set_pdc+0x154/0x17b
...
The table mutex is held in acpi_tb_install_and_load_table() and is re-visited by
acpi_get_table().

Noticing that the early mutex requirement actually belongs to the OSL layer
and has already been handled in acpi_os_wait_semaphore()/acpi_os_signal_semaphore(),
the regression canbe fixed by removing this hidden logic from the ACPICA core
to the OS-specific code.

Fixes: 174cc7187e6f ("ACPICA: Tables: Back port acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel")
Reported-and-tested-by: Tomi Sarvela <tomi.p.sarvela@intel.com>
Reported-by: Ye Xiaolong <xiaolong.ye@intel.com>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
drivers/acpi/acpica/tbdata.c
drivers/acpi/acpica/tbinstal.c

index 82b0b571097960919ce6ed36a703402422ac3cc7..b0399e8f6d27df774b175cb2a3aa7c8f3cce7189 100644 (file)
@@ -852,23 +852,18 @@ acpi_tb_install_and_load_table(acpi_physical_address address,
 
        ACPI_FUNCTION_TRACE(tb_install_and_load_table);
 
-       (void)acpi_ut_acquire_mutex(ACPI_MTX_TABLES);
-
        /* Install the table and load it into the namespace */
 
        status = acpi_tb_install_standard_table(address, flags, TRUE,
                                                override, &i);
        if (ACPI_FAILURE(status)) {
-               goto unlock_and_exit;
+               goto exit;
        }
 
-       (void)acpi_ut_release_mutex(ACPI_MTX_TABLES);
        status = acpi_tb_load_table(i, acpi_gbl_root_node);
-       (void)acpi_ut_acquire_mutex(ACPI_MTX_TABLES);
 
-unlock_and_exit:
+exit:
        *table_index = i;
-       (void)acpi_ut_release_mutex(ACPI_MTX_TABLES);
        return_ACPI_STATUS(status);
 }
 
index 5fdf251a9f9797a2a00f479880d6449dfa6dd940..01e1b3d63fc0dc8ae0e0b17767dae535be2bec68 100644 (file)
@@ -217,6 +217,10 @@ acpi_tb_install_standard_table(acpi_physical_address address,
                goto release_and_exit;
        }
 
+       /* Acquire the table lock */
+
+       (void)acpi_ut_acquire_mutex(ACPI_MTX_TABLES);
+
        if (reload) {
                /*
                 * Validate the incoming table signature.
@@ -244,7 +248,7 @@ acpi_tb_install_standard_table(acpi_physical_address address,
                                         new_table_desc.signature.integer));
 
                        status = AE_BAD_SIGNATURE;
-                       goto release_and_exit;
+                       goto unlock_and_exit;
                }
 
                /* Check if table is already registered */
@@ -279,7 +283,7 @@ acpi_tb_install_standard_table(acpi_physical_address address,
                                /* Table is still loaded, this is an error */
 
                                status = AE_ALREADY_EXISTS;
-                               goto release_and_exit;
+                               goto unlock_and_exit;
                        } else {
                                /*
                                 * Table was unloaded, allow it to be reloaded.
@@ -290,6 +294,7 @@ acpi_tb_install_standard_table(acpi_physical_address address,
                                 * indicate the re-installation.
                                 */
                                acpi_tb_uninstall_table(&new_table_desc);
+                               (void)acpi_ut_release_mutex(ACPI_MTX_TABLES);
                                *table_index = i;
                                return_ACPI_STATUS(AE_OK);
                        }
@@ -303,11 +308,19 @@ acpi_tb_install_standard_table(acpi_physical_address address,
 
        /* Invoke table handler if present */
 
+       (void)acpi_ut_release_mutex(ACPI_MTX_TABLES);
        if (acpi_gbl_table_handler) {
                (void)acpi_gbl_table_handler(ACPI_TABLE_EVENT_INSTALL,
                                             new_table_desc.pointer,
                                             acpi_gbl_table_handler_context);
        }
+       (void)acpi_ut_acquire_mutex(ACPI_MTX_TABLES);
+
+unlock_and_exit:
+
+       /* Release the table lock */
+
+       (void)acpi_ut_release_mutex(ACPI_MTX_TABLES);
 
 release_and_exit: