Skip to content

Commit

Permalink
Tables: Fix a hidden logic related to AcpiTbInstallStandardTable()
Browse files Browse the repository at this point in the history
There is a hidden logic for AcpiTbInstallStandardTable():
1. When it is invoked from the OS boot stage, ACPICA mutex may not be
   available, and thus no AcpiUtAcquireMutex()/AcpiUtReleaseMutex() are
   invoked in these code paths:
   AcpiInitializeTables
     AcpiTbParseRootTable
       AcpiTbInstallStandardTable (4 invocations)
   AcpiInstallTable
       AcpiTbInstallStandardTable
2. When it is invoked during the runtime, ACPICA mutex is correctly used:
   AcpiExLoadOp
     AcpiTbInstallAndLoadTable
       Lock(TABLES)
       AcpiTbInstallStandardTable
       UnLock(TABLES)
   AcpiLoadTable
     AcpiTbInstallAndLoadTable
       Lock(TABLES)
       AcpiTbInstallStandardTable
       UnLock(TABLES)
So the hidden logic is: AcpiTbInstallStandardTable() tries not to hold
mutexes to handle the difference of the boot stage and the runtime, but
leaves the mutexes held from some calling runtime APIs.

This introduces another problem in AcpiTbInstallStandardTable() where
AcpiGbl_TableHandler is invoked from and the lock contexts are thus not
consistent for the table handlers. Linux registers such a handler to track
the table installation and the handler is actually invoked from both
contexts. Since the handler also invokes AcpiGetTable(), when the following
commit corrected AcpiGetTable() to make it start to hold the table lock,
a regression is then triggered.
  Commit: cac6790
  Subject: Tables: Back port acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel

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 dead lock:
[  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 held in AcpiTbInstallAndLoadTable() is re-visited by
AcpiGetTable().

Noticing that the early mutex requirement actually belongs to the OSL layer
and has already been handled in Linux
acpi_os_wait_semaphore()/acpi_os_signal_semaphore(). This patch then can
fix the regression by removing this hidden logic from ACPICA core and
leaving it to OSPMs.

Tested-by: Tomi Sarvela <tomi.p.sarvela@intel.com>
Tested-by: Ye Xiaolong <xiaolong.ye@intel.com>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
  • Loading branch information
Lv Zheng committed Jan 23, 2017
1 parent 45ed82d commit 09d9d7f
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 9 deletions.
9 changes: 2 additions & 7 deletions source/components/tables/tbdata.c
Original file line number Diff line number Diff line change
Expand Up @@ -1034,24 +1034,19 @@ AcpiTbInstallAndLoadTable (
ACPI_FUNCTION_TRACE (TbInstallAndLoadTable);


(void) AcpiUtAcquireMutex (ACPI_MTX_TABLES);

/* Install the table and load it into the namespace */

Status = AcpiTbInstallStandardTable (Address, Flags, TRUE,
Override, &i);
if (ACPI_FAILURE (Status))
{
goto UnlockAndExit;
goto Exit;
}

(void) AcpiUtReleaseMutex (ACPI_MTX_TABLES);
Status = AcpiTbLoadTable (i, AcpiGbl_RootNode);
(void) AcpiUtAcquireMutex (ACPI_MTX_TABLES);

UnlockAndExit:
Exit:
*TableIndex = i;
(void) AcpiUtReleaseMutex (ACPI_MTX_TABLES);
return_ACPI_STATUS (Status);
}

Expand Down
17 changes: 15 additions & 2 deletions source/components/tables/tbinstal.c
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,10 @@ AcpiTbInstallStandardTable (
goto ReleaseAndExit;
}

/* Acquire the table lock */

(void) AcpiUtAcquireMutex (ACPI_MTX_TABLES);

if (Reload)
{
/*
Expand All @@ -334,7 +338,7 @@ AcpiTbInstallStandardTable (
NewTableDesc.Signature.Integer));

Status = AE_BAD_SIGNATURE;
goto ReleaseAndExit;
goto UnlockAndExit;
}

/* Check if table is already registered */
Expand Down Expand Up @@ -370,7 +374,7 @@ AcpiTbInstallStandardTable (
/* Table is still loaded, this is an error */

Status = AE_ALREADY_EXISTS;
goto ReleaseAndExit;
goto UnlockAndExit;
}
else
{
Expand All @@ -383,6 +387,7 @@ AcpiTbInstallStandardTable (
* indicate the re-installation.
*/
AcpiTbUninstallTable (&NewTableDesc);
(void) AcpiUtReleaseMutex (ACPI_MTX_TABLES);
*TableIndex = i;
return_ACPI_STATUS (AE_OK);
}
Expand All @@ -395,11 +400,19 @@ AcpiTbInstallStandardTable (

/* Invoke table handler if present */

(void) AcpiUtReleaseMutex (ACPI_MTX_TABLES);
if (AcpiGbl_TableHandler)
{
(void) AcpiGbl_TableHandler (ACPI_TABLE_EVENT_INSTALL,
NewTableDesc.Pointer, AcpiGbl_TableHandlerContext);
}
(void) AcpiUtAcquireMutex (ACPI_MTX_TABLES);

UnlockAndExit:

/* Release the table lock */

(void) AcpiUtReleaseMutex (ACPI_MTX_TABLES);

ReleaseAndExit:

Expand Down

0 comments on commit 09d9d7f

Please sign in to comment.