linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6] ACPI/OSL: Rework of ACPICA memory OSLs to improve performance.
@ 2014-10-23  2:11 Lv Zheng
  2014-10-23  2:12 ` [PATCH 1/6] ACPI/OSL: Split memory operation region implementations to a seperate file Lv Zheng
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: Lv Zheng @ 2014-10-23  2:11 UTC (permalink / raw)
  To: Rafael J. Wysocki, Len Brown
  Cc: Lv Zheng, Lv Zheng, linux-kernel, linux-acpi, Fei Yang

It is reported that there is a performance issue in the ACPICA OSL
implementation around memory mappings.

On the reported platforms, there is a debugging facility implemented in the
ACPI namespace using circular logging buffer:
        Name (DPTR, 0x3AFEB000)
        Name (EPTR, 0x3AFFB000)
        Name (CPTR, 0x3AFEB010)
        Mutex (MMUT, 0x00)
        Method (MDBG, 1, Serialized)
        {
            Store (Acquire (MMUT, 0x03E8), Local0)
            If (Local0 == Zero)
            {
                OperationRegion (ABLK, SystemMemory, CPTR, 0x10)
                Field (ABLK, ByteAcc, NoLock, Preserve)
                {
                    AAAA,   128
                }
                Store (Arg0, AAAA) /* \MDBG.AAAA */
                CPTR = (CPTR + 0x10) /* \CPTR */
                If (CPTR >= EPTR)
                {
                    CPTR = (DPTR + 0x10) /* \CPTR */
                }
                Release (MMUT)
            }
            Return (Local0)
        }
This function is heavily invoked by other ACPI control methods.
The reported platforms suffer from 2-5 minutes kernel stuck in the end of
accessing the mapped circular buffer system memory region. By
instrumentation, the 2-5 minutes time consumption overhead can be seen to
happen on the synchronize_rcu() invoked in the acpi_os_unmap_memory().

This patchset removes the synchronize_rcu() from the hot path to improve
the performance.

Lv Zheng (6):
  ACPI/OSL: Split memory operation region implementations to a seperate
    file.
  ACPI/OSL: Rename system memory functions.
  ACPI/OSL: Cleanup memory access functions by merging duplicate code.
  ACPI/OSL: Add acpi_map2virt() to merge duplicate code.
  ACPI/OSL: Cleanup branch logics.
  ACPI/OSL: Fix performance issue in system memory lockings.

 drivers/acpi/Makefile |    2 +-
 drivers/acpi/mem.c    |  414 +++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/acpi/osl.c    |  381 ---------------------------------------------
 3 files changed, 415 insertions(+), 382 deletions(-)
 create mode 100644 drivers/acpi/mem.c

-- 
1.7.10


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/6] ACPI/OSL: Split memory operation region implementations to a seperate file.
  2014-10-23  2:11 [PATCH 0/6] ACPI/OSL: Rework of ACPICA memory OSLs to improve performance Lv Zheng
@ 2014-10-23  2:12 ` Lv Zheng
  2014-10-23  2:12 ` [PATCH 2/6] ACPI/OSL: Rename system memory functions Lv Zheng
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Lv Zheng @ 2014-10-23  2:12 UTC (permalink / raw)
  To: Rafael J. Wysocki, Len Brown; +Cc: Lv Zheng, Lv Zheng, linux-kernel, linux-acpi

This patch moves SystemMemory operation region implementations to a
seperate file before doing cleanups. No functional changes.

Signed-off-by: Lv Zheng <lv.zheng@intel.com>
Tested-by: Fei Yang <fei.yang@intel.com>
---
 drivers/acpi/Makefile |    2 +-
 drivers/acpi/mem.c    |  395 +++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/acpi/osl.c    |  381 -----------------------------------------------
 3 files changed, 396 insertions(+), 382 deletions(-)
 create mode 100644 drivers/acpi/mem.c

diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index 505d4d7..802b887 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -18,7 +18,7 @@ obj-y				+= acpi.o \
 					acpica/
 
 # All the builtin files are in the "acpi." module_param namespace.
-acpi-y				+= osl.o utils.o reboot.o
+acpi-y				+= mem.o osl.o utils.o reboot.o
 acpi-y				+= nvs.o
 
 # Power management related files
diff --git a/drivers/acpi/mem.c b/drivers/acpi/mem.c
new file mode 100644
index 0000000..722241e
--- /dev/null
+++ b/drivers/acpi/mem.c
@@ -0,0 +1,395 @@
+/*
+ * ACPI system memory implementation
+ *
+ * Copyright (C) 2014, Intel Corporation
+ *   Author: Lv Zheng <lv.zheng@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/mm.h>
+#include <linux/highmem.h>
+#include <linux/io.h>
+#include <linux/acpi.h>
+
+#include "internal.h"
+
+
+/*
+ * This list of permanent mappings is for memory that may be accessed from
+ * interrupt context, where we can't do the ioremap().
+ */
+struct acpi_ioremap {
+	struct list_head list;
+	void __iomem *virt;
+	acpi_physical_address phys;
+	acpi_size size;
+	unsigned long refcount;
+};
+
+static LIST_HEAD(acpi_ioremaps);
+static DEFINE_MUTEX(acpi_ioremap_lock);
+
+static void acpi_os_drop_map_ref(struct acpi_ioremap *map)
+{
+	if (!--map->refcount)
+		list_del_rcu(&map->list);
+}
+
+/* Must be called with 'acpi_ioremap_lock' or RCU read lock held. */
+static struct acpi_ioremap *
+acpi_map_lookup(acpi_physical_address phys, acpi_size size)
+{
+	struct acpi_ioremap *map;
+
+	list_for_each_entry_rcu(map, &acpi_ioremaps, list)
+		if (map->phys <= phys &&
+		    phys + size <= map->phys + map->size)
+			return map;
+
+	return NULL;
+}
+
+/* Must be called with 'acpi_ioremap_lock' or RCU read lock held. */
+static struct acpi_ioremap *
+acpi_map_lookup_virt(void __iomem *virt, acpi_size size)
+{
+	struct acpi_ioremap *map;
+
+	list_for_each_entry_rcu(map, &acpi_ioremaps, list)
+		if (map->virt <= virt &&
+		    virt + size <= map->virt + map->size)
+			return map;
+
+	return NULL;
+}
+
+/* Must be called with 'acpi_ioremap_lock' or RCU read lock held. */
+static void __iomem *
+acpi_map_vaddr_lookup(acpi_physical_address phys, unsigned int size)
+{
+	struct acpi_ioremap *map;
+
+	map = acpi_map_lookup(phys, size);
+	if (map)
+		return map->virt + (phys - map->phys);
+
+	return NULL;
+}
+
+#ifndef CONFIG_IA64
+#define should_use_kmap(pfn)   page_is_ram(pfn)
+#else
+/* ioremap will take care of cache attributes */
+#define should_use_kmap(pfn)   0
+#endif
+
+static void __iomem *acpi_map(acpi_physical_address pg_off, unsigned long pg_sz)
+{
+	unsigned long pfn;
+
+	pfn = pg_off >> PAGE_SHIFT;
+	if (should_use_kmap(pfn)) {
+		if (pg_sz > PAGE_SIZE)
+			return NULL;
+		return (void __iomem __force *)kmap(pfn_to_page(pfn));
+	}
+	return acpi_os_ioremap(pg_off, pg_sz);
+}
+
+static void acpi_unmap(acpi_physical_address pg_off, void __iomem *vaddr)
+{
+	unsigned long pfn;
+
+	pfn = pg_off >> PAGE_SHIFT;
+	if (should_use_kmap(pfn))
+		kunmap(pfn_to_page(pfn));
+	else
+		iounmap(vaddr);
+}
+
+static void acpi_os_map_cleanup(struct acpi_ioremap *map)
+{
+	if (!map->refcount) {
+		synchronize_rcu();
+		acpi_unmap(map->phys, map->virt);
+		kfree(map);
+	}
+}
+
+void __iomem *acpi_os_get_iomem(acpi_physical_address phys, unsigned int size)
+{
+	struct acpi_ioremap *map;
+	void __iomem *virt = NULL;
+
+	mutex_lock(&acpi_ioremap_lock);
+	map = acpi_map_lookup(phys, size);
+	if (map) {
+		virt = map->virt + (phys - map->phys);
+		map->refcount++;
+	}
+	mutex_unlock(&acpi_ioremap_lock);
+
+	return virt;
+}
+EXPORT_SYMBOL_GPL(acpi_os_get_iomem);
+
+void __iomem *__init_refok
+acpi_os_map_iomem(acpi_physical_address phys, acpi_size size)
+{
+	struct acpi_ioremap *map;
+	void __iomem *virt;
+	acpi_physical_address pg_off;
+	acpi_size pg_sz;
+
+	if (phys > ULONG_MAX) {
+		pr_err(PREFIX "Cannot map memory that high\n");
+		return NULL;
+	}
+
+	if (!acpi_gbl_permanent_mmap)
+		return __acpi_map_table((unsigned long)phys, size);
+
+	mutex_lock(&acpi_ioremap_lock);
+	/* Check if there's a suitable mapping already. */
+	map = acpi_map_lookup(phys, size);
+	if (map) {
+		map->refcount++;
+		goto out;
+	}
+
+	map = kzalloc(sizeof(*map), GFP_KERNEL);
+	if (!map) {
+		mutex_unlock(&acpi_ioremap_lock);
+		return NULL;
+	}
+
+	pg_off = round_down(phys, PAGE_SIZE);
+	pg_sz = round_up(phys + size, PAGE_SIZE) - pg_off;
+	virt = acpi_map(pg_off, pg_sz);
+	if (!virt) {
+		mutex_unlock(&acpi_ioremap_lock);
+		kfree(map);
+		return NULL;
+	}
+
+	INIT_LIST_HEAD(&map->list);
+	map->virt = virt;
+	map->phys = pg_off;
+	map->size = pg_sz;
+	map->refcount = 1;
+	list_add_tail_rcu(&map->list, &acpi_ioremaps);
+out:
+	mutex_unlock(&acpi_ioremap_lock);
+	return map->virt + (phys - map->phys);
+}
+EXPORT_SYMBOL_GPL(acpi_os_map_iomem);
+
+void __ref acpi_os_unmap_iomem(void __iomem *virt, acpi_size size)
+{
+	struct acpi_ioremap *map;
+
+	if (!acpi_gbl_permanent_mmap) {
+		__acpi_unmap_table(virt, size);
+		return;
+	}
+
+	mutex_lock(&acpi_ioremap_lock);
+	map = acpi_map_lookup_virt(virt, size);
+	if (!map) {
+		mutex_unlock(&acpi_ioremap_lock);
+		WARN(true, PREFIX "%s: bad address %p\n", __func__, virt);
+		return;
+	}
+	acpi_os_drop_map_ref(map);
+	mutex_unlock(&acpi_ioremap_lock);
+
+	acpi_os_map_cleanup(map);
+}
+EXPORT_SYMBOL_GPL(acpi_os_unmap_iomem);
+
+void *__init_refok
+acpi_os_map_memory(acpi_physical_address phys, acpi_size size)
+{
+	return (void *)acpi_os_map_iomem(phys, size);
+}
+EXPORT_SYMBOL_GPL(acpi_os_map_memory);
+
+void __ref acpi_os_unmap_memory(void *virt, acpi_size size)
+{
+	return acpi_os_unmap_iomem((void __iomem *)virt, size);
+}
+EXPORT_SYMBOL_GPL(acpi_os_unmap_memory);
+
+void __init early_acpi_os_unmap_memory(void __iomem *virt, acpi_size size)
+{
+	if (!acpi_gbl_permanent_mmap)
+		__acpi_unmap_table(virt, size);
+}
+
+int acpi_os_map_generic_address(struct acpi_generic_address *gas)
+{
+	u64 addr;
+	void __iomem *virt;
+
+	if (gas->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY)
+		return 0;
+
+	/* Handle possible alignment issues */
+	memcpy(&addr, &gas->address, sizeof(addr));
+	if (!addr || !gas->bit_width)
+		return -EINVAL;
+
+	virt = acpi_os_map_iomem(addr, gas->bit_width / 8);
+	if (!virt)
+		return -EIO;
+
+	return 0;
+}
+EXPORT_SYMBOL(acpi_os_map_generic_address);
+
+void acpi_os_unmap_generic_address(struct acpi_generic_address *gas)
+{
+	u64 addr;
+	struct acpi_ioremap *map;
+
+	if (gas->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY)
+		return;
+
+	/* Handle possible alignment issues */
+	memcpy(&addr, &gas->address, sizeof(addr));
+	if (!addr || !gas->bit_width)
+		return;
+
+	mutex_lock(&acpi_ioremap_lock);
+	map = acpi_map_lookup(addr, gas->bit_width / 8);
+	if (!map) {
+		mutex_unlock(&acpi_ioremap_lock);
+		return;
+	}
+	acpi_os_drop_map_ref(map);
+	mutex_unlock(&acpi_ioremap_lock);
+
+	acpi_os_map_cleanup(map);
+}
+EXPORT_SYMBOL(acpi_os_unmap_generic_address);
+
+#ifdef readq
+static inline u64 read64(const volatile void __iomem *addr)
+{
+	return readq(addr);
+}
+#else
+static inline u64 read64(const volatile void __iomem *addr)
+{
+	u64 l, h;
+
+	l = readl(addr);
+	h = readl(addr+4);
+	return l | (h << 32);
+}
+#endif
+
+#ifdef writeq
+static inline void write64(u64 val, volatile void __iomem *addr)
+{
+	writeq(val, addr);
+}
+#else
+static inline void write64(u64 val, volatile void __iomem *addr)
+{
+	writel(val, addr);
+	writel(val>>32, addr+4);
+}
+#endif
+
+acpi_status
+acpi_os_read_memory(acpi_physical_address phys_addr, u64 *value, u32 width)
+{
+	void __iomem *virt_addr;
+	unsigned int size = width / 8;
+	bool unmap = false;
+	u64 dummy;
+
+	rcu_read_lock();
+	virt_addr = acpi_map_vaddr_lookup(phys_addr, size);
+	if (!virt_addr) {
+		rcu_read_unlock();
+		virt_addr = acpi_os_ioremap(phys_addr, size);
+		if (!virt_addr)
+			return AE_BAD_ADDRESS;
+		unmap = true;
+	}
+
+	if (!value)
+		value = &dummy;
+
+	switch (width) {
+	case 8:
+		*(u8 *) value = readb(virt_addr);
+		break;
+	case 16:
+		*(u16 *) value = readw(virt_addr);
+		break;
+	case 32:
+		*(u32 *) value = readl(virt_addr);
+		break;
+	case 64:
+		*(u64 *) value = read64(virt_addr);
+		break;
+	default:
+		BUG();
+	}
+
+	if (unmap)
+		iounmap(virt_addr);
+	else
+		rcu_read_unlock();
+
+	return AE_OK;
+}
+
+acpi_status
+acpi_os_write_memory(acpi_physical_address phys_addr, u64 value, u32 width)
+{
+	void __iomem *virt_addr;
+	unsigned int size = width / 8;
+	bool unmap = false;
+
+	rcu_read_lock();
+	virt_addr = acpi_map_vaddr_lookup(phys_addr, size);
+	if (!virt_addr) {
+		rcu_read_unlock();
+		virt_addr = acpi_os_ioremap(phys_addr, size);
+		if (!virt_addr)
+			return AE_BAD_ADDRESS;
+		unmap = true;
+	}
+
+	switch (width) {
+	case 8:
+		writeb(value, virt_addr);
+		break;
+	case 16:
+		writew(value, virt_addr);
+		break;
+	case 32:
+		writel(value, virt_addr);
+		break;
+	case 64:
+		write64(value, virt_addr);
+		break;
+	default:
+		BUG();
+	}
+
+	if (unmap)
+		iounmap(virt_addr);
+	else
+		rcu_read_unlock();
+
+	return AE_OK;
+}
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 9964f70..3533814 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -30,8 +30,6 @@
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/slab.h>
-#include <linux/mm.h>
-#include <linux/highmem.h>
 #include <linux/pci.h>
 #include <linux/interrupt.h>
 #include <linux/kmod.h>
@@ -45,9 +43,6 @@
 #include <linux/jiffies.h>
 #include <linux/semaphore.h>
 
-#include <asm/io.h>
-#include <asm/uaccess.h>
-
 #include "internal.h"
 
 #define _COMPONENT		ACPI_OS_SERVICES
@@ -84,21 +79,6 @@ static struct workqueue_struct *kacpid_wq;
 static struct workqueue_struct *kacpi_notify_wq;
 static struct workqueue_struct *kacpi_hotplug_wq;
 
-/*
- * This list of permanent mappings is for memory that may be accessed from
- * interrupt context, where we can't do the ioremap().
- */
-struct acpi_ioremap {
-	struct list_head list;
-	void __iomem *virt;
-	acpi_physical_address phys;
-	acpi_size size;
-	unsigned long refcount;
-};
-
-static LIST_HEAD(acpi_ioremaps);
-static DEFINE_MUTEX(acpi_ioremap_lock);
-
 static void __init acpi_osi_setup_late(void);
 
 /*
@@ -279,251 +259,6 @@ acpi_physical_address __init acpi_os_get_root_pointer(void)
 	return 0;
 }
 
-/* Must be called with 'acpi_ioremap_lock' or RCU read lock held. */
-static struct acpi_ioremap *
-acpi_map_lookup(acpi_physical_address phys, acpi_size size)
-{
-	struct acpi_ioremap *map;
-
-	list_for_each_entry_rcu(map, &acpi_ioremaps, list)
-		if (map->phys <= phys &&
-		    phys + size <= map->phys + map->size)
-			return map;
-
-	return NULL;
-}
-
-/* Must be called with 'acpi_ioremap_lock' or RCU read lock held. */
-static void __iomem *
-acpi_map_vaddr_lookup(acpi_physical_address phys, unsigned int size)
-{
-	struct acpi_ioremap *map;
-
-	map = acpi_map_lookup(phys, size);
-	if (map)
-		return map->virt + (phys - map->phys);
-
-	return NULL;
-}
-
-void __iomem *acpi_os_get_iomem(acpi_physical_address phys, unsigned int size)
-{
-	struct acpi_ioremap *map;
-	void __iomem *virt = NULL;
-
-	mutex_lock(&acpi_ioremap_lock);
-	map = acpi_map_lookup(phys, size);
-	if (map) {
-		virt = map->virt + (phys - map->phys);
-		map->refcount++;
-	}
-	mutex_unlock(&acpi_ioremap_lock);
-	return virt;
-}
-EXPORT_SYMBOL_GPL(acpi_os_get_iomem);
-
-/* Must be called with 'acpi_ioremap_lock' or RCU read lock held. */
-static struct acpi_ioremap *
-acpi_map_lookup_virt(void __iomem *virt, acpi_size size)
-{
-	struct acpi_ioremap *map;
-
-	list_for_each_entry_rcu(map, &acpi_ioremaps, list)
-		if (map->virt <= virt &&
-		    virt + size <= map->virt + map->size)
-			return map;
-
-	return NULL;
-}
-
-#ifndef CONFIG_IA64
-#define should_use_kmap(pfn)   page_is_ram(pfn)
-#else
-/* ioremap will take care of cache attributes */
-#define should_use_kmap(pfn)   0
-#endif
-
-static void __iomem *acpi_map(acpi_physical_address pg_off, unsigned long pg_sz)
-{
-	unsigned long pfn;
-
-	pfn = pg_off >> PAGE_SHIFT;
-	if (should_use_kmap(pfn)) {
-		if (pg_sz > PAGE_SIZE)
-			return NULL;
-		return (void __iomem __force *)kmap(pfn_to_page(pfn));
-	} else
-		return acpi_os_ioremap(pg_off, pg_sz);
-}
-
-static void acpi_unmap(acpi_physical_address pg_off, void __iomem *vaddr)
-{
-	unsigned long pfn;
-
-	pfn = pg_off >> PAGE_SHIFT;
-	if (should_use_kmap(pfn))
-		kunmap(pfn_to_page(pfn));
-	else
-		iounmap(vaddr);
-}
-
-void __iomem *__init_refok
-acpi_os_map_iomem(acpi_physical_address phys, acpi_size size)
-{
-	struct acpi_ioremap *map;
-	void __iomem *virt;
-	acpi_physical_address pg_off;
-	acpi_size pg_sz;
-
-	if (phys > ULONG_MAX) {
-		printk(KERN_ERR PREFIX "Cannot map memory that high\n");
-		return NULL;
-	}
-
-	if (!acpi_gbl_permanent_mmap)
-		return __acpi_map_table((unsigned long)phys, size);
-
-	mutex_lock(&acpi_ioremap_lock);
-	/* Check if there's a suitable mapping already. */
-	map = acpi_map_lookup(phys, size);
-	if (map) {
-		map->refcount++;
-		goto out;
-	}
-
-	map = kzalloc(sizeof(*map), GFP_KERNEL);
-	if (!map) {
-		mutex_unlock(&acpi_ioremap_lock);
-		return NULL;
-	}
-
-	pg_off = round_down(phys, PAGE_SIZE);
-	pg_sz = round_up(phys + size, PAGE_SIZE) - pg_off;
-	virt = acpi_map(pg_off, pg_sz);
-	if (!virt) {
-		mutex_unlock(&acpi_ioremap_lock);
-		kfree(map);
-		return NULL;
-	}
-
-	INIT_LIST_HEAD(&map->list);
-	map->virt = virt;
-	map->phys = pg_off;
-	map->size = pg_sz;
-	map->refcount = 1;
-
-	list_add_tail_rcu(&map->list, &acpi_ioremaps);
-
-out:
-	mutex_unlock(&acpi_ioremap_lock);
-	return map->virt + (phys - map->phys);
-}
-EXPORT_SYMBOL_GPL(acpi_os_map_iomem);
-
-void *__init_refok
-acpi_os_map_memory(acpi_physical_address phys, acpi_size size)
-{
-	return (void *)acpi_os_map_iomem(phys, size);
-}
-EXPORT_SYMBOL_GPL(acpi_os_map_memory);
-
-static void acpi_os_drop_map_ref(struct acpi_ioremap *map)
-{
-	if (!--map->refcount)
-		list_del_rcu(&map->list);
-}
-
-static void acpi_os_map_cleanup(struct acpi_ioremap *map)
-{
-	if (!map->refcount) {
-		synchronize_rcu();
-		acpi_unmap(map->phys, map->virt);
-		kfree(map);
-	}
-}
-
-void __ref acpi_os_unmap_iomem(void __iomem *virt, acpi_size size)
-{
-	struct acpi_ioremap *map;
-
-	if (!acpi_gbl_permanent_mmap) {
-		__acpi_unmap_table(virt, size);
-		return;
-	}
-
-	mutex_lock(&acpi_ioremap_lock);
-	map = acpi_map_lookup_virt(virt, size);
-	if (!map) {
-		mutex_unlock(&acpi_ioremap_lock);
-		WARN(true, PREFIX "%s: bad address %p\n", __func__, virt);
-		return;
-	}
-	acpi_os_drop_map_ref(map);
-	mutex_unlock(&acpi_ioremap_lock);
-
-	acpi_os_map_cleanup(map);
-}
-EXPORT_SYMBOL_GPL(acpi_os_unmap_iomem);
-
-void __ref acpi_os_unmap_memory(void *virt, acpi_size size)
-{
-	return acpi_os_unmap_iomem((void __iomem *)virt, size);
-}
-EXPORT_SYMBOL_GPL(acpi_os_unmap_memory);
-
-void __init early_acpi_os_unmap_memory(void __iomem *virt, acpi_size size)
-{
-	if (!acpi_gbl_permanent_mmap)
-		__acpi_unmap_table(virt, size);
-}
-
-int acpi_os_map_generic_address(struct acpi_generic_address *gas)
-{
-	u64 addr;
-	void __iomem *virt;
-
-	if (gas->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY)
-		return 0;
-
-	/* Handle possible alignment issues */
-	memcpy(&addr, &gas->address, sizeof(addr));
-	if (!addr || !gas->bit_width)
-		return -EINVAL;
-
-	virt = acpi_os_map_iomem(addr, gas->bit_width / 8);
-	if (!virt)
-		return -EIO;
-
-	return 0;
-}
-EXPORT_SYMBOL(acpi_os_map_generic_address);
-
-void acpi_os_unmap_generic_address(struct acpi_generic_address *gas)
-{
-	u64 addr;
-	struct acpi_ioremap *map;
-
-	if (gas->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY)
-		return;
-
-	/* Handle possible alignment issues */
-	memcpy(&addr, &gas->address, sizeof(addr));
-	if (!addr || !gas->bit_width)
-		return;
-
-	mutex_lock(&acpi_ioremap_lock);
-	map = acpi_map_lookup(addr, gas->bit_width / 8);
-	if (!map) {
-		mutex_unlock(&acpi_ioremap_lock);
-		return;
-	}
-	acpi_os_drop_map_ref(map);
-	mutex_unlock(&acpi_ioremap_lock);
-
-	acpi_os_map_cleanup(map);
-}
-EXPORT_SYMBOL(acpi_os_unmap_generic_address);
-
 #ifdef ACPI_FUTURE_USAGE
 acpi_status
 acpi_os_get_physical_address(void *virt, acpi_physical_address * phys)
@@ -929,122 +664,6 @@ acpi_status acpi_os_write_port(acpi_io_address port, u32 value, u32 width)
 
 EXPORT_SYMBOL(acpi_os_write_port);
 
-#ifdef readq
-static inline u64 read64(const volatile void __iomem *addr)
-{
-	return readq(addr);
-}
-#else
-static inline u64 read64(const volatile void __iomem *addr)
-{
-	u64 l, h;
-	l = readl(addr);
-	h = readl(addr+4);
-	return l | (h << 32);
-}
-#endif
-
-acpi_status
-acpi_os_read_memory(acpi_physical_address phys_addr, u64 *value, u32 width)
-{
-	void __iomem *virt_addr;
-	unsigned int size = width / 8;
-	bool unmap = false;
-	u64 dummy;
-
-	rcu_read_lock();
-	virt_addr = acpi_map_vaddr_lookup(phys_addr, size);
-	if (!virt_addr) {
-		rcu_read_unlock();
-		virt_addr = acpi_os_ioremap(phys_addr, size);
-		if (!virt_addr)
-			return AE_BAD_ADDRESS;
-		unmap = true;
-	}
-
-	if (!value)
-		value = &dummy;
-
-	switch (width) {
-	case 8:
-		*(u8 *) value = readb(virt_addr);
-		break;
-	case 16:
-		*(u16 *) value = readw(virt_addr);
-		break;
-	case 32:
-		*(u32 *) value = readl(virt_addr);
-		break;
-	case 64:
-		*(u64 *) value = read64(virt_addr);
-		break;
-	default:
-		BUG();
-	}
-
-	if (unmap)
-		iounmap(virt_addr);
-	else
-		rcu_read_unlock();
-
-	return AE_OK;
-}
-
-#ifdef writeq
-static inline void write64(u64 val, volatile void __iomem *addr)
-{
-	writeq(val, addr);
-}
-#else
-static inline void write64(u64 val, volatile void __iomem *addr)
-{
-	writel(val, addr);
-	writel(val>>32, addr+4);
-}
-#endif
-
-acpi_status
-acpi_os_write_memory(acpi_physical_address phys_addr, u64 value, u32 width)
-{
-	void __iomem *virt_addr;
-	unsigned int size = width / 8;
-	bool unmap = false;
-
-	rcu_read_lock();
-	virt_addr = acpi_map_vaddr_lookup(phys_addr, size);
-	if (!virt_addr) {
-		rcu_read_unlock();
-		virt_addr = acpi_os_ioremap(phys_addr, size);
-		if (!virt_addr)
-			return AE_BAD_ADDRESS;
-		unmap = true;
-	}
-
-	switch (width) {
-	case 8:
-		writeb(value, virt_addr);
-		break;
-	case 16:
-		writew(value, virt_addr);
-		break;
-	case 32:
-		writel(value, virt_addr);
-		break;
-	case 64:
-		write64(value, virt_addr);
-		break;
-	default:
-		BUG();
-	}
-
-	if (unmap)
-		iounmap(virt_addr);
-	else
-		rcu_read_unlock();
-
-	return AE_OK;
-}
-
 acpi_status
 acpi_os_read_pci_configuration(struct acpi_pci_id * pci_id, u32 reg,
 			       u64 *value, u32 width)
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/6] ACPI/OSL: Rename system memory functions.
  2014-10-23  2:11 [PATCH 0/6] ACPI/OSL: Rework of ACPICA memory OSLs to improve performance Lv Zheng
  2014-10-23  2:12 ` [PATCH 1/6] ACPI/OSL: Split memory operation region implementations to a seperate file Lv Zheng
@ 2014-10-23  2:12 ` Lv Zheng
  2014-10-23  2:12 ` [PATCH 3/6] ACPI/OSL: Cleanup memory access functions by merging duplicate code Lv Zheng
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Lv Zheng @ 2014-10-23  2:12 UTC (permalink / raw)
  To: Rafael J. Wysocki, Len Brown; +Cc: Lv Zheng, Lv Zheng, linux-kernel, linux-acpi

This patch cleans up system memory functions to make it easier to
understand the meaning of such functions.
No functional cleanup.

Signed-off-by: Lv Zheng <lv.zheng@intel.com>
Tested-by: Fei Yang <fei.yang@intel.com>
---
 drivers/acpi/mem.c |   38 ++++++++++++++++++++++----------------
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/drivers/acpi/mem.c b/drivers/acpi/mem.c
index 722241e..defd317 100644
--- a/drivers/acpi/mem.c
+++ b/drivers/acpi/mem.c
@@ -33,15 +33,23 @@ struct acpi_ioremap {
 static LIST_HEAD(acpi_ioremaps);
 static DEFINE_MUTEX(acpi_ioremap_lock);
 
-static void acpi_os_drop_map_ref(struct acpi_ioremap *map)
+/*
+ * The following functions must be called with 'acpi_ioremap_lock' or RCU
+ * read lock held.
+ */
+static inline void acpi_map_get(struct acpi_ioremap *map)
+{
+	map->refcount++;
+}
+
+static inline void acpi_map_put(struct acpi_ioremap *map)
 {
 	if (!--map->refcount)
 		list_del_rcu(&map->list);
 }
 
-/* Must be called with 'acpi_ioremap_lock' or RCU read lock held. */
 static struct acpi_ioremap *
-acpi_map_lookup(acpi_physical_address phys, acpi_size size)
+acpi_map_lookup_phys(acpi_physical_address phys, acpi_size size)
 {
 	struct acpi_ioremap *map;
 
@@ -53,7 +61,6 @@ acpi_map_lookup(acpi_physical_address phys, acpi_size size)
 	return NULL;
 }
 
-/* Must be called with 'acpi_ioremap_lock' or RCU read lock held. */
 static struct acpi_ioremap *
 acpi_map_lookup_virt(void __iomem *virt, acpi_size size)
 {
@@ -67,13 +74,12 @@ acpi_map_lookup_virt(void __iomem *virt, acpi_size size)
 	return NULL;
 }
 
-/* Must be called with 'acpi_ioremap_lock' or RCU read lock held. */
 static void __iomem *
 acpi_map_vaddr_lookup(acpi_physical_address phys, unsigned int size)
 {
 	struct acpi_ioremap *map;
 
-	map = acpi_map_lookup(phys, size);
+	map = acpi_map_lookup_phys(phys, size);
 	if (map)
 		return map->virt + (phys - map->phys);
 
@@ -111,7 +117,7 @@ static void acpi_unmap(acpi_physical_address pg_off, void __iomem *vaddr)
 		iounmap(vaddr);
 }
 
-static void acpi_os_map_cleanup(struct acpi_ioremap *map)
+static void acpi_map_cleanup(struct acpi_ioremap *map)
 {
 	if (!map->refcount) {
 		synchronize_rcu();
@@ -126,10 +132,10 @@ void __iomem *acpi_os_get_iomem(acpi_physical_address phys, unsigned int size)
 	void __iomem *virt = NULL;
 
 	mutex_lock(&acpi_ioremap_lock);
-	map = acpi_map_lookup(phys, size);
+	map = acpi_map_lookup_phys(phys, size);
 	if (map) {
 		virt = map->virt + (phys - map->phys);
-		map->refcount++;
+		acpi_map_get(map);
 	}
 	mutex_unlock(&acpi_ioremap_lock);
 
@@ -155,9 +161,9 @@ acpi_os_map_iomem(acpi_physical_address phys, acpi_size size)
 
 	mutex_lock(&acpi_ioremap_lock);
 	/* Check if there's a suitable mapping already. */
-	map = acpi_map_lookup(phys, size);
+	map = acpi_map_lookup_phys(phys, size);
 	if (map) {
-		map->refcount++;
+		acpi_map_get(map);
 		goto out;
 	}
 
@@ -204,10 +210,10 @@ void __ref acpi_os_unmap_iomem(void __iomem *virt, acpi_size size)
 		WARN(true, PREFIX "%s: bad address %p\n", __func__, virt);
 		return;
 	}
-	acpi_os_drop_map_ref(map);
+	acpi_map_put(map);
 	mutex_unlock(&acpi_ioremap_lock);
 
-	acpi_os_map_cleanup(map);
+	acpi_map_cleanup(map);
 }
 EXPORT_SYMBOL_GPL(acpi_os_unmap_iomem);
 
@@ -265,15 +271,15 @@ void acpi_os_unmap_generic_address(struct acpi_generic_address *gas)
 		return;
 
 	mutex_lock(&acpi_ioremap_lock);
-	map = acpi_map_lookup(addr, gas->bit_width / 8);
+	map = acpi_map_lookup_phys(addr, gas->bit_width / 8);
 	if (!map) {
 		mutex_unlock(&acpi_ioremap_lock);
 		return;
 	}
-	acpi_os_drop_map_ref(map);
+	acpi_map_put(map);
 	mutex_unlock(&acpi_ioremap_lock);
 
-	acpi_os_map_cleanup(map);
+	acpi_map_cleanup(map);
 }
 EXPORT_SYMBOL(acpi_os_unmap_generic_address);
 
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/6] ACPI/OSL: Cleanup memory access functions by merging duplicate code.
  2014-10-23  2:11 [PATCH 0/6] ACPI/OSL: Rework of ACPICA memory OSLs to improve performance Lv Zheng
  2014-10-23  2:12 ` [PATCH 1/6] ACPI/OSL: Split memory operation region implementations to a seperate file Lv Zheng
  2014-10-23  2:12 ` [PATCH 2/6] ACPI/OSL: Rename system memory functions Lv Zheng
@ 2014-10-23  2:12 ` Lv Zheng
  2014-10-23  2:12 ` [PATCH 4/6] ACPI/OSL: Add acpi_map2virt() to merge " Lv Zheng
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Lv Zheng @ 2014-10-23  2:12 UTC (permalink / raw)
  To: Rafael J. Wysocki, Len Brown; +Cc: Lv Zheng, Lv Zheng, linux-kernel, linux-acpi

This patch merges duplicate code to cleanup ACPI memory access
implementations. No functional changes.

Signed-off-by: Lv Zheng <lv.zheng@intel.com>
Tested-by: Fei Yang <fei.yang@intel.com>
---
 drivers/acpi/mem.c |   13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/drivers/acpi/mem.c b/drivers/acpi/mem.c
index defd317..ba74086 100644
--- a/drivers/acpi/mem.c
+++ b/drivers/acpi/mem.c
@@ -260,7 +260,6 @@ EXPORT_SYMBOL(acpi_os_map_generic_address);
 void acpi_os_unmap_generic_address(struct acpi_generic_address *gas)
 {
 	u64 addr;
-	struct acpi_ioremap *map;
 
 	if (gas->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY)
 		return;
@@ -270,16 +269,8 @@ void acpi_os_unmap_generic_address(struct acpi_generic_address *gas)
 	if (!addr || !gas->bit_width)
 		return;
 
-	mutex_lock(&acpi_ioremap_lock);
-	map = acpi_map_lookup_phys(addr, gas->bit_width / 8);
-	if (!map) {
-		mutex_unlock(&acpi_ioremap_lock);
-		return;
-	}
-	acpi_map_put(map);
-	mutex_unlock(&acpi_ioremap_lock);
-
-	acpi_map_cleanup(map);
+	acpi_os_unmap_iomem((void __iomem *)((unsigned long)addr),
+			    gas->bit_width / 8);
 }
 EXPORT_SYMBOL(acpi_os_unmap_generic_address);
 
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 4/6] ACPI/OSL: Add acpi_map2virt() to merge duplicate code.
  2014-10-23  2:11 [PATCH 0/6] ACPI/OSL: Rework of ACPICA memory OSLs to improve performance Lv Zheng
                   ` (2 preceding siblings ...)
  2014-10-23  2:12 ` [PATCH 3/6] ACPI/OSL: Cleanup memory access functions by merging duplicate code Lv Zheng
@ 2014-10-23  2:12 ` Lv Zheng
  2014-10-23  2:12 ` [PATCH 5/6] ACPI/OSL: Cleanup branch logics Lv Zheng
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Lv Zheng @ 2014-10-23  2:12 UTC (permalink / raw)
  To: Rafael J. Wysocki, Len Brown; +Cc: Lv Zheng, Lv Zheng, linux-kernel, linux-acpi

This patch adds acpi_map2virt() so that some duplicated code can be merged.
No functional changes.

Signed-off-by: Lv Zheng <lv.zheng@intel.com>
Tested-by: Fei Yang <fei.yang@intel.com>
---
 drivers/acpi/mem.c |   45 ++++++++++++++++++++-------------------------
 1 file changed, 20 insertions(+), 25 deletions(-)

diff --git a/drivers/acpi/mem.c b/drivers/acpi/mem.c
index ba74086..c472237 100644
--- a/drivers/acpi/mem.c
+++ b/drivers/acpi/mem.c
@@ -74,16 +74,10 @@ acpi_map_lookup_virt(void __iomem *virt, acpi_size size)
 	return NULL;
 }
 
-static void __iomem *
-acpi_map_vaddr_lookup(acpi_physical_address phys, unsigned int size)
+static inline void __iomem *
+acpi_map2virt(struct acpi_ioremap *map, acpi_physical_address phys)
 {
-	struct acpi_ioremap *map;
-
-	map = acpi_map_lookup_phys(phys, size);
-	if (map)
-		return map->virt + (phys - map->phys);
-
-	return NULL;
+	return map ? map->virt + (phys - map->phys) : NULL;
 }
 
 #ifndef CONFIG_IA64
@@ -129,17 +123,14 @@ static void acpi_map_cleanup(struct acpi_ioremap *map)
 void __iomem *acpi_os_get_iomem(acpi_physical_address phys, unsigned int size)
 {
 	struct acpi_ioremap *map;
-	void __iomem *virt = NULL;
 
 	mutex_lock(&acpi_ioremap_lock);
 	map = acpi_map_lookup_phys(phys, size);
-	if (map) {
-		virt = map->virt + (phys - map->phys);
+	if (map)
 		acpi_map_get(map);
-	}
 	mutex_unlock(&acpi_ioremap_lock);
 
-	return virt;
+	return acpi_map2virt(map, phys);
 }
 EXPORT_SYMBOL_GPL(acpi_os_get_iomem);
 
@@ -168,18 +159,16 @@ acpi_os_map_iomem(acpi_physical_address phys, acpi_size size)
 	}
 
 	map = kzalloc(sizeof(*map), GFP_KERNEL);
-	if (!map) {
-		mutex_unlock(&acpi_ioremap_lock);
-		return NULL;
-	}
+	if (!map)
+		goto out;
 
 	pg_off = round_down(phys, PAGE_SIZE);
 	pg_sz = round_up(phys + size, PAGE_SIZE) - pg_off;
 	virt = acpi_map(pg_off, pg_sz);
 	if (!virt) {
-		mutex_unlock(&acpi_ioremap_lock);
 		kfree(map);
-		return NULL;
+		map = NULL;
+		goto out;
 	}
 
 	INIT_LIST_HEAD(&map->list);
@@ -190,7 +179,7 @@ acpi_os_map_iomem(acpi_physical_address phys, acpi_size size)
 	list_add_tail_rcu(&map->list, &acpi_ioremaps);
 out:
 	mutex_unlock(&acpi_ioremap_lock);
-	return map->virt + (phys - map->phys);
+	return acpi_map2virt(map, phys);
 }
 EXPORT_SYMBOL_GPL(acpi_os_map_iomem);
 
@@ -310,10 +299,13 @@ acpi_os_read_memory(acpi_physical_address phys_addr, u64 *value, u32 width)
 	unsigned int size = width / 8;
 	bool unmap = false;
 	u64 dummy;
+	struct acpi_ioremap *map;
 
 	rcu_read_lock();
-	virt_addr = acpi_map_vaddr_lookup(phys_addr, size);
-	if (!virt_addr) {
+	map = acpi_map_lookup_phys(phys_addr, size);
+	if (map)
+		virt_addr = acpi_map2virt(map, phys_addr);
+	else {
 		rcu_read_unlock();
 		virt_addr = acpi_os_ioremap(phys_addr, size);
 		if (!virt_addr)
@@ -355,10 +347,13 @@ acpi_os_write_memory(acpi_physical_address phys_addr, u64 value, u32 width)
 	void __iomem *virt_addr;
 	unsigned int size = width / 8;
 	bool unmap = false;
+	struct acpi_ioremap *map;
 
 	rcu_read_lock();
-	virt_addr = acpi_map_vaddr_lookup(phys_addr, size);
-	if (!virt_addr) {
+	map = acpi_map_lookup_phys(phys_addr, size);
+	if (map)
+		virt_addr = acpi_map2virt(map, phys_addr);
+	else {
 		rcu_read_unlock();
 		virt_addr = acpi_os_ioremap(phys_addr, size);
 		if (!virt_addr)
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 5/6] ACPI/OSL: Cleanup branch logics.
  2014-10-23  2:11 [PATCH 0/6] ACPI/OSL: Rework of ACPICA memory OSLs to improve performance Lv Zheng
                   ` (3 preceding siblings ...)
  2014-10-23  2:12 ` [PATCH 4/6] ACPI/OSL: Add acpi_map2virt() to merge " Lv Zheng
@ 2014-10-23  2:12 ` Lv Zheng
  2014-10-23  2:12 ` [PATCH 6/6] ACPI/OSL: Fix performance issue in system memory lockings Lv Zheng
  2014-10-28  5:15 ` [PATCH 0/6] ACPI/OSL: Rework of ACPICA memory OSLs to improve performance Zheng, Lv
  6 siblings, 0 replies; 8+ messages in thread
From: Lv Zheng @ 2014-10-23  2:12 UTC (permalink / raw)
  To: Rafael J. Wysocki, Len Brown; +Cc: Lv Zheng, Lv Zheng, linux-kernel, linux-acpi

This patch cleans up trivial branch logics in acpi_os_unmap_iomem() to
eliminate several lines. No functional changes.

Signed-off-by: Lv Zheng <lv.zheng@intel.com>
Tested-by: Fei Yang <fei.yang@intel.com>
---
 drivers/acpi/mem.c |    8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/mem.c b/drivers/acpi/mem.c
index c472237..c7c35f7 100644
--- a/drivers/acpi/mem.c
+++ b/drivers/acpi/mem.c
@@ -194,12 +194,10 @@ void __ref acpi_os_unmap_iomem(void __iomem *virt, acpi_size size)
 
 	mutex_lock(&acpi_ioremap_lock);
 	map = acpi_map_lookup_virt(virt, size);
-	if (!map) {
-		mutex_unlock(&acpi_ioremap_lock);
+	if (map)
+		acpi_map_put(map);
+	else
 		WARN(true, PREFIX "%s: bad address %p\n", __func__, virt);
-		return;
-	}
-	acpi_map_put(map);
 	mutex_unlock(&acpi_ioremap_lock);
 
 	acpi_map_cleanup(map);
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 6/6] ACPI/OSL: Fix performance issue in system memory lockings.
  2014-10-23  2:11 [PATCH 0/6] ACPI/OSL: Rework of ACPICA memory OSLs to improve performance Lv Zheng
                   ` (4 preceding siblings ...)
  2014-10-23  2:12 ` [PATCH 5/6] ACPI/OSL: Cleanup branch logics Lv Zheng
@ 2014-10-23  2:12 ` Lv Zheng
  2014-10-28  5:15 ` [PATCH 0/6] ACPI/OSL: Rework of ACPICA memory OSLs to improve performance Zheng, Lv
  6 siblings, 0 replies; 8+ messages in thread
From: Lv Zheng @ 2014-10-23  2:12 UTC (permalink / raw)
  To: Rafael J. Wysocki, Len Brown; +Cc: Lv Zheng, Lv Zheng, linux-kernel, linux-acpi

It is reported that the synchronize_rcu() used in the hot path is not
performance friendly:
 <6>[    3.998532] acpi_ut_update_object_reference: obj:ffff88003c305f78 type:10
 <6>[    4.006137] acpi_ut_update_ref_count:locked count:1 action:1 flags:286
 <6>[    4.013450] acpi_ut_delete_internal_obj: obj:ffff88003c305f78 flag:2
 <6>[    4.020569] acpi_ut_delete_internal_obj:second_desc handler_desc:ffff88003c0784c8 flags:1
 <6>[    4.029729] acpi_ut_delete_internal_obj: going to setup @ ffffffff81489299
 <6>[    4.037431] acpi_ev_system_memory_region_setup: addr:ffffc900000b0070, length:16
*<6>[    4.045716] acpi_os_unmap_memory: locked
*<6>[    4.050111] acpi_os_unmap_memory: found map
*<6>[    4.054798] acpi_os_unmap_memory: map dropped
*<6>[    4.059678] acpi_os_unmap_memory: unlocked
*<6>[  122.761255] acpi_os_map_cleanup: after synchronize_rcu
*<6>[  122.767027] acpi_os_unmap_memory: end
*<6>[  122.771134] After unmap
*<6>[  122.773877] After ACPI_FREE
 <6>[  122.777010] acpi_ut_delete_internal_obj: after setup
 <6>[  122.782576] acpi_ut_update_object_reference: obj:ffff88003c0784c8 type:24
 <6>[  122.790183] acpi_ut_update_ref_count:locked count:1c action:1 flags:282
 <6>[  122.797595] acpi_ut_delete_internal_obj: handler_desc removed
 <6>[  122.804034] acpi_ut_delete_internal_obj: second_desc deleted
 <6>[  122.810376] acpi_ut_delete_internal_obj: object deleted
 <6>[  122.816231] acpi_ns_detach_object: reference removed
Note: The "*" marked acpi_os_unmap_memory() instrumentation messages are
      added by the reporter.

The patch converts RCU synchronization into the deferred executed work
queue item to eliminate the consumed time from the hot path. By doing so,
acpi_os_read_memory()/acpi_os_write_memory() need to hold the reference
count and the RCU locks will then be unnecessary. The original
acpi_ioremap_lock (renamed to acpi_map_lock) must be converted into
spinlock to be used for both the IRQ and the task contexts.
But it is still needed that the map operations are serialized because the
spinlock cannot be held around ioremap(), so that the parallel map
operations can wait for each other instead of inserting duplicated entries
into the acpi_ioremaps. This is achieved by introducing another
acpi_serial_map_mutex. Note that the unmap serialization is not required
due to the reference count protected list removal, thus
acpi_os_unmap_iomem() and acpi_map_relinquish() are not protected by the
acpi_serial_map_mutex.
The test feedback from the reporter:
 "The google reboot stability test passed with your 6 patches. Thanks for
  the help."

Known issues:
1. It is better to use a seperate work queue other than the ACPICA
   dedicated GPE work queue to perform mapping relinquishment. Until a real
   case can be seen on bugzilla.kernel.org complaining this, no further
   improvements are done in this patch.

Signed-off-by: Lv Zheng <lv.zheng@intel.com>
Reported-and-tested-by: Fei Yang <fei.yang@intel.com>
---
 drivers/acpi/mem.c |   87 ++++++++++++++++++++++++++++++++++------------------
 1 file changed, 58 insertions(+), 29 deletions(-)

diff --git a/drivers/acpi/mem.c b/drivers/acpi/mem.c
index c7c35f7..b631007 100644
--- a/drivers/acpi/mem.c
+++ b/drivers/acpi/mem.c
@@ -30,13 +30,13 @@ struct acpi_ioremap {
 	unsigned long refcount;
 };
 
+/* ioremap() serialization, no need to serialize iounmap() operations */
+static DEFINE_MUTEX(acpi_serial_map_mutex);
+
 static LIST_HEAD(acpi_ioremaps);
-static DEFINE_MUTEX(acpi_ioremap_lock);
+static DEFINE_SPINLOCK(acpi_map_lock);
 
-/*
- * The following functions must be called with 'acpi_ioremap_lock' or RCU
- * read lock held.
- */
+/* The following functions must be called with 'acpi_map_lock' held. */
 static inline void acpi_map_get(struct acpi_ioremap *map)
 {
 	map->refcount++;
@@ -45,7 +45,7 @@ static inline void acpi_map_get(struct acpi_ioremap *map)
 static inline void acpi_map_put(struct acpi_ioremap *map)
 {
 	if (!--map->refcount)
-		list_del_rcu(&map->list);
+		list_del(&map->list);
 }
 
 static struct acpi_ioremap *
@@ -53,7 +53,7 @@ acpi_map_lookup_phys(acpi_physical_address phys, acpi_size size)
 {
 	struct acpi_ioremap *map;
 
-	list_for_each_entry_rcu(map, &acpi_ioremaps, list)
+	list_for_each_entry(map, &acpi_ioremaps, list)
 		if (map->phys <= phys &&
 		    phys + size <= map->phys + map->size)
 			return map;
@@ -66,7 +66,7 @@ acpi_map_lookup_virt(void __iomem *virt, acpi_size size)
 {
 	struct acpi_ioremap *map;
 
-	list_for_each_entry_rcu(map, &acpi_ioremaps, list)
+	list_for_each_entry(map, &acpi_ioremaps, list)
 		if (map->virt <= virt &&
 		    virt + size <= map->virt + map->size)
 			return map;
@@ -80,6 +80,7 @@ acpi_map2virt(struct acpi_ioremap *map, acpi_physical_address phys)
 	return map ? map->virt + (phys - map->phys) : NULL;
 }
 
+/* The following functions must be called without 'acpi_map_lock' held. */
 #ifndef CONFIG_IA64
 #define should_use_kmap(pfn)   page_is_ram(pfn)
 #else
@@ -113,22 +114,33 @@ static void acpi_unmap(acpi_physical_address pg_off, void __iomem *vaddr)
 
 static void acpi_map_cleanup(struct acpi_ioremap *map)
 {
-	if (!map->refcount) {
-		synchronize_rcu();
+	if (map && !map->refcount) {
 		acpi_unmap(map->phys, map->virt);
 		kfree(map);
 	}
 }
 
+static void acpi_map_relinquish(void *ctx)
+{
+	struct acpi_ioremap *map = ctx;
+	unsigned long flags;
+
+	spin_lock_irqsave(&acpi_map_lock, flags);
+	acpi_map_put(map);
+	spin_unlock_irqrestore(&acpi_map_lock, flags);
+	acpi_map_cleanup(map);
+}
+
 void __iomem *acpi_os_get_iomem(acpi_physical_address phys, unsigned int size)
 {
 	struct acpi_ioremap *map;
+	unsigned long flags;
 
-	mutex_lock(&acpi_ioremap_lock);
+	spin_lock_irqsave(&acpi_map_lock, flags);
 	map = acpi_map_lookup_phys(phys, size);
 	if (map)
 		acpi_map_get(map);
-	mutex_unlock(&acpi_ioremap_lock);
+	spin_unlock_irqrestore(&acpi_map_lock, flags);
 
 	return acpi_map2virt(map, phys);
 }
@@ -141,6 +153,7 @@ acpi_os_map_iomem(acpi_physical_address phys, acpi_size size)
 	void __iomem *virt;
 	acpi_physical_address pg_off;
 	acpi_size pg_sz;
+	unsigned long flags;
 
 	if (phys > ULONG_MAX) {
 		pr_err(PREFIX "Cannot map memory that high\n");
@@ -150,17 +163,20 @@ acpi_os_map_iomem(acpi_physical_address phys, acpi_size size)
 	if (!acpi_gbl_permanent_mmap)
 		return __acpi_map_table((unsigned long)phys, size);
 
-	mutex_lock(&acpi_ioremap_lock);
+	mutex_lock(&acpi_serial_map_mutex);
+
+	spin_lock_irqsave(&acpi_map_lock, flags);
 	/* Check if there's a suitable mapping already. */
 	map = acpi_map_lookup_phys(phys, size);
 	if (map) {
 		acpi_map_get(map);
 		goto out;
 	}
+	spin_unlock_irqrestore(&acpi_map_lock, flags);
 
 	map = kzalloc(sizeof(*map), GFP_KERNEL);
 	if (!map)
-		goto out;
+		goto err_exit;
 
 	pg_off = round_down(phys, PAGE_SIZE);
 	pg_sz = round_up(phys + size, PAGE_SIZE) - pg_off;
@@ -168,17 +184,21 @@ acpi_os_map_iomem(acpi_physical_address phys, acpi_size size)
 	if (!virt) {
 		kfree(map);
 		map = NULL;
-		goto out;
+		goto err_exit;
 	}
 
+	spin_lock_irqsave(&acpi_map_lock, flags);
 	INIT_LIST_HEAD(&map->list);
 	map->virt = virt;
 	map->phys = pg_off;
 	map->size = pg_sz;
 	map->refcount = 1;
-	list_add_tail_rcu(&map->list, &acpi_ioremaps);
+	list_add_tail(&map->list, &acpi_ioremaps);
 out:
-	mutex_unlock(&acpi_ioremap_lock);
+	spin_unlock_irqrestore(&acpi_map_lock, flags);
+
+err_exit:
+	mutex_unlock(&acpi_serial_map_mutex);
 	return acpi_map2virt(map, phys);
 }
 EXPORT_SYMBOL_GPL(acpi_os_map_iomem);
@@ -186,19 +206,20 @@ EXPORT_SYMBOL_GPL(acpi_os_map_iomem);
 void __ref acpi_os_unmap_iomem(void __iomem *virt, acpi_size size)
 {
 	struct acpi_ioremap *map;
+	unsigned long flags;
 
 	if (!acpi_gbl_permanent_mmap) {
 		__acpi_unmap_table(virt, size);
 		return;
 	}
 
-	mutex_lock(&acpi_ioremap_lock);
+	spin_lock_irqsave(&acpi_map_lock, flags);
 	map = acpi_map_lookup_virt(virt, size);
 	if (map)
 		acpi_map_put(map);
 	else
 		WARN(true, PREFIX "%s: bad address %p\n", __func__, virt);
-	mutex_unlock(&acpi_ioremap_lock);
+	spin_unlock_irqrestore(&acpi_map_lock, flags);
 
 	acpi_map_cleanup(map);
 }
@@ -298,13 +319,16 @@ acpi_os_read_memory(acpi_physical_address phys_addr, u64 *value, u32 width)
 	bool unmap = false;
 	u64 dummy;
 	struct acpi_ioremap *map;
+	unsigned long flags;
 
-	rcu_read_lock();
+	spin_lock_irqsave(&acpi_map_lock, flags);
 	map = acpi_map_lookup_phys(phys_addr, size);
-	if (map)
+	if (map) {
+		acpi_map_get(map);
+		spin_unlock_irqrestore(&acpi_map_lock, flags);
 		virt_addr = acpi_map2virt(map, phys_addr);
-	else {
-		rcu_read_unlock();
+	} else {
+		spin_unlock_irqrestore(&acpi_map_lock, flags);
 		virt_addr = acpi_os_ioremap(phys_addr, size);
 		if (!virt_addr)
 			return AE_BAD_ADDRESS;
@@ -334,7 +358,8 @@ acpi_os_read_memory(acpi_physical_address phys_addr, u64 *value, u32 width)
 	if (unmap)
 		iounmap(virt_addr);
 	else
-		rcu_read_unlock();
+		acpi_os_execute(OSL_GPE_HANDLER,
+				acpi_map_relinquish, map);
 
 	return AE_OK;
 }
@@ -346,13 +371,16 @@ acpi_os_write_memory(acpi_physical_address phys_addr, u64 value, u32 width)
 	unsigned int size = width / 8;
 	bool unmap = false;
 	struct acpi_ioremap *map;
+	unsigned long flags;
 
-	rcu_read_lock();
+	spin_lock_irqsave(&acpi_map_lock, flags);
 	map = acpi_map_lookup_phys(phys_addr, size);
-	if (map)
+	if (map) {
+		acpi_map_get(map);
+		spin_unlock_irqrestore(&acpi_map_lock, flags);
 		virt_addr = acpi_map2virt(map, phys_addr);
-	else {
-		rcu_read_unlock();
+	} else {
+		spin_unlock_irqrestore(&acpi_map_lock, flags);
 		virt_addr = acpi_os_ioremap(phys_addr, size);
 		if (!virt_addr)
 			return AE_BAD_ADDRESS;
@@ -379,7 +407,8 @@ acpi_os_write_memory(acpi_physical_address phys_addr, u64 value, u32 width)
 	if (unmap)
 		iounmap(virt_addr);
 	else
-		rcu_read_unlock();
+		acpi_os_execute(OSL_GPE_HANDLER,
+				acpi_map_relinquish, map);
 
 	return AE_OK;
 }
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* RE: [PATCH 0/6] ACPI/OSL: Rework of ACPICA memory OSLs to improve performance.
  2014-10-23  2:11 [PATCH 0/6] ACPI/OSL: Rework of ACPICA memory OSLs to improve performance Lv Zheng
                   ` (5 preceding siblings ...)
  2014-10-23  2:12 ` [PATCH 6/6] ACPI/OSL: Fix performance issue in system memory lockings Lv Zheng
@ 2014-10-28  5:15 ` Zheng, Lv
  6 siblings, 0 replies; 8+ messages in thread
From: Zheng, Lv @ 2014-10-28  5:15 UTC (permalink / raw)
  To: Wysocki, Rafael J, Brown, Len
  Cc: Lv Zheng, linux-kernel, linux-acpi, Yang, Fei

There are reasons for why RCU is used here.
Using spinlock here will break specific acpi_os_read_memory()/acpi_os_write_memory() users.
So this patchset is not correct, please ignore.

Thanks
-Lv

> From: Zheng, Lv
> Sent: Thursday, October 23, 2014 10:12 AM
> 
> It is reported that there is a performance issue in the ACPICA OSL
> implementation around memory mappings.
> 
> On the reported platforms, there is a debugging facility implemented in the
> ACPI namespace using circular logging buffer:
>         Name (DPTR, 0x3AFEB000)
>         Name (EPTR, 0x3AFFB000)
>         Name (CPTR, 0x3AFEB010)
>         Mutex (MMUT, 0x00)
>         Method (MDBG, 1, Serialized)
>         {
>             Store (Acquire (MMUT, 0x03E8), Local0)
>             If (Local0 == Zero)
>             {
>                 OperationRegion (ABLK, SystemMemory, CPTR, 0x10)
>                 Field (ABLK, ByteAcc, NoLock, Preserve)
>                 {
>                     AAAA,   128
>                 }
>                 Store (Arg0, AAAA) /* \MDBG.AAAA */
>                 CPTR = (CPTR + 0x10) /* \CPTR */
>                 If (CPTR >= EPTR)
>                 {
>                     CPTR = (DPTR + 0x10) /* \CPTR */
>                 }
>                 Release (MMUT)
>             }
>             Return (Local0)
>         }
> This function is heavily invoked by other ACPI control methods.
> The reported platforms suffer from 2-5 minutes kernel stuck in the end of
> accessing the mapped circular buffer system memory region. By
> instrumentation, the 2-5 minutes time consumption overhead can be seen to
> happen on the synchronize_rcu() invoked in the acpi_os_unmap_memory().
> 
> This patchset removes the synchronize_rcu() from the hot path to improve
> the performance.
> 
> Lv Zheng (6):
>   ACPI/OSL: Split memory operation region implementations to a seperate
>     file.
>   ACPI/OSL: Rename system memory functions.
>   ACPI/OSL: Cleanup memory access functions by merging duplicate code.
>   ACPI/OSL: Add acpi_map2virt() to merge duplicate code.
>   ACPI/OSL: Cleanup branch logics.
>   ACPI/OSL: Fix performance issue in system memory lockings.
> 
>  drivers/acpi/Makefile |    2 +-
>  drivers/acpi/mem.c    |  414 +++++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/acpi/osl.c    |  381 ---------------------------------------------
>  3 files changed, 415 insertions(+), 382 deletions(-)
>  create mode 100644 drivers/acpi/mem.c
> 
> --
> 1.7.10


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2014-10-28  5:17 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-23  2:11 [PATCH 0/6] ACPI/OSL: Rework of ACPICA memory OSLs to improve performance Lv Zheng
2014-10-23  2:12 ` [PATCH 1/6] ACPI/OSL: Split memory operation region implementations to a seperate file Lv Zheng
2014-10-23  2:12 ` [PATCH 2/6] ACPI/OSL: Rename system memory functions Lv Zheng
2014-10-23  2:12 ` [PATCH 3/6] ACPI/OSL: Cleanup memory access functions by merging duplicate code Lv Zheng
2014-10-23  2:12 ` [PATCH 4/6] ACPI/OSL: Add acpi_map2virt() to merge " Lv Zheng
2014-10-23  2:12 ` [PATCH 5/6] ACPI/OSL: Cleanup branch logics Lv Zheng
2014-10-23  2:12 ` [PATCH 6/6] ACPI/OSL: Fix performance issue in system memory lockings Lv Zheng
2014-10-28  5:15 ` [PATCH 0/6] ACPI/OSL: Rework of ACPICA memory OSLs to improve performance Zheng, Lv

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).