Skip to content

Commit 1757659

Browse files
committed
ACPI: OSL: Implement deferred unmapping of ACPI memory
The ACPI OS layer in Linux uses RCU to protect the walkers of the list of ACPI memory mappings from seeing an inconsistent state while it is being updated. Among other situations, that list can be walked in (NMI and 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 schedule The 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." The source of this is that acpi_ex_system_memory_space_handler() unmaps the memory mapping currently cached by it at the access time if that mapping doesn't cover the memory area being accessed. Consequently, if there is a memory opregion with two fields separated from each other by an unused chunk of address space that is large enough for not being covered by a single mapping, and they happen to be used in an alternating pattern, the unmapping will occur on every acpi_ex_system_memory_space_handler() invocation for that memory opregion and that will lead to significant overhead. Moreover, acpi_ex_system_memory_space_handler() carries out the memory unmapping with the namespace and interpreter mutexes held which may lead to additional latency, because all of the tasks wanting to acquire on of these mutexes need to wait for the memory unmapping operation to complete. To address that, rework acpi_os_unmap_memory() so that it does not release the memory mapping covering the given address range right away and instead make it queue up the mapping at hand for removal via queue_rcu_work(). Reported-by: Dan Williams <[email protected]> Tested-by: Xiang Li <[email protected]> Signed-off-by: Rafael J. Wysocki <[email protected]>
1 parent 9ebcfad commit 1757659

File tree

1 file changed

+77
-35
lines changed

1 file changed

+77
-35
lines changed

drivers/acpi/osl.c

Lines changed: 77 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,10 @@ struct acpi_ioremap {
7777
void __iomem *virt;
7878
acpi_physical_address phys;
7979
acpi_size size;
80-
unsigned long refcount;
80+
union {
81+
unsigned long refcount;
82+
struct rcu_work rwork;
83+
} track;
8184
};
8285

8386
static LIST_HEAD(acpi_ioremaps);
@@ -250,7 +253,7 @@ void __iomem *acpi_os_get_iomem(acpi_physical_address phys, unsigned int size)
250253
map = acpi_map_lookup(phys, size);
251254
if (map) {
252255
virt = map->virt + (phys - map->phys);
253-
map->refcount++;
256+
map->track.refcount++;
254257
}
255258
mutex_unlock(&acpi_ioremap_lock);
256259
return virt;
@@ -335,7 +338,7 @@ void __iomem __ref
335338
/* Check if there's a suitable mapping already. */
336339
map = acpi_map_lookup(phys, size);
337340
if (map) {
338-
map->refcount++;
341+
map->track.refcount++;
339342
goto out;
340343
}
341344

@@ -358,7 +361,7 @@ void __iomem __ref
358361
map->virt = virt;
359362
map->phys = pg_off;
360363
map->size = pg_sz;
361-
map->refcount = 1;
364+
map->track.refcount = 1;
362365

363366
list_add_tail_rcu(&map->list, &acpi_ioremaps);
364367

@@ -374,65 +377,103 @@ void *__ref acpi_os_map_memory(acpi_physical_address phys, acpi_size size)
374377
}
375378
EXPORT_SYMBOL_GPL(acpi_os_map_memory);
376379

380+
static void acpi_os_map_remove(struct acpi_ioremap *map)
381+
{
382+
acpi_unmap(map->phys, map->virt);
383+
kfree(map);
384+
}
385+
386+
static void acpi_os_map_cleanup_deferred(struct work_struct *work)
387+
{
388+
acpi_os_map_remove(container_of(to_rcu_work(work), struct acpi_ioremap,
389+
track.rwork));
390+
}
391+
377392
/* Must be called with mutex_lock(&acpi_ioremap_lock) */
378-
static unsigned long acpi_os_drop_map_ref(struct acpi_ioremap *map)
393+
static bool acpi_os_drop_map_ref(struct acpi_ioremap *map, bool defer)
379394
{
380-
unsigned long refcount = --map->refcount;
395+
if (--map->track.refcount)
396+
return true;
381397

382-
if (!refcount)
383-
list_del_rcu(&map->list);
384-
return refcount;
398+
list_del_rcu(&map->list);
399+
400+
if (defer) {
401+
INIT_RCU_WORK(&map->track.rwork, acpi_os_map_cleanup_deferred);
402+
queue_rcu_work(system_wq, &map->track.rwork);
403+
}
404+
return defer;
385405
}
386406

387407
static void acpi_os_map_cleanup(struct acpi_ioremap *map)
388408
{
409+
if (!map)
410+
return;
411+
389412
synchronize_rcu_expedited();
390-
acpi_unmap(map->phys, map->virt);
391-
kfree(map);
413+
acpi_os_map_remove(map);
392414
}
393415

394-
/**
395-
* acpi_os_unmap_iomem - Drop a memory mapping reference.
396-
* @virt: Start of the address range to drop a reference to.
397-
* @size: Size of the address range to drop a reference to.
398-
*
399-
* Look up the given virtual address range in the list of existing ACPI memory
400-
* mappings, drop a reference to it and unmap it if there are no more active
401-
* references to it.
402-
*
403-
* During early init (when acpi_permanent_mmap has not been set yet) this
404-
* routine simply calls __acpi_unmap_table() to get the job done. Since
405-
* __acpi_unmap_table() is an __init function, the __ref annotation is needed
406-
* here.
407-
*/
408-
void __ref acpi_os_unmap_iomem(void __iomem *virt, acpi_size size)
416+
static void __ref __acpi_os_unmap_iomem(void __iomem *virt, acpi_size size,
417+
bool defer)
409418
{
410419
struct acpi_ioremap *map;
411-
unsigned long refcount;
412420

413421
if (!acpi_permanent_mmap) {
414422
__acpi_unmap_table(virt, size);
415423
return;
416424
}
417425

418426
mutex_lock(&acpi_ioremap_lock);
427+
419428
map = acpi_map_lookup_virt(virt, size);
420429
if (!map) {
421430
mutex_unlock(&acpi_ioremap_lock);
422431
WARN(true, PREFIX "%s: bad address %p\n", __func__, virt);
423432
return;
424433
}
425-
refcount = acpi_os_drop_map_ref(map);
434+
if (acpi_os_drop_map_ref(map, defer))
435+
map = NULL;
436+
426437
mutex_unlock(&acpi_ioremap_lock);
427438

428-
if (!refcount)
429-
acpi_os_map_cleanup(map);
439+
acpi_os_map_cleanup(map);
440+
}
441+
442+
/**
443+
* acpi_os_unmap_iomem - Drop a memory mapping reference.
444+
* @virt: Start of the address range to drop a reference to.
445+
* @size: Size of the address range to drop a reference to.
446+
*
447+
* Look up the given virtual address range in the list of existing ACPI memory
448+
* mappings, drop a reference to it and unmap it if there are no more active
449+
* references to it.
450+
*
451+
* During early init (when acpi_permanent_mmap has not been set yet) this
452+
* routine simply calls __acpi_unmap_table() to get the job done. Since
453+
* __acpi_unmap_table() is an __init function, the __ref annotation is needed
454+
* here.
455+
*/
456+
void __ref acpi_os_unmap_iomem(void __iomem *virt, acpi_size size)
457+
{
458+
__acpi_os_unmap_iomem(virt, size, false);
430459
}
431460
EXPORT_SYMBOL_GPL(acpi_os_unmap_iomem);
432461

462+
/**
463+
* acpi_os_unmap_memory - Drop a memory mapping reference.
464+
* @virt: Start of the address range to drop a reference to.
465+
* @size: Size of the address range to drop a reference to.
466+
*
467+
* Look up the given virtual address range in the list of existing ACPI memory
468+
* mappings, drop a reference to it and if there are no more active references
469+
* to it, queue it up for later removal.
470+
*
471+
* During early init (when acpi_permanent_mmap has not been set yet) this
472+
* routine behaves like acpi_os_unmap_iomem().
473+
*/
433474
void __ref acpi_os_unmap_memory(void *virt, acpi_size size)
434475
{
435-
return acpi_os_unmap_iomem((void __iomem *)virt, size);
476+
__acpi_os_unmap_iomem((void __iomem *)virt, size, true);
436477
}
437478
EXPORT_SYMBOL_GPL(acpi_os_unmap_memory);
438479

@@ -461,7 +502,6 @@ void acpi_os_unmap_generic_address(struct acpi_generic_address *gas)
461502
{
462503
u64 addr;
463504
struct acpi_ioremap *map;
464-
unsigned long refcount;
465505

466506
if (gas->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY)
467507
return;
@@ -472,16 +512,18 @@ void acpi_os_unmap_generic_address(struct acpi_generic_address *gas)
472512
return;
473513

474514
mutex_lock(&acpi_ioremap_lock);
515+
475516
map = acpi_map_lookup(addr, gas->bit_width / 8);
476517
if (!map) {
477518
mutex_unlock(&acpi_ioremap_lock);
478519
return;
479520
}
480-
refcount = acpi_os_drop_map_ref(map);
521+
if (acpi_os_drop_map_ref(map, false))
522+
map = NULL;
523+
481524
mutex_unlock(&acpi_ioremap_lock);
482525

483-
if (!refcount)
484-
acpi_os_map_cleanup(map);
526+
acpi_os_map_cleanup(map);
485527
}
486528
EXPORT_SYMBOL(acpi_os_unmap_generic_address);
487529

0 commit comments

Comments
 (0)