On Fri, Jun 5, 2020 at 7:09 PM Dan Williams dan.j.williams@intel.com wrote:
On Fri, Jun 5, 2020 at 7:06 AM Rafael J. Wysocki rjw@rjwysocki.net wrote:
From: Rafael J. Wysocki rafael.j.wysocki@intel.com Subject: [PATCH] ACPI: OSL: Use rwlock instead of RCU for memory management
The ACPI OS layer uses RCU to protect the list of ACPI memory mappings from being walked while it is updated. Among other situations, that list can be walked in non-NMI interrupt context, so using a sleeping lock to protect it is not an option.
However, performance issues related to the RCU usage in there appear, as described by Dan Williams:
"Recently a performance problem was reported for a process invoking a non-trival ASL program. The method call in this case ends up repetitively triggering a call path like:
acpi_ex_store acpi_ex_store_object_to_node acpi_ex_write_data_to_field acpi_ex_insert_into_field acpi_ex_write_with_update_rule acpi_ex_field_datum_io acpi_ex_access_region acpi_ev_address_space_dispatch acpi_ex_system_memory_space_handler acpi_os_map_cleanup.part.14 _synchronize_rcu_expedited.constprop.89 scheduleThe end result of frequent synchronize_rcu_expedited() invocation is tiny sub-millisecond spurts of execution where the scheduler freely migrates this apparently sleepy task. The overhead of frequent scheduler invocation multiplies the execution time by a factor of 2-3X."
In order to avoid these issues, replace the RCU in the ACPI OS layer by an rwlock.
That rwlock should not be frequently contended, so the performance impact of it is not expected to be significant.
Reported-by: Dan Williams dan.j.williams@intel.com Signed-off-by: Rafael J. Wysocki rafael.j.wysocki@intel.com
Hi Dan,
This is a possible fix for the ACPI OSL RCU-related performance issues, but can you please arrange for the testing of it on the affected systems?
Ugh, is it really this simple? I did not realize the read-side is NMI safe. I'll take a look.
But if an NMI triggers while the lock is being held for writing, it will deadlock, won't it?
OTOH, according to the RCU documentation it is valid to call rcu_read_[un]lock() from an NMI handler (see Interrupts and NMIs in Documentation/RCU/Design/Requirements/Requirements.rst) so we are good from this perspective today.
Unless we teach APEI to avoid mapping lookups from apei_{read|write}(), which wouldn't be unreasonable by itself, we need to hold on to the RCU in ACPI OSL, so it looks like addressing the problem in ACPICA is the best way to do it (and the current ACPICA code in question is suboptimal, so it would be good to rework it anyway).
Cheers!