[PATCH v4 0/3] introduce static_vm for ARM-specific static mapped area

[PATCH v4 0/3] introduce static_vm for ARM-specific static mapped area

[Joonsoo Kim]

In current implementation, we used ARM-specific flag, that is,
VM_ARM_STATIC_MAPPING, for distinguishing ARM specific static mapped area.
The purpose of static mapped area is to re-use static mapped area when
entire physical address range of the ioremap request can be covered
by this area.

This implementation causes needless overhead for some cases.
For example, assume that there is only one static mapped area and
vmlist has 300 areas. Every time we call ioremap, we check 300 areas for
deciding whether it is matched or not. Moreover, even if there is
no static mapped area and vmlist has 300 areas, every time we call
ioremap, we check 300 areas in now.

If we construct a extra list for static mapped area, we can eliminate
above mentioned overhead.
With a extra list, if there is one static mapped area,
we just check only one area and proceed next operation quickly.

In fact, it is not a critical problem, because ioremap is not frequently
used. But reducing overhead is better idea.

Another reason for doing this work is for removing vm_struct list management,
entirely. For more information, look at the following link.
http://lkml.org/lkml/2012/12/6/184

Changelog
v3->v4:
  [2/3]: Changes from Nicolas' suggestion
         - embed static_vm code in ioremap.c
         - simplify struct static_vm
         - remove init_static_vm, instead, add_static_vm_early() init static_vm
         Use generic list for list management of static_vm
         Convert spin_lock to rw_lock
         Modify static_vm's flags bits
  [3/3]: Rework according to [2/3] change
  Rebased on v3.8-rc5

v2->v3:
  coverletter: refer a link related to this work
  [2/3]: drop @flags of find_static_vm_vaddr
  Rebased on v3.8-rc4

v1->v2: 
  [2/3]: patch description is improved.
  Rebased on v3.7-rc7

Joonsoo Kim (3):
  ARM: vmregion: remove vmregion code entirely
  ARM: ioremap: introduce an infrastructure for static mapped area
  ARM: mm: use static_vm for managing static mapped areas

 arch/arm/mm/Makefile   |    2 +-
 arch/arm/mm/ioremap.c  |  147 +++++++++++++++++++++++-----------
 arch/arm/mm/mm.h       |   28 ++++---
 arch/arm/mm/mmu.c      |   47 ++++++-----
 arch/arm/mm/vmregion.c |  205 ------------------------------------------------
 arch/arm/mm/vmregion.h |   31 --------
 6 files changed, 147 insertions(+), 313 deletions(-)
 delete mode 100644 arch/arm/mm/vmregion.c
 delete mode 100644 arch/arm/mm/vmregion.h

-- 
1.7.9.5

[PATCH v4 1/3] ARM: vmregion: remove vmregion code entirely

[Joonsoo Kim]

Now, there is no user for vmregion.
So remove it.

Acked-by: Nicolas Pitre <nico@linaro.org>
Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>

diff --git a/arch/arm/mm/Makefile b/arch/arm/mm/Makefile
index 8a9c4cb..4e333fa 100644
--- a/arch/arm/mm/Makefile
+++ b/arch/arm/mm/Makefile
@@ -6,7 +6,7 @@ obj-y				:= dma-mapping.o extable.o fault.o init.o \
 				   iomap.o
 
 obj-$(CONFIG_MMU)		+= fault-armv.o flush.o idmap.o ioremap.o \
-				   mmap.o pgd.o mmu.o vmregion.o
+				   mmap.o pgd.o mmu.o
 
 ifneq ($(CONFIG_MMU),y)
 obj-y				+= nommu.o
diff --git a/arch/arm/mm/vmregion.c b/arch/arm/mm/vmregion.c
deleted file mode 100644
index a631016..0000000
--- a/arch/arm/mm/vmregion.c
+++ /dev/null
@@ -1,205 +0,0 @@
-#include <linux/fs.h>
-#include <linux/spinlock.h>
-#include <linux/list.h>
-#include <linux/proc_fs.h>
-#include <linux/seq_file.h>
-#include <linux/slab.h>
-
-#include "vmregion.h"
-
-/*
- * VM region handling support.
- *
- * This should become something generic, handling VM region allocations for
- * vmalloc and similar (ioremap, module space, etc).
- *
- * I envisage vmalloc()'s supporting vm_struct becoming:
- *
- *  struct vm_struct {
- *    struct vmregion	region;
- *    unsigned long	flags;
- *    struct page	**pages;
- *    unsigned int	nr_pages;
- *    unsigned long	phys_addr;
- *  };
- *
- * get_vm_area() would then call vmregion_alloc with an appropriate
- * struct vmregion head (eg):
- *
- *  struct vmregion vmalloc_head = {
- *	.vm_list	= LIST_HEAD_INIT(vmalloc_head.vm_list),
- *	.vm_start	= VMALLOC_START,
- *	.vm_end		= VMALLOC_END,
- *  };
- *
- * However, vmalloc_head.vm_start is variable (typically, it is dependent on
- * the amount of RAM found at boot time.)  I would imagine that get_vm_area()
- * would have to initialise this each time prior to calling vmregion_alloc().
- */
-
-struct arm_vmregion *
-arm_vmregion_alloc(struct arm_vmregion_head *head, size_t align,
-		   size_t size, gfp_t gfp, const void *caller)
-{
-	unsigned long start = head->vm_start, addr = head->vm_end;
-	unsigned long flags;
-	struct arm_vmregion *c, *new;
-
-	if (head->vm_end - head->vm_start < size) {
-		printk(KERN_WARNING "%s: allocation too big (requested %#x)\n",
-			__func__, size);
-		goto out;
-	}
-
-	new = kmalloc(sizeof(struct arm_vmregion), gfp);
-	if (!new)
-		goto out;
-
-	new->caller = caller;
-
-	spin_lock_irqsave(&head->vm_lock, flags);
-
-	addr = rounddown(addr - size, align);
-	list_for_each_entry_reverse(c, &head->vm_list, vm_list) {
-		if (addr >= c->vm_end)
-			goto found;
-		addr = rounddown(c->vm_start - size, align);
-		if (addr < start)
-			goto nospc;
-	}
-
- found:
-	/*
-	 * Insert this entry after the one we found.
-	 */
-	list_add(&new->vm_list, &c->vm_list);
-	new->vm_start = addr;
-	new->vm_end = addr + size;
-	new->vm_active = 1;
-
-	spin_unlock_irqrestore(&head->vm_lock, flags);
-	return new;
-
- nospc:
-	spin_unlock_irqrestore(&head->vm_lock, flags);
-	kfree(new);
- out:
-	return NULL;
-}
-
-static struct arm_vmregion *__arm_vmregion_find(struct arm_vmregion_head *head, unsigned long addr)
-{
-	struct arm_vmregion *c;
-
-	list_for_each_entry(c, &head->vm_list, vm_list) {
-		if (c->vm_active && c->vm_start == addr)
-			goto out;
-	}
-	c = NULL;
- out:
-	return c;
-}
-
-struct arm_vmregion *arm_vmregion_find(struct arm_vmregion_head *head, unsigned long addr)
-{
-	struct arm_vmregion *c;
-	unsigned long flags;
-
-	spin_lock_irqsave(&head->vm_lock, flags);
-	c = __arm_vmregion_find(head, addr);
-	spin_unlock_irqrestore(&head->vm_lock, flags);
-	return c;
-}
-
-struct arm_vmregion *arm_vmregion_find_remove(struct arm_vmregion_head *head, unsigned long addr)
-{
-	struct arm_vmregion *c;
-	unsigned long flags;
-
-	spin_lock_irqsave(&head->vm_lock, flags);
-	c = __arm_vmregion_find(head, addr);
-	if (c)
-		c->vm_active = 0;
-	spin_unlock_irqrestore(&head->vm_lock, flags);
-	return c;
-}
-
-void arm_vmregion_free(struct arm_vmregion_head *head, struct arm_vmregion *c)
-{
-	unsigned long flags;
-
-	spin_lock_irqsave(&head->vm_lock, flags);
-	list_del(&c->vm_list);
-	spin_unlock_irqrestore(&head->vm_lock, flags);
-
-	kfree(c);
-}
-
-#ifdef CONFIG_PROC_FS
-static int arm_vmregion_show(struct seq_file *m, void *p)
-{
-	struct arm_vmregion *c = list_entry(p, struct arm_vmregion, vm_list);
-
-	seq_printf(m, "0x%08lx-0x%08lx %7lu", c->vm_start, c->vm_end,
-		c->vm_end - c->vm_start);
-	if (c->caller)
-		seq_printf(m, " %pS", (void *)c->caller);
-	seq_putc(m, '\n');
-	return 0;
-}
-
-static void *arm_vmregion_start(struct seq_file *m, loff_t *pos)
-{
-	struct arm_vmregion_head *h = m->private;
-	spin_lock_irq(&h->vm_lock);
-	return seq_list_start(&h->vm_list, *pos);
-}
-
-static void *arm_vmregion_next(struct seq_file *m, void *p, loff_t *pos)
-{
-	struct arm_vmregion_head *h = m->private;
-	return seq_list_next(p, &h->vm_list, pos);
-}
-
-static void arm_vmregion_stop(struct seq_file *m, void *p)
-{
-	struct arm_vmregion_head *h = m->private;
-	spin_unlock_irq(&h->vm_lock);
-}
-
-static const struct seq_operations arm_vmregion_ops = {
-	.start	= arm_vmregion_start,
-	.stop	= arm_vmregion_stop,
-	.next	= arm_vmregion_next,
-	.show	= arm_vmregion_show,
-};
-
-static int arm_vmregion_open(struct inode *inode, struct file *file)
-{
-	struct arm_vmregion_head *h = PDE(inode)->data;
-	int ret = seq_open(file, &arm_vmregion_ops);
-	if (!ret) {
-		struct seq_file *m = file->private_data;
-		m->private = h;
-	}
-	return ret;
-}
-
-static const struct file_operations arm_vmregion_fops = {
-	.open	= arm_vmregion_open,
-	.read	= seq_read,
-	.llseek	= seq_lseek,
-	.release = seq_release,
-};
-
-int arm_vmregion_create_proc(const char *path, struct arm_vmregion_head *h)
-{
-	proc_create_data(path, S_IRUSR, NULL, &arm_vmregion_fops, h);
-	return 0;
-}
-#else
-int arm_vmregion_create_proc(const char *path, struct arm_vmregion_head *h)
-{
-	return 0;
-}
-#endif
diff --git a/arch/arm/mm/vmregion.h b/arch/arm/mm/vmregion.h
deleted file mode 100644
index 0f5a5f2..0000000
--- a/arch/arm/mm/vmregion.h
+++ /dev/null
@@ -1,31 +0,0 @@
-#ifndef VMREGION_H
-#define VMREGION_H
-
-#include <linux/spinlock.h>
-#include <linux/list.h>
-
-struct page;
-
-struct arm_vmregion_head {
-	spinlock_t		vm_lock;
-	struct list_head	vm_list;
-	unsigned long		vm_start;
-	unsigned long		vm_end;
-};
-
-struct arm_vmregion {
-	struct list_head	vm_list;
-	unsigned long		vm_start;
-	unsigned long		vm_end;
-	int			vm_active;
-	const void		*caller;
-};
-
-struct arm_vmregion *arm_vmregion_alloc(struct arm_vmregion_head *, size_t, size_t, gfp_t, const void *);
-struct arm_vmregion *arm_vmregion_find(struct arm_vmregion_head *, unsigned long);
-struct arm_vmregion *arm_vmregion_find_remove(struct arm_vmregion_head *, unsigned long);
-void arm_vmregion_free(struct arm_vmregion_head *, struct arm_vmregion *);
-
-int arm_vmregion_create_proc(const char *, struct arm_vmregion_head *);
-
-#endif
-- 
1.7.9.5

[PATCH v4 2/3] ARM: ioremap: introduce an infrastructure for static mapped area

[Joonsoo Kim]

In current implementation, we used ARM-specific flag, that is,
VM_ARM_STATIC_MAPPING, for distinguishing ARM specific static mapped area.
The purpose of static mapped area is to re-use static mapped area when
entire physical address range of the ioremap request can be covered
by this area.

This implementation causes needless overhead for some cases.
For example, assume that there is only one static mapped area and
vmlist has 300 areas. Every time we call ioremap, we check 300 areas for
deciding whether it is matched or not. Moreover, even if there is
no static mapped area and vmlist has 300 areas, every time we call
ioremap, we check 300 areas in now.

If we construct a extra list for static mapped area, we can eliminate
above mentioned overhead.
With a extra list, if there is one static mapped area,
we just check only one area and proceed next operation quickly.

In fact, it is not a critical problem, because ioremap is not frequently
used. But reducing overhead is better idea.

Another reason for doing this work is for removing architecture dependency
on vmalloc layer. I think that vmlist and vmlist_lock is internal data
structure for vmalloc layer. Some codes for debugging and stat inevitably
use vmlist and vmlist_lock. But it is preferable that they are used
as least as possible in outside of vmalloc.c

Now, I introduce an ARM-specific infrastructure for static mapped area. In
the following patch, we will use this and resolve above mentioned problem.

Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>

diff --git a/arch/arm/mm/ioremap.c b/arch/arm/mm/ioremap.c
index 88fd86c..ceb34ae 100644
--- a/arch/arm/mm/ioremap.c
+++ b/arch/arm/mm/ioremap.c
@@ -39,6 +39,78 @@
 #include <asm/mach/pci.h>
 #include "mm.h"
 
+
+LIST_HEAD(static_vmlist);
+static DEFINE_RWLOCK(static_vmlist_lock);
+
+static struct static_vm *find_static_vm_paddr(phys_addr_t paddr,
+			size_t size, unsigned long flags)
+{
+	struct static_vm *svm;
+	struct vm_struct *vm;
+
+	read_lock(&static_vmlist_lock);
+	list_for_each_entry(svm, &static_vmlist, list) {
+		if (svm->flags != flags)
+			continue;
+
+		vm = &svm->vm;
+		if (vm->phys_addr > paddr ||
+			paddr + size - 1 > vm->phys_addr + vm->size - 1)
+			continue;
+
+		read_unlock(&static_vmlist_lock);
+		return svm;
+	}
+
+	return NULL;
+}
+
+struct static_vm *find_static_vm_vaddr(void *vaddr)
+{
+	struct static_vm *svm;
+	struct vm_struct *vm;
+
+	read_lock(&static_vmlist_lock);
+	list_for_each_entry(svm, &static_vmlist, list) {
+		vm = &svm->vm;
+
+		/* static_vmlist is ascending order */
+		if (vm->addr > vaddr)
+			break;
+
+		if (vm->addr <= vaddr && vm->addr + vm->size > vaddr) {
+			read_unlock(&static_vmlist_lock);
+			return svm;
+		}
+	}
+	read_unlock(&static_vmlist_lock);
+
+	return NULL;
+}
+
+void add_static_vm_early(struct static_vm *svm, unsigned long flags)
+{
+	struct static_vm *curr_svm;
+	struct vm_struct *vm;
+	void *vaddr;
+
+	vm_area_add_early(&svm->vm);
+
+	vaddr = svm->vm.addr;
+	svm->flags = flags;
+
+	write_lock(&static_vmlist_lock);
+	list_for_each_entry(curr_svm, &static_vmlist, list) {
+		vm = &curr_svm->vm;
+
+		if (vm->addr > vaddr)
+			break;
+	}
+	list_add_tail(&svm->list, &curr_svm->list);
+	write_unlock(&static_vmlist_lock);
+}
+
 int ioremap_page(unsigned long virt, unsigned long phys,
 		 const struct mem_type *mtype)
 {
diff --git a/arch/arm/mm/mm.h b/arch/arm/mm/mm.h
index a8ee92d..fb45c79 100644
--- a/arch/arm/mm/mm.h
+++ b/arch/arm/mm/mm.h
@@ -1,4 +1,6 @@
 #ifdef CONFIG_MMU
+#include <linux/list.h>
+#include <linux/vmalloc.h>
 
 /* the upper-most page table pointer */
 extern pmd_t *top_pmd;
@@ -65,6 +67,24 @@ extern void __flush_dcache_page(struct address_space *mapping, struct page *page
 /* consistent regions used by dma_alloc_attrs() */
 #define VM_ARM_DMA_CONSISTENT	0x20000000
 
+
+/* ARM specific static_vm->flags bits */
+#define STATIC_VM_MEM		0x00000001
+#define STATIC_VM_EMPTY		0x00000002
+#define STATIC_VM_MTYPE(mtype)	((mtype) << 20)
+
+#define STATIC_VM_TYPE(type, mtype) (type | STATIC_VM_MTYPE(mtype))
+
+struct static_vm {
+	struct vm_struct vm;
+	struct list_head list;
+	unsigned long flags;
+};
+
+extern struct list_head static_vmlist;
+extern struct static_vm *find_static_vm_vaddr(void *vaddr);
+extern void add_static_vm_early(struct static_vm *svm, unsigned long flags);
+
 #endif
 
 #ifdef CONFIG_ZONE_DMA
-- 
1.7.9.5

[PATCH v4 3/3] ARM: mm: use static_vm for managing static mapped areas

[Joonsoo Kim]

A static mapped area is ARM-specific, so it is better not to use
generic vmalloc data structure, that is, vmlist and vmlist_lock
for managing static mapped area. And it causes some needless overhead and
reducing this overhead is better idea.

Now, we have newly introduced static_vm infrastructure.
With it, we don't need to iterate all mapped areas. Instead, we just
iterate static mapped areas. It helps to reduce an overhead of finding
matched area. And architecture dependency on vmalloc layer is removed,
so it will help to maintainability for vmalloc layer.

Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>

diff --git a/arch/arm/mm/ioremap.c b/arch/arm/mm/ioremap.c
index ceb34ae..7fe5b48 100644
--- a/arch/arm/mm/ioremap.c
+++ b/arch/arm/mm/ioremap.c
@@ -269,13 +269,14 @@ void __iomem * __arm_ioremap_pfn_caller(unsigned long pfn,
 	const struct mem_type *type;
 	int err;
 	unsigned long addr;
- 	struct vm_struct * area;
+	struct vm_struct *area;
+	phys_addr_t paddr = __pfn_to_phys(pfn);
 
 #ifndef CONFIG_ARM_LPAE
 	/*
 	 * High mappings must be supersection aligned
 	 */
-	if (pfn >= 0x100000 && (__pfn_to_phys(pfn) & ~SUPERSECTION_MASK))
+	if (pfn >= 0x100000 && (paddr & ~SUPERSECTION_MASK))
 		return NULL;
 #endif
 
@@ -291,24 +292,17 @@ void __iomem * __arm_ioremap_pfn_caller(unsigned long pfn,
 	/*
 	 * Try to reuse one of the static mapping whenever possible.
 	 */
-	read_lock(&vmlist_lock);
-	for (area = vmlist; area; area = area->next) {
-		if (!size || (sizeof(phys_addr_t) == 4 && pfn >= 0x100000))
-			break;
-		if (!(area->flags & VM_ARM_STATIC_MAPPING))
-			continue;
-		if ((area->flags & VM_ARM_MTYPE_MASK) != VM_ARM_MTYPE(mtype))
-			continue;
-		if (__phys_to_pfn(area->phys_addr) > pfn ||
-		    __pfn_to_phys(pfn) + size-1 > area->phys_addr + area->size-1)
-			continue;
-		/* we can drop the lock here as we know *area is static */
-		read_unlock(&vmlist_lock);
-		addr = (unsigned long)area->addr;
-		addr += __pfn_to_phys(pfn) - area->phys_addr;
-		return (void __iomem *) (offset + addr);
+	if (size && !((sizeof(phys_addr_t) == 4 && pfn >= 0x100000))) {
+		struct static_vm *svm;
+
+		svm = find_static_vm_paddr(paddr, size,
+				STATIC_VM_TYPE(STATIC_VM_MEM, mtype));
+		if (svm) {
+			addr = (unsigned long)svm->vm.addr;
+			addr += paddr - svm->vm.phys_addr;
+			return (void __iomem *) (offset + addr);
+		}
 	}
-	read_unlock(&vmlist_lock);
 
 	/*
 	 * Don't allow RAM to be mapped - this causes problems with ARMv6+
@@ -320,21 +314,21 @@ void __iomem * __arm_ioremap_pfn_caller(unsigned long pfn,
  	if (!area)
  		return NULL;
  	addr = (unsigned long)area->addr;
-	area->phys_addr = __pfn_to_phys(pfn);
+	area->phys_addr = paddr;
 
 #if !defined(CONFIG_SMP) && !defined(CONFIG_ARM_LPAE)
 	if (DOMAIN_IO == 0 &&
 	    (((cpu_architecture() >= CPU_ARCH_ARMv6) && (get_cr() & CR_XP)) ||
 	       cpu_is_xsc3()) && pfn >= 0x100000 &&
-	       !((__pfn_to_phys(pfn) | size | addr) & ~SUPERSECTION_MASK)) {
+	       !((paddr | size | addr) & ~SUPERSECTION_MASK)) {
 		area->flags |= VM_ARM_SECTION_MAPPING;
 		err = remap_area_supersections(addr, pfn, size, type);
-	} else if (!((__pfn_to_phys(pfn) | size | addr) & ~PMD_MASK)) {
+	} else if (!((paddr | size | addr) & ~PMD_MASK)) {
 		area->flags |= VM_ARM_SECTION_MAPPING;
 		err = remap_area_sections(addr, pfn, size, type);
 	} else
 #endif
-		err = ioremap_page_range(addr, addr + size, __pfn_to_phys(pfn),
+		err = ioremap_page_range(addr, addr + size, paddr,
 					 __pgprot(type->prot_pte));
 
 	if (err) {
@@ -418,34 +412,21 @@ __arm_ioremap_exec(unsigned long phys_addr, size_t size, bool cached)
 void __iounmap(volatile void __iomem *io_addr)
 {
 	void *addr = (void *)(PAGE_MASK & (unsigned long)io_addr);
-	struct vm_struct *vm;
+	struct static_vm *svm;
+
+	svm = find_static_vm_vaddr(addr);
+	if (svm)
+		return;
 
-	read_lock(&vmlist_lock);
-	for (vm = vmlist; vm; vm = vm->next) {
-		if (vm->addr > addr)
-			break;
-		if (!(vm->flags & VM_IOREMAP))
-			continue;
-		/* If this is a static mapping we must leave it alone */
-		if ((vm->flags & VM_ARM_STATIC_MAPPING) &&
-		    (vm->addr <= addr) && (vm->addr + vm->size > addr)) {
-			read_unlock(&vmlist_lock);
-			return;
-		}
 #if !defined(CONFIG_SMP) && !defined(CONFIG_ARM_LPAE)
-		/*
-		 * If this is a section based mapping we need to handle it
-		 * specially as the VM subsystem does not know how to handle
-		 * such a beast.
-		 */
-		if ((vm->addr == addr) &&
-		    (vm->flags & VM_ARM_SECTION_MAPPING)) {
+	{
+		struct vm_struct *vm;
+
+		vm = find_vm_area(addr);
+		if (vm && (vm->flags & VM_ARM_SECTION_MAPPING))
 			unmap_area_sections((unsigned long)vm->addr, vm->size);
-			break;
-		}
-#endif
 	}
-	read_unlock(&vmlist_lock);
+#endif
 
 	vunmap(addr);
 }
diff --git a/arch/arm/mm/mm.h b/arch/arm/mm/mm.h
index fb45c79..24c1df4 100644
--- a/arch/arm/mm/mm.h
+++ b/arch/arm/mm/mm.h
@@ -54,16 +54,6 @@ extern void __flush_dcache_page(struct address_space *mapping, struct page *page
 /* (super)section-mapped I/O regions used by ioremap()/iounmap() */
 #define VM_ARM_SECTION_MAPPING	0x80000000
 
-/* permanent static mappings from iotable_init() */
-#define VM_ARM_STATIC_MAPPING	0x40000000
-
-/* empty mapping */
-#define VM_ARM_EMPTY_MAPPING	0x20000000
-
-/* mapping type (attributes) for permanent static mappings */
-#define VM_ARM_MTYPE(mt)		((mt) << 20)
-#define VM_ARM_MTYPE_MASK	(0x1f << 20)
-
 /* consistent regions used by dma_alloc_attrs() */
 #define VM_ARM_DMA_CONSISTENT	0x20000000
 
diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index ce328c7..a7dc9a2 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -757,21 +757,24 @@ void __init iotable_init(struct map_desc *io_desc, int nr)
 {
 	struct map_desc *md;
 	struct vm_struct *vm;
+	struct static_vm *svm;
 
 	if (!nr)
 		return;
 
-	vm = early_alloc_aligned(sizeof(*vm) * nr, __alignof__(*vm));
+	svm = early_alloc_aligned(sizeof(*svm) * nr, __alignof__(*svm));
 
 	for (md = io_desc; nr; md++, nr--) {
 		create_mapping(md);
+
+		vm = &svm->vm;
 		vm->addr = (void *)(md->virtual & PAGE_MASK);
 		vm->size = PAGE_ALIGN(md->length + (md->virtual & ~PAGE_MASK));
 		vm->phys_addr = __pfn_to_phys(md->pfn);
-		vm->flags = VM_IOREMAP | VM_ARM_STATIC_MAPPING;
-		vm->flags |= VM_ARM_MTYPE(md->type);
+		vm->flags = VM_IOREMAP;
 		vm->caller = iotable_init;
-		vm_area_add_early(vm++);
+		add_static_vm_early(svm++,
+			STATIC_VM_TYPE(STATIC_VM_MEM, md->type));
 	}
 }
 
@@ -779,13 +782,16 @@ void __init vm_reserve_area_early(unsigned long addr, unsigned long size,
 				  void *caller)
 {
 	struct vm_struct *vm;
+	struct static_vm *svm;
+
+	svm = early_alloc_aligned(sizeof(*svm), __alignof__(*svm));
 
-	vm = early_alloc_aligned(sizeof(*vm), __alignof__(*vm));
+	vm = &svm->vm;
 	vm->addr = (void *)addr;
 	vm->size = size;
-	vm->flags = VM_IOREMAP | VM_ARM_EMPTY_MAPPING;
+	vm->flags = VM_IOREMAP;
 	vm->caller = caller;
-	vm_area_add_early(vm);
+	add_static_vm_early(svm, STATIC_VM_TYPE(STATIC_VM_EMPTY, 0));
 }
 
 #ifndef CONFIG_ARM_LPAE
@@ -810,14 +816,20 @@ static void __init pmd_empty_section_gap(unsigned long addr)
 
 static void __init fill_pmd_gaps(void)
 {
+	struct static_vm *svm;
 	struct vm_struct *vm;
 	unsigned long addr, next = 0;
 	pmd_t *pmd;
 
-	/* we're still single threaded hence no lock needed here */
-	for (vm = vmlist; vm; vm = vm->next) {
-		if (!(vm->flags & (VM_ARM_STATIC_MAPPING | VM_ARM_EMPTY_MAPPING)))
-			continue;
+	/*
+	 * We should not take a lock here, because pmd_empty_section_gap()
+	 * invokes vm_reserve_area_early(), and then it call
+	 * add_static_vm_early() which try to take a lock.
+	 * We're still single thread, so traverse whole list without a lock
+	 * is safe for now. And inserting new entry is also safe.
+	 */
+	list_for_each_entry(svm, &static_vmlist, list) {
+		vm = &svm->vm;
 		addr = (unsigned long)vm->addr;
 		if (addr < next)
 			continue;
@@ -859,17 +871,12 @@ static void __init pci_reserve_io(void)
 {
 	struct vm_struct *vm;
 	unsigned long addr;
+	struct static_vm *svm;
 
-	/* we're still single threaded hence no lock needed here */
-	for (vm = vmlist; vm; vm = vm->next) {
-		if (!(vm->flags & VM_ARM_STATIC_MAPPING))
-			continue;
-		addr = (unsigned long)vm->addr;
-		addr &= ~(SZ_2M - 1);
-		if (addr == PCI_IO_VIRT_BASE)
-			return;
+	svm = find_static_vm_vaddr((void *)PCI_IO_VIRT_BASE);
+	if (svm)
+		return;
 
-	}
 	vm_reserve_area_early(PCI_IO_VIRT_BASE, SZ_2M, pci_reserve_io);
 }
 #else
-- 
1.7.9.5

Re: [PATCH v4 2/3] ARM: ioremap: introduce an infrastructure for static mapped area

[Nicolas Pitre]

On Thu, 31 Jan 2013, Joonsoo Kim wrote:

> In current implementation, we used ARM-specific flag, that is,
> VM_ARM_STATIC_MAPPING, for distinguishing ARM specific static mapped area.
> The purpose of static mapped area is to re-use static mapped area when
> entire physical address range of the ioremap request can be covered
> by this area.
> 
> This implementation causes needless overhead for some cases.
> For example, assume that there is only one static mapped area and
> vmlist has 300 areas. Every time we call ioremap, we check 300 areas for
> deciding whether it is matched or not. Moreover, even if there is
> no static mapped area and vmlist has 300 areas, every time we call
> ioremap, we check 300 areas in now.
> 
> If we construct a extra list for static mapped area, we can eliminate
> above mentioned overhead.
> With a extra list, if there is one static mapped area,
> we just check only one area and proceed next operation quickly.
> 
> In fact, it is not a critical problem, because ioremap is not frequently
> used. But reducing overhead is better idea.
> 
> Another reason for doing this work is for removing architecture dependency
> on vmalloc layer. I think that vmlist and vmlist_lock is internal data
> structure for vmalloc layer. Some codes for debugging and stat inevitably
> use vmlist and vmlist_lock. But it is preferable that they are used
> as least as possible in outside of vmalloc.c
> 
> Now, I introduce an ARM-specific infrastructure for static mapped area. In
> the following patch, we will use this and resolve above mentioned problem.
> 
> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>

Much better.  Comments below.

> diff --git a/arch/arm/mm/ioremap.c b/arch/arm/mm/ioremap.c
> index 88fd86c..ceb34ae 100644
> --- a/arch/arm/mm/ioremap.c
> +++ b/arch/arm/mm/ioremap.c
> @@ -39,6 +39,78 @@
>  #include <asm/mach/pci.h>
>  #include "mm.h"
>  
> +
> +LIST_HEAD(static_vmlist);
> +static DEFINE_RWLOCK(static_vmlist_lock);

In fact you don't need a lock at all.  The only writer is 
add_static_vm_early() and we know it is only used during boot when the 
kernel is still single-threaded.

> +
> +static struct static_vm *find_static_vm_paddr(phys_addr_t paddr,
> +			size_t size, unsigned long flags)
> +{
> +	struct static_vm *svm;
> +	struct vm_struct *vm;
> +
> +	read_lock(&static_vmlist_lock);
> +	list_for_each_entry(svm, &static_vmlist, list) {
> +		if (svm->flags != flags)
> +			continue;
> +
> +		vm = &svm->vm;
> +		if (vm->phys_addr > paddr ||
> +			paddr + size - 1 > vm->phys_addr + vm->size - 1)
> +			continue;
> +
> +		read_unlock(&static_vmlist_lock);
> +		return svm;
> +	}
> +
> +	return NULL;
> +}
> +
> +struct static_vm *find_static_vm_vaddr(void *vaddr)
> +{
> +	struct static_vm *svm;
> +	struct vm_struct *vm;
> +
> +	read_lock(&static_vmlist_lock);
> +	list_for_each_entry(svm, &static_vmlist, list) {
> +		vm = &svm->vm;
> +
> +		/* static_vmlist is ascending order */
> +		if (vm->addr > vaddr)
> +			break;
> +
> +		if (vm->addr <= vaddr && vm->addr + vm->size > vaddr) {
> +			read_unlock(&static_vmlist_lock);
> +			return svm;
> +		}
> +	}
> +	read_unlock(&static_vmlist_lock);
> +
> +	return NULL;
> +}
> +
> +void add_static_vm_early(struct static_vm *svm, unsigned long flags)

This should be marked with __init.  This way, it is less likely to be 
used after boot, especially with no locking.  And vm_area_add_early() is 
valid only if !vmap_initialized anyway, and also __init.

> +{
> +	struct static_vm *curr_svm;
> +	struct vm_struct *vm;
> +	void *vaddr;
> +
> +	vm_area_add_early(&svm->vm);
> +
> +	vaddr = svm->vm.addr;
> +	svm->flags = flags;
> +
> +	write_lock(&static_vmlist_lock);
> +	list_for_each_entry(curr_svm, &static_vmlist, list) {
> +		vm = &curr_svm->vm;
> +
> +		if (vm->addr > vaddr)
> +			break;
> +	}
> +	list_add_tail(&svm->list, &curr_svm->list);
> +	write_unlock(&static_vmlist_lock);
> +}
> +
>  int ioremap_page(unsigned long virt, unsigned long phys,
>  		 const struct mem_type *mtype)
>  {
> diff --git a/arch/arm/mm/mm.h b/arch/arm/mm/mm.h
> index a8ee92d..fb45c79 100644
> --- a/arch/arm/mm/mm.h
> +++ b/arch/arm/mm/mm.h
> @@ -1,4 +1,6 @@
>  #ifdef CONFIG_MMU
> +#include <linux/list.h>
> +#include <linux/vmalloc.h>
>  
>  /* the upper-most page table pointer */
>  extern pmd_t *top_pmd;
> @@ -65,6 +67,24 @@ extern void __flush_dcache_page(struct address_space *mapping, struct page *page
>  /* consistent regions used by dma_alloc_attrs() */
>  #define VM_ARM_DMA_CONSISTENT	0x20000000
>  
> +
> +/* ARM specific static_vm->flags bits */
> +#define STATIC_VM_MEM		0x00000001
> +#define STATIC_VM_EMPTY		0x00000002
> +#define STATIC_VM_MTYPE(mtype)	((mtype) << 20)
> +
> +#define STATIC_VM_TYPE(type, mtype) (type | STATIC_VM_MTYPE(mtype))
> +
> +struct static_vm {
> +	struct vm_struct vm;
> +	struct list_head list;
> +	unsigned long flags;
> +};

What is your motivation for having separate flags instead of simply 
keeping the current vm->flags usage?


Nicolas

Re: [PATCH v4 3/3] ARM: mm: use static_vm for managing static mapped areas

[Nicolas Pitre]

On Thu, 31 Jan 2013, Joonsoo Kim wrote:

> A static mapped area is ARM-specific, so it is better not to use
> generic vmalloc data structure, that is, vmlist and vmlist_lock
> for managing static mapped area. And it causes some needless overhead and
> reducing this overhead is better idea.
> 
> Now, we have newly introduced static_vm infrastructure.
> With it, we don't need to iterate all mapped areas. Instead, we just
> iterate static mapped areas. It helps to reduce an overhead of finding
> matched area. And architecture dependency on vmalloc layer is removed,
> so it will help to maintainability for vmalloc layer.
> 
> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>

Comments below.

> 
> diff --git a/arch/arm/mm/ioremap.c b/arch/arm/mm/ioremap.c
> index ceb34ae..7fe5b48 100644
> --- a/arch/arm/mm/ioremap.c
> +++ b/arch/arm/mm/ioremap.c
> @@ -269,13 +269,14 @@ void __iomem * __arm_ioremap_pfn_caller(unsigned long pfn,
>  	const struct mem_type *type;
>  	int err;
>  	unsigned long addr;
> - 	struct vm_struct * area;
> +	struct vm_struct *area;
> +	phys_addr_t paddr = __pfn_to_phys(pfn);
>  
>  #ifndef CONFIG_ARM_LPAE
>  	/*
>  	 * High mappings must be supersection aligned
>  	 */
> -	if (pfn >= 0x100000 && (__pfn_to_phys(pfn) & ~SUPERSECTION_MASK))
> +	if (pfn >= 0x100000 && (paddr & ~SUPERSECTION_MASK))
>  		return NULL;
>  #endif
>  
> @@ -291,24 +292,17 @@ void __iomem * __arm_ioremap_pfn_caller(unsigned long pfn,
>  	/*
>  	 * Try to reuse one of the static mapping whenever possible.
>  	 */
> -	read_lock(&vmlist_lock);
> -	for (area = vmlist; area; area = area->next) {
> -		if (!size || (sizeof(phys_addr_t) == 4 && pfn >= 0x100000))
> -			break;
> -		if (!(area->flags & VM_ARM_STATIC_MAPPING))
> -			continue;
> -		if ((area->flags & VM_ARM_MTYPE_MASK) != VM_ARM_MTYPE(mtype))
> -			continue;
> -		if (__phys_to_pfn(area->phys_addr) > pfn ||
> -		    __pfn_to_phys(pfn) + size-1 > area->phys_addr + area->size-1)
> -			continue;
> -		/* we can drop the lock here as we know *area is static */
> -		read_unlock(&vmlist_lock);
> -		addr = (unsigned long)area->addr;
> -		addr += __pfn_to_phys(pfn) - area->phys_addr;
> -		return (void __iomem *) (offset + addr);
> +	if (size && !((sizeof(phys_addr_t) == 4 && pfn >= 0x100000))) {
> +		struct static_vm *svm;
> +
> +		svm = find_static_vm_paddr(paddr, size,
> +				STATIC_VM_TYPE(STATIC_VM_MEM, mtype));
> +		if (svm) {
> +			addr = (unsigned long)svm->vm.addr;
> +			addr += paddr - svm->vm.phys_addr;
> +			return (void __iomem *) (offset + addr);
> +		}
>  	}
> -	read_unlock(&vmlist_lock);
>  
>  	/*
>  	 * Don't allow RAM to be mapped - this causes problems with ARMv6+
> @@ -320,21 +314,21 @@ void __iomem * __arm_ioremap_pfn_caller(unsigned long pfn,
>   	if (!area)
>   		return NULL;
>   	addr = (unsigned long)area->addr;
> -	area->phys_addr = __pfn_to_phys(pfn);
> +	area->phys_addr = paddr;
>  
>  #if !defined(CONFIG_SMP) && !defined(CONFIG_ARM_LPAE)
>  	if (DOMAIN_IO == 0 &&
>  	    (((cpu_architecture() >= CPU_ARCH_ARMv6) && (get_cr() & CR_XP)) ||
>  	       cpu_is_xsc3()) && pfn >= 0x100000 &&
> -	       !((__pfn_to_phys(pfn) | size | addr) & ~SUPERSECTION_MASK)) {
> +	       !((paddr | size | addr) & ~SUPERSECTION_MASK)) {
>  		area->flags |= VM_ARM_SECTION_MAPPING;
>  		err = remap_area_supersections(addr, pfn, size, type);
> -	} else if (!((__pfn_to_phys(pfn) | size | addr) & ~PMD_MASK)) {
> +	} else if (!((paddr | size | addr) & ~PMD_MASK)) {
>  		area->flags |= VM_ARM_SECTION_MAPPING;
>  		err = remap_area_sections(addr, pfn, size, type);
>  	} else
>  #endif
> -		err = ioremap_page_range(addr, addr + size, __pfn_to_phys(pfn),
> +		err = ioremap_page_range(addr, addr + size, paddr,
>  					 __pgprot(type->prot_pte));
>  
>  	if (err) {
> @@ -418,34 +412,21 @@ __arm_ioremap_exec(unsigned long phys_addr, size_t size, bool cached)
>  void __iounmap(volatile void __iomem *io_addr)
>  {
>  	void *addr = (void *)(PAGE_MASK & (unsigned long)io_addr);
> -	struct vm_struct *vm;
> +	struct static_vm *svm;
> +

You could salvage the "/* If this is a static mapping we must leave it 
alone */" comment here.

> +	svm = find_static_vm_vaddr(addr);
> +	if (svm)
> +		return;
>  
> -	read_lock(&vmlist_lock);
> -	for (vm = vmlist; vm; vm = vm->next) {
> -		if (vm->addr > addr)
> -			break;
> -		if (!(vm->flags & VM_IOREMAP))
> -			continue;
> -		/* If this is a static mapping we must leave it alone */
> -		if ((vm->flags & VM_ARM_STATIC_MAPPING) &&
> -		    (vm->addr <= addr) && (vm->addr + vm->size > addr)) {
> -			read_unlock(&vmlist_lock);
> -			return;
> -		}
>  #if !defined(CONFIG_SMP) && !defined(CONFIG_ARM_LPAE)
> -		/*
> -		 * If this is a section based mapping we need to handle it
> -		 * specially as the VM subsystem does not know how to handle
> -		 * such a beast.
> -		 */

Please don't remove the above comment.  It is still relevant.

> -		if ((vm->addr == addr) &&
> -		    (vm->flags & VM_ARM_SECTION_MAPPING)) {
> +	{
> +		struct vm_struct *vm;
> +
> +		vm = find_vm_area(addr);
> +		if (vm && (vm->flags & VM_ARM_SECTION_MAPPING))
>  			unmap_area_sections((unsigned long)vm->addr, vm->size);
> -			break;
> -		}
> -#endif
>  	}
> -	read_unlock(&vmlist_lock);
> +#endif
>  
>  	vunmap(addr);
>  }
> diff --git a/arch/arm/mm/mm.h b/arch/arm/mm/mm.h
> index fb45c79..24c1df4 100644
> --- a/arch/arm/mm/mm.h
> +++ b/arch/arm/mm/mm.h
> @@ -54,16 +54,6 @@ extern void __flush_dcache_page(struct address_space *mapping, struct page *page
>  /* (super)section-mapped I/O regions used by ioremap()/iounmap() */
>  #define VM_ARM_SECTION_MAPPING	0x80000000
>  
> -/* permanent static mappings from iotable_init() */
> -#define VM_ARM_STATIC_MAPPING	0x40000000
> -
> -/* empty mapping */
> -#define VM_ARM_EMPTY_MAPPING	0x20000000
> -
> -/* mapping type (attributes) for permanent static mappings */
> -#define VM_ARM_MTYPE(mt)		((mt) << 20)
> -#define VM_ARM_MTYPE_MASK	(0x1f << 20)
> -

This goes with a related question in my previous email: why didn't you 
keep those flags as they were?


Nicolas

Re: [PATCH v4 2/3] ARM: ioremap: introduce an infrastructure for static mapped area

[JoonSoo Kim]

Hello, Nicolas.

2013/2/1 Nicolas Pitre <nicolas.pitre@linaro.org>:
> On Thu, 31 Jan 2013, Joonsoo Kim wrote:
>
>> In current implementation, we used ARM-specific flag, that is,
>> VM_ARM_STATIC_MAPPING, for distinguishing ARM specific static mapped area.
>> The purpose of static mapped area is to re-use static mapped area when
>> entire physical address range of the ioremap request can be covered
>> by this area.
>>
>> This implementation causes needless overhead for some cases.
>> For example, assume that there is only one static mapped area and
>> vmlist has 300 areas. Every time we call ioremap, we check 300 areas for
>> deciding whether it is matched or not. Moreover, even if there is
>> no static mapped area and vmlist has 300 areas, every time we call
>> ioremap, we check 300 areas in now.
>>
>> If we construct a extra list for static mapped area, we can eliminate
>> above mentioned overhead.
>> With a extra list, if there is one static mapped area,
>> we just check only one area and proceed next operation quickly.
>>
>> In fact, it is not a critical problem, because ioremap is not frequently
>> used. But reducing overhead is better idea.
>>
>> Another reason for doing this work is for removing architecture dependency
>> on vmalloc layer. I think that vmlist and vmlist_lock is internal data
>> structure for vmalloc layer. Some codes for debugging and stat inevitably
>> use vmlist and vmlist_lock. But it is preferable that they are used
>> as least as possible in outside of vmalloc.c
>>
>> Now, I introduce an ARM-specific infrastructure for static mapped area. In
>> the following patch, we will use this and resolve above mentioned problem.
>>
>> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
>
> Much better.  Comments below.

Thanks.

>> diff --git a/arch/arm/mm/ioremap.c b/arch/arm/mm/ioremap.c
>> index 88fd86c..ceb34ae 100644
>> --- a/arch/arm/mm/ioremap.c
>> +++ b/arch/arm/mm/ioremap.c
>> @@ -39,6 +39,78 @@
>>  #include <asm/mach/pci.h>
>>  #include "mm.h"
>>
>> +
>> +LIST_HEAD(static_vmlist);
>> +static DEFINE_RWLOCK(static_vmlist_lock);
>
> In fact you don't need a lock at all.  The only writer is
> add_static_vm_early() and we know it is only used during boot when the
> kernel is still single-threaded.

Yes!

>> +
>> +static struct static_vm *find_static_vm_paddr(phys_addr_t paddr,
>> +                     size_t size, unsigned long flags)
>> +{
>> +     struct static_vm *svm;
>> +     struct vm_struct *vm;
>> +
>> +     read_lock(&static_vmlist_lock);
>> +     list_for_each_entry(svm, &static_vmlist, list) {
>> +             if (svm->flags != flags)
>> +                     continue;
>> +
>> +             vm = &svm->vm;
>> +             if (vm->phys_addr > paddr ||
>> +                     paddr + size - 1 > vm->phys_addr + vm->size - 1)
>> +                     continue;
>> +
>> +             read_unlock(&static_vmlist_lock);
>> +             return svm;
>> +     }
>> +
>> +     return NULL;
>> +}
>> +
>> +struct static_vm *find_static_vm_vaddr(void *vaddr)
>> +{
>> +     struct static_vm *svm;
>> +     struct vm_struct *vm;
>> +
>> +     read_lock(&static_vmlist_lock);
>> +     list_for_each_entry(svm, &static_vmlist, list) {
>> +             vm = &svm->vm;
>> +
>> +             /* static_vmlist is ascending order */
>> +             if (vm->addr > vaddr)
>> +                     break;
>> +
>> +             if (vm->addr <= vaddr && vm->addr + vm->size > vaddr) {
>> +                     read_unlock(&static_vmlist_lock);
>> +                     return svm;
>> +             }
>> +     }
>> +     read_unlock(&static_vmlist_lock);
>> +
>> +     return NULL;
>> +}
>> +
>> +void add_static_vm_early(struct static_vm *svm, unsigned long flags)
>
> This should be marked with __init.  This way, it is less likely to be
> used after boot, especially with no locking.  And vm_area_add_early() is
> valid only if !vmap_initialized anyway, and also __init.

Okay.

>> +{
>> +     struct static_vm *curr_svm;
>> +     struct vm_struct *vm;
>> +     void *vaddr;
>> +
>> +     vm_area_add_early(&svm->vm);
>> +
>> +     vaddr = svm->vm.addr;
>> +     svm->flags = flags;
>> +
>> +     write_lock(&static_vmlist_lock);
>> +     list_for_each_entry(curr_svm, &static_vmlist, list) {
>> +             vm = &curr_svm->vm;
>> +
>> +             if (vm->addr > vaddr)
>> +                     break;
>> +     }
>> +     list_add_tail(&svm->list, &curr_svm->list);
>> +     write_unlock(&static_vmlist_lock);
>> +}
>> +
>>  int ioremap_page(unsigned long virt, unsigned long phys,
>>                const struct mem_type *mtype)
>>  {
>> diff --git a/arch/arm/mm/mm.h b/arch/arm/mm/mm.h
>> index a8ee92d..fb45c79 100644
>> --- a/arch/arm/mm/mm.h
>> +++ b/arch/arm/mm/mm.h
>> @@ -1,4 +1,6 @@
>>  #ifdef CONFIG_MMU
>> +#include <linux/list.h>
>> +#include <linux/vmalloc.h>
>>
>>  /* the upper-most page table pointer */
>>  extern pmd_t *top_pmd;
>> @@ -65,6 +67,24 @@ extern void __flush_dcache_page(struct address_space *mapping, struct page *page
>>  /* consistent regions used by dma_alloc_attrs() */
>>  #define VM_ARM_DMA_CONSISTENT        0x20000000
>>
>> +
>> +/* ARM specific static_vm->flags bits */
>> +#define STATIC_VM_MEM                0x00000001
>> +#define STATIC_VM_EMPTY              0x00000002
>> +#define STATIC_VM_MTYPE(mtype)       ((mtype) << 20)
>> +
>> +#define STATIC_VM_TYPE(type, mtype) (type | STATIC_VM_MTYPE(mtype))
>> +
>> +struct static_vm {
>> +     struct vm_struct vm;
>> +     struct list_head list;
>> +     unsigned long flags;
>> +};
>
> What is your motivation for having separate flags instead of simply
> keeping the current vm->flags usage?
>

Keeping the current vm->flags is better idea.
I will re-work about all your comments.

Thanks.

Re: [PATCH v4 3/3] ARM: mm: use static_vm for managing static mapped areas

[JoonSoo Kim]

2013/2/1 Nicolas Pitre <nicolas.pitre@linaro.org>:
> On Thu, 31 Jan 2013, Joonsoo Kim wrote:
>
>> A static mapped area is ARM-specific, so it is better not to use
>> generic vmalloc data structure, that is, vmlist and vmlist_lock
>> for managing static mapped area. And it causes some needless overhead and
>> reducing this overhead is better idea.
>>
>> Now, we have newly introduced static_vm infrastructure.
>> With it, we don't need to iterate all mapped areas. Instead, we just
>> iterate static mapped areas. It helps to reduce an overhead of finding
>> matched area. And architecture dependency on vmalloc layer is removed,
>> so it will help to maintainability for vmalloc layer.
>>
>> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
>
> Comments below.
>
>>
>> diff --git a/arch/arm/mm/ioremap.c b/arch/arm/mm/ioremap.c
>> index ceb34ae..7fe5b48 100644
>> --- a/arch/arm/mm/ioremap.c
>> +++ b/arch/arm/mm/ioremap.c
>> @@ -269,13 +269,14 @@ void __iomem * __arm_ioremap_pfn_caller(unsigned long pfn,
>>       const struct mem_type *type;
>>       int err;
>>       unsigned long addr;
>> -     struct vm_struct * area;
>> +     struct vm_struct *area;
>> +     phys_addr_t paddr = __pfn_to_phys(pfn);
>>
>>  #ifndef CONFIG_ARM_LPAE
>>       /*
>>        * High mappings must be supersection aligned
>>        */
>> -     if (pfn >= 0x100000 && (__pfn_to_phys(pfn) & ~SUPERSECTION_MASK))
>> +     if (pfn >= 0x100000 && (paddr & ~SUPERSECTION_MASK))
>>               return NULL;
>>  #endif
>>
>> @@ -291,24 +292,17 @@ void __iomem * __arm_ioremap_pfn_caller(unsigned long pfn,
>>       /*
>>        * Try to reuse one of the static mapping whenever possible.
>>        */
>> -     read_lock(&vmlist_lock);
>> -     for (area = vmlist; area; area = area->next) {
>> -             if (!size || (sizeof(phys_addr_t) == 4 && pfn >= 0x100000))
>> -                     break;
>> -             if (!(area->flags & VM_ARM_STATIC_MAPPING))
>> -                     continue;
>> -             if ((area->flags & VM_ARM_MTYPE_MASK) != VM_ARM_MTYPE(mtype))
>> -                     continue;
>> -             if (__phys_to_pfn(area->phys_addr) > pfn ||
>> -                 __pfn_to_phys(pfn) + size-1 > area->phys_addr + area->size-1)
>> -                     continue;
>> -             /* we can drop the lock here as we know *area is static */
>> -             read_unlock(&vmlist_lock);
>> -             addr = (unsigned long)area->addr;
>> -             addr += __pfn_to_phys(pfn) - area->phys_addr;
>> -             return (void __iomem *) (offset + addr);
>> +     if (size && !((sizeof(phys_addr_t) == 4 && pfn >= 0x100000))) {
>> +             struct static_vm *svm;
>> +
>> +             svm = find_static_vm_paddr(paddr, size,
>> +                             STATIC_VM_TYPE(STATIC_VM_MEM, mtype));
>> +             if (svm) {
>> +                     addr = (unsigned long)svm->vm.addr;
>> +                     addr += paddr - svm->vm.phys_addr;
>> +                     return (void __iomem *) (offset + addr);
>> +             }
>>       }
>> -     read_unlock(&vmlist_lock);
>>
>>       /*
>>        * Don't allow RAM to be mapped - this causes problems with ARMv6+
>> @@ -320,21 +314,21 @@ void __iomem * __arm_ioremap_pfn_caller(unsigned long pfn,
>>       if (!area)
>>               return NULL;
>>       addr = (unsigned long)area->addr;
>> -     area->phys_addr = __pfn_to_phys(pfn);
>> +     area->phys_addr = paddr;
>>
>>  #if !defined(CONFIG_SMP) && !defined(CONFIG_ARM_LPAE)
>>       if (DOMAIN_IO == 0 &&
>>           (((cpu_architecture() >= CPU_ARCH_ARMv6) && (get_cr() & CR_XP)) ||
>>              cpu_is_xsc3()) && pfn >= 0x100000 &&
>> -            !((__pfn_to_phys(pfn) | size | addr) & ~SUPERSECTION_MASK)) {
>> +            !((paddr | size | addr) & ~SUPERSECTION_MASK)) {
>>               area->flags |= VM_ARM_SECTION_MAPPING;
>>               err = remap_area_supersections(addr, pfn, size, type);
>> -     } else if (!((__pfn_to_phys(pfn) | size | addr) & ~PMD_MASK)) {
>> +     } else if (!((paddr | size | addr) & ~PMD_MASK)) {
>>               area->flags |= VM_ARM_SECTION_MAPPING;
>>               err = remap_area_sections(addr, pfn, size, type);
>>       } else
>>  #endif
>> -             err = ioremap_page_range(addr, addr + size, __pfn_to_phys(pfn),
>> +             err = ioremap_page_range(addr, addr + size, paddr,
>>                                        __pgprot(type->prot_pte));
>>
>>       if (err) {
>> @@ -418,34 +412,21 @@ __arm_ioremap_exec(unsigned long phys_addr, size_t size, bool cached)
>>  void __iounmap(volatile void __iomem *io_addr)
>>  {
>>       void *addr = (void *)(PAGE_MASK & (unsigned long)io_addr);
>> -     struct vm_struct *vm;
>> +     struct static_vm *svm;
>> +
>
> You could salvage the "/* If this is a static mapping we must leave it
> alone */" comment here.

Okay.

>> +     svm = find_static_vm_vaddr(addr);
>> +     if (svm)
>> +             return;
>>
>> -     read_lock(&vmlist_lock);
>> -     for (vm = vmlist; vm; vm = vm->next) {
>> -             if (vm->addr > addr)
>> -                     break;
>> -             if (!(vm->flags & VM_IOREMAP))
>> -                     continue;
>> -             /* If this is a static mapping we must leave it alone */
>> -             if ((vm->flags & VM_ARM_STATIC_MAPPING) &&
>> -                 (vm->addr <= addr) && (vm->addr + vm->size > addr)) {
>> -                     read_unlock(&vmlist_lock);
>> -                     return;
>> -             }
>>  #if !defined(CONFIG_SMP) && !defined(CONFIG_ARM_LPAE)
>> -             /*
>> -              * If this is a section based mapping we need to handle it
>> -              * specially as the VM subsystem does not know how to handle
>> -              * such a beast.
>> -              */
>
> Please don't remove the above comment.  It is still relevant.

Yes.

Thanks for comments.

>> -             if ((vm->addr == addr) &&
>> -                 (vm->flags & VM_ARM_SECTION_MAPPING)) {
>> +     {
>> +             struct vm_struct *vm;
>> +
>> +             vm = find_vm_area(addr);
>> +             if (vm && (vm->flags & VM_ARM_SECTION_MAPPING))
>>                       unmap_area_sections((unsigned long)vm->addr, vm->size);
>> -                     break;
>> -             }
>> -#endif
>>       }
>> -     read_unlock(&vmlist_lock);
>> +#endif
>>
>>       vunmap(addr);
>>  }
>> diff --git a/arch/arm/mm/mm.h b/arch/arm/mm/mm.h
>> index fb45c79..24c1df4 100644
>> --- a/arch/arm/mm/mm.h
>> +++ b/arch/arm/mm/mm.h
>> @@ -54,16 +54,6 @@ extern void __flush_dcache_page(struct address_space *mapping, struct page *page
>>  /* (super)section-mapped I/O regions used by ioremap()/iounmap() */
>>  #define VM_ARM_SECTION_MAPPING       0x80000000
>>
>> -/* permanent static mappings from iotable_init() */
>> -#define VM_ARM_STATIC_MAPPING        0x40000000
>> -
>> -/* empty mapping */
>> -#define VM_ARM_EMPTY_MAPPING 0x20000000
>> -
>> -/* mapping type (attributes) for permanent static mappings */
>> -#define VM_ARM_MTYPE(mt)             ((mt) << 20)
>> -#define VM_ARM_MTYPE_MASK    (0x1f << 20)
>> -
>
> This goes with a related question in my previous email: why didn't you
> keep those flags as they were?
>
>
> Nicolas