[RFC][PATCH 0/2] Add IO mapping space reused support

[RFC][PATCH 0/2] Add IO mapping space reused support

[Richard Lee]

Richard Lee (2):
  mm/vmalloc: Add IO mapping space reused interface.
  ARM: ioremap: Add IO mapping space reused support.

 arch/arm/mm/ioremap.c   | 11 ++++++++-
 include/linux/vmalloc.h |  5 ++++
 mm/vmalloc.c            | 63 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 78 insertions(+), 1 deletion(-)

-- 
1.8.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[RFC][PATCH 1/2] mm/vmalloc: Add IO mapping space reused interface.

[Richard Lee]

For the IO mapping, for the same physical address space maybe
mapped more than one time, for example, in some SoCs:
0x20000000 ~ 0x20001000: are global control IO physical map,
and this range space will be used by many drivers.
And then if each driver will do the same ioremap operation, we
will waste to much malloc virtual spaces.

This patch add the IO mapping space reusing interface:
- find_vm_area_paddr: used to find the exsit vmalloc area using
  the IO physical address.
- vm_area_is_aready_to_free: before vfree the IO mapped areas
  using this to do the check that if this area is used by more
  than one consumer.

Signed-off-by: Richard Lee <superlibj@gmail.com>
---
 include/linux/vmalloc.h |  5 ++++
 mm/vmalloc.c            | 63 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 68 insertions(+)

diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
index 4b8a891..2b811f6 100644
--- a/include/linux/vmalloc.h
+++ b/include/linux/vmalloc.h
@@ -34,6 +34,7 @@ struct vm_struct {
 	struct page		**pages;
 	unsigned int		nr_pages;
 	phys_addr_t		phys_addr;
+	unsigned int		used;
 	const void		*caller;
 };
 
@@ -100,6 +101,10 @@ static inline size_t get_vm_area_size(const struct vm_struct *area)
 	return area->size - PAGE_SIZE;
 }
 
+extern int vm_area_is_aready_to_free(phys_addr_t addr);
+struct vm_struct *find_vm_area_paddr(phys_addr_t paddr, size_t size,
+				     unsigned long *offset,
+				     unsigned long flags);
 extern struct vm_struct *get_vm_area(unsigned long size, unsigned long flags);
 extern struct vm_struct *get_vm_area_caller(unsigned long size,
 					unsigned long flags, const void *caller);
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index bf233b2..f75b7b3 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -1293,6 +1293,7 @@ static void setup_vmalloc_vm(struct vm_struct *vm, struct vmap_area *va,
 	vm->addr = (void *)va->va_start;
 	vm->size = va->va_end - va->va_start;
 	vm->caller = caller;
+	vm->used = 1;
 	va->vm = vm;
 	va->flags |= VM_VM_AREA;
 	spin_unlock(&vmap_area_lock);
@@ -1383,6 +1384,68 @@ struct vm_struct *get_vm_area_caller(unsigned long size, unsigned long flags,
 				  NUMA_NO_NODE, GFP_KERNEL, caller);
 }
 
+int vm_area_is_aready_to_free(phys_addr_t addr)
+{
+	struct vmap_area *va;
+
+	va = find_vmap_area((unsigned long)addr);
+	if (!va || !(va->flags & VM_VM_AREA) || !va->vm)
+		return 1;
+
+	if (va->vm->used <= 1)
+		return 1;
+
+	--va->vm->used;
+
+	return 0;
+}
+
+/**
+ *	find_vm_area_paddr  -  find a continuous kernel virtual area using the
+ *			physical addreess.
+ *	@paddr:		base physical address
+ *	@size:		size of the physical area range
+ *	@offset:	the start offset of the vm area
+ *	@flags:		%VM_IOREMAP for I/O mappings
+ *
+ *	Search for the kernel VM area, whoes physical address starting at @paddr,
+ *	and if the exsit VM area's size is large enough, then just return it, or
+ *	return NULL.
+ */
+struct vm_struct *find_vm_area_paddr(phys_addr_t paddr, size_t size,
+				     unsigned long *offset,
+				     unsigned long flags)
+{
+	struct vmap_area *va;
+
+	if (!(flags & VM_IOREMAP))
+		return NULL;
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(va, &vmap_area_list, list) {
+		phys_addr_t phys_addr;
+
+		if (!va || !(va->flags & VM_VM_AREA) || !va->vm)
+			continue;
+
+		phys_addr = va->vm->phys_addr;
+
+		if (paddr < phys_addr || paddr + size > phys_addr + va->vm->size)
+			continue;
+
+		*offset = paddr - phys_addr;
+
+		if (va->vm->flags & VM_IOREMAP && va->vm->size >= size) {
+			va->vm->used++;
+			rcu_read_unlock();
+			return va->vm;
+		}
+	}
+	rcu_read_unlock();
+
+	return NULL;
+}
+
 /**
  *	find_vm_area  -  find a continuous kernel virtual area
  *	@addr:		base address
-- 
1.8.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [RFC][PATCH 1/2] mm/vmalloc: Add IO mapping space reused interface.

[Rob Herring]

On Sun, May 11, 2014 at 9:19 PM, Richard Lee <superlibj8301@gmail.com> wrote:
> For the IO mapping, for the same physical address space maybe
> mapped more than one time, for example, in some SoCs:
> 0x20000000 ~ 0x20001000: are global control IO physical map,
> and this range space will be used by many drivers.

What address or who the user is isn't really relevant.

> And then if each driver will do the same ioremap operation, we
> will waste to much malloc virtual spaces.

s/malloc/vmalloc/

>
> This patch add the IO mapping space reusing interface:
> - find_vm_area_paddr: used to find the exsit vmalloc area using

s/exsit/exist/

>   the IO physical address.
> - vm_area_is_aready_to_free: before vfree the IO mapped areas
>   using this to do the check that if this area is used by more
>   than one consumer.
>
> Signed-off-by: Richard Lee <superlibj@gmail.com>
> ---
>  include/linux/vmalloc.h |  5 ++++
>  mm/vmalloc.c            | 63 +++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 68 insertions(+)
>
> diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
> index 4b8a891..2b811f6 100644
> --- a/include/linux/vmalloc.h
> +++ b/include/linux/vmalloc.h
> @@ -34,6 +34,7 @@ struct vm_struct {
>         struct page             **pages;
>         unsigned int            nr_pages;
>         phys_addr_t             phys_addr;
> +       unsigned int            used;
>         const void              *caller;
>  };
>
> @@ -100,6 +101,10 @@ static inline size_t get_vm_area_size(const struct vm_struct *area)
>         return area->size - PAGE_SIZE;
>  }
>
> +extern int vm_area_is_aready_to_free(phys_addr_t addr);
> +struct vm_struct *find_vm_area_paddr(phys_addr_t paddr, size_t size,
> +                                    unsigned long *offset,
> +                                    unsigned long flags);
>  extern struct vm_struct *get_vm_area(unsigned long size, unsigned long flags);
>  extern struct vm_struct *get_vm_area_caller(unsigned long size,
>                                         unsigned long flags, const void *caller);
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index bf233b2..f75b7b3 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -1293,6 +1293,7 @@ static void setup_vmalloc_vm(struct vm_struct *vm, struct vmap_area *va,
>         vm->addr = (void *)va->va_start;
>         vm->size = va->va_end - va->va_start;
>         vm->caller = caller;
> +       vm->used = 1;
>         va->vm = vm;
>         va->flags |= VM_VM_AREA;
>         spin_unlock(&vmap_area_lock);
> @@ -1383,6 +1384,68 @@ struct vm_struct *get_vm_area_caller(unsigned long size, unsigned long flags,
>                                   NUMA_NO_NODE, GFP_KERNEL, caller);
>  }
>
> +int vm_area_is_aready_to_free(phys_addr_t addr)

aready is not a word.

> +{
> +       struct vmap_area *va;
> +
> +       va = find_vmap_area((unsigned long)addr);
> +       if (!va || !(va->flags & VM_VM_AREA) || !va->vm)
> +               return 1;
> +
> +       if (va->vm->used <= 1)
> +               return 1;
> +
> +       --va->vm->used;

What lock protects this? You should use atomic ops here.

> +
> +       return 0;
> +}
> +
> +/**
> + *     find_vm_area_paddr  -  find a continuous kernel virtual area using the
> + *                     physical addreess.
> + *     @paddr:         base physical address
> + *     @size:          size of the physical area range
> + *     @offset:        the start offset of the vm area
> + *     @flags:         %VM_IOREMAP for I/O mappings
> + *
> + *     Search for the kernel VM area, whoes physical address starting at @paddr,
> + *     and if the exsit VM area's size is large enough, then just return it, or
> + *     return NULL.
> + */
> +struct vm_struct *find_vm_area_paddr(phys_addr_t paddr, size_t size,
> +                                    unsigned long *offset,
> +                                    unsigned long flags)
> +{
> +       struct vmap_area *va;
> +
> +       if (!(flags & VM_IOREMAP))
> +               return NULL;
> +
> +       rcu_read_lock();
> +       list_for_each_entry_rcu(va, &vmap_area_list, list) {
> +               phys_addr_t phys_addr;
> +
> +               if (!va || !(va->flags & VM_VM_AREA) || !va->vm)
> +                       continue;
> +
> +               phys_addr = va->vm->phys_addr;
> +
> +               if (paddr < phys_addr || paddr + size > phys_addr + va->vm->size)
> +                       continue;
> +
> +               *offset = paddr - phys_addr;
> +
> +               if (va->vm->flags & VM_IOREMAP && va->vm->size >= size) {
> +                       va->vm->used++;

What lock protects this? It looks like you are modifying this with
only a rcu reader lock.

> +                       rcu_read_unlock();
> +                       return va->vm;
> +               }
> +       }
> +       rcu_read_unlock();
> +
> +       return NULL;
> +}
> +
>  /**
>   *     find_vm_area  -  find a continuous kernel virtual area
>   *     @addr:          base address
> --
> 1.8.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [RFC][PATCH 1/2] mm/vmalloc: Add IO mapping space reused interface.

[Richard Lee]

On Tue, May 13, 2014 at 11:13 AM, Rob Herring <robherring2@gmail.com> wrote:
> On Sun, May 11, 2014 at 9:19 PM, Richard Lee <superlibj8301@gmail.com> wrote:
>> For the IO mapping, for the same physical address space maybe
>> mapped more than one time, for example, in some SoCs:
>> 0x20000000 ~ 0x20001000: are global control IO physical map,
>> and this range space will be used by many drivers.
>
> What address or who the user is isn't really relevant.
>
>> And then if each driver will do the same ioremap operation, we
>> will waste to much malloc virtual spaces.
>
> s/malloc/vmalloc/
>
>>
>> This patch add the IO mapping space reusing interface:
>> - find_vm_area_paddr: used to find the exsit vmalloc area using
>
> s/exsit/exist/
>

Yes, see the next version.

[...]
>> +{
>> +       struct vmap_area *va;
>> +
>> +       va = find_vmap_area((unsigned long)addr);
>> +       if (!va || !(va->flags & VM_VM_AREA) || !va->vm)
>> +               return 1;
>> +
>> +       if (va->vm->used <= 1)
>> +               return 1;
>> +
>> +       --va->vm->used;
>
> What lock protects this? You should use atomic ops here.
>

Yes, it is.


[...]
>> +       if (!(flags & VM_IOREMAP))
>> +               return NULL;
>> +
>> +       rcu_read_lock();
>> +       list_for_each_entry_rcu(va, &vmap_area_list, list) {
>> +               phys_addr_t phys_addr;
>> +
>> +               if (!va || !(va->flags & VM_VM_AREA) || !va->vm)
>> +                       continue;
>> +
>> +               phys_addr = va->vm->phys_addr;
>> +
>> +               if (paddr < phys_addr || paddr + size > phys_addr + va->vm->size)
>> +                       continue;
>> +
>> +               *offset = paddr - phys_addr;
>> +
>> +               if (va->vm->flags & VM_IOREMAP && va->vm->size >= size) {
>> +                       va->vm->used++;
>
> What lock protects this? It looks like you are modifying this with
> only a rcu reader lock.

I'll try to use the proper lock ops for this later.



Thanks very much,

Richard


>
>> +                       rcu_read_unlock();
>> +                       return va->vm;
>> +               }
>> +       }
>> +       rcu_read_unlock();
>> +
>> +       return NULL;
>> +}
>> +
>>  /**
>>   *     find_vm_area  -  find a continuous kernel virtual area
>>   *     @addr:          base address
>> --
>> 1.8.4
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[RFC][PATCH 2/2] ARM: ioremap: Add IO mapping space reused support.

[Richard Lee]

For the IO mapping, for the same physical address space maybe
mapped more than one time, for example, in some SoCs:
0x20000000 ~ 0x20001000: are global control IO physical map,
and this range space will be used by many drivers.
And then if each driver will do the same ioremap operation, we
will waste to much malloc virtual spaces.

This patch add IO mapping space reused support.

Signed-off-by: Richard Lee <superlibj@gmail.com>
---
 arch/arm/mm/ioremap.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mm/ioremap.c b/arch/arm/mm/ioremap.c
index f9c32ba..26a3744 100644
--- a/arch/arm/mm/ioremap.c
+++ b/arch/arm/mm/ioremap.c
@@ -260,7 +260,7 @@ void __iomem * __arm_ioremap_pfn_caller(unsigned long pfn,
 {
 	const struct mem_type *type;
 	int err;
-	unsigned long addr;
+	unsigned long addr, off;
 	struct vm_struct *area;
 	phys_addr_t paddr = __pfn_to_phys(pfn);
 
@@ -301,6 +301,12 @@ void __iomem * __arm_ioremap_pfn_caller(unsigned long pfn,
 	if (WARN_ON(pfn_valid(pfn)))
 		return NULL;
 
+	area = find_vm_area_paddr(paddr, size, &off, VM_IOREMAP);
+	if (area) {
+		addr = (unsigned long)area->addr;
+		return (void __iomem *)(offset + off + addr);
+	}
+
 	area = get_vm_area_caller(size, VM_IOREMAP, caller);
  	if (!area)
  		return NULL;
@@ -410,6 +416,9 @@ void __iounmap(volatile void __iomem *io_addr)
 	if (svm)
 		return;
 
+	if (!vm_area_is_aready_to_free((unsigned long)addr))
+		return;
+
 #if !defined(CONFIG_SMP) && !defined(CONFIG_ARM_LPAE)
 	{
 		struct vm_struct *vm;
-- 
1.8.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [RFC][PATCH 2/2] ARM: ioremap: Add IO mapping space reused support.

[Arnd Bergmann]

On Monday 12 May 2014 10:19:55 Richard Lee wrote:
> For the IO mapping, for the same physical address space maybe
> mapped more than one time, for example, in some SoCs:
> 0x20000000 ~ 0x20001000: are global control IO physical map,
> and this range space will be used by many drivers.
> And then if each driver will do the same ioremap operation, we
> will waste to much malloc virtual spaces.
> 
> This patch add IO mapping space reused support.
> 
> Signed-off-by: Richard Lee <superlibj@gmail.com>

What happens if the first driver then unmaps the area?

	Arnd

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [RFC][PATCH 2/2] ARM: ioremap: Add IO mapping space reused support.

[Richard Lee]

> On Mon, May 12, 2014 at 3:51 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Monday 12 May 2014 10:19:55 Richard Lee wrote:
>> For the IO mapping, for the same physical address space maybe
>> mapped more than one time, for example, in some SoCs:
>> 0x20000000 ~ 0x20001000: are global control IO physical map,
>> and this range space will be used by many drivers.
>> And then if each driver will do the same ioremap operation, we
>> will waste to much malloc virtual spaces.
>>
>> This patch add IO mapping space reused support.
>>
>> Signed-off-by: Richard Lee <superlibj@gmail.com>
>
> What happens if the first driver then unmaps the area?
>

If the first driver will unmap the area, it shouldn't do any thing
except decreasing the 'used' counter.

Thanks,

BRs
Richard Lee


>         Arnd

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [RFC][PATCH 2/2] ARM: ioremap: Add IO mapping space reused support.

[Arnd Bergmann]

On Tuesday 13 May 2014 09:45:08 Richard Lee wrote:
> > On Mon, May 12, 2014 at 3:51 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Monday 12 May 2014 10:19:55 Richard Lee wrote:
> >> For the IO mapping, for the same physical address space maybe
> >> mapped more than one time, for example, in some SoCs:
> >> 0x20000000 ~ 0x20001000: are global control IO physical map,
> >> and this range space will be used by many drivers.
> >> And then if each driver will do the same ioremap operation, we
> >> will waste to much malloc virtual spaces.
> >>
> >> This patch add IO mapping space reused support.
> >>
> >> Signed-off-by: Richard Lee <superlibj@gmail.com>
> >
> > What happens if the first driver then unmaps the area?
> >
> 
> If the first driver will unmap the area, it shouldn't do any thing
> except decreasing the 'used' counter.

Ah, for some reason I didn't see your first patch that introduces
that counter.

	Arnd

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [RFC][PATCH 2/2] ARM: ioremap: Add IO mapping space reused support.

[Richard Lee]

On Tue, May 13, 2014 at 4:43 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday 13 May 2014 09:45:08 Richard Lee wrote:
>> > On Mon, May 12, 2014 at 3:51 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> > On Monday 12 May 2014 10:19:55 Richard Lee wrote:
>> >> For the IO mapping, for the same physical address space maybe
>> >> mapped more than one time, for example, in some SoCs:
>> >> 0x20000000 ~ 0x20001000: are global control IO physical map,
>> >> and this range space will be used by many drivers.
>> >> And then if each driver will do the same ioremap operation, we
>> >> will waste to much malloc virtual spaces.
>> >>
>> >> This patch add IO mapping space reused support.
>> >>
>> >> Signed-off-by: Richard Lee <superlibj@gmail.com>
>> >
>> > What happens if the first driver then unmaps the area?
>> >
>>
>> If the first driver will unmap the area, it shouldn't do any thing
>> except decreasing the 'used' counter.
>
> Ah, for some reason I didn't see your first patch that introduces
> that counter.
>

It's "[PATCH 1/2] mm/vmalloc: Add IO mapping space reused".

Thanks,

BRs
Richard



>         Arnd

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [RFC][PATCH 2/2] ARM: ioremap: Add IO mapping space reused support.

[Catalin Marinas]

On Tue, May 13, 2014 at 09:45:08AM +0800, Richard Lee wrote:
> > On Mon, May 12, 2014 at 3:51 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Monday 12 May 2014 10:19:55 Richard Lee wrote:
> >> For the IO mapping, for the same physical address space maybe
> >> mapped more than one time, for example, in some SoCs:
> >> 0x20000000 ~ 0x20001000: are global control IO physical map,
> >> and this range space will be used by many drivers.
> >> And then if each driver will do the same ioremap operation, we
> >> will waste to much malloc virtual spaces.
> >>
> >> This patch add IO mapping space reused support.
> >>
> >> Signed-off-by: Richard Lee <superlibj@gmail.com>
> >
> > What happens if the first driver then unmaps the area?
> 
> If the first driver will unmap the area, it shouldn't do any thing
> except decreasing the 'used' counter.

It's still racy. What if the first driver manage to decrement the used
counter, unmaps the regions but doesn't yet free the vm_struct while
another driver finds the vm_struct, increments the used count and
assumes it can use it?

BTW, vm_area_is_aready_to_free() name implies a query but it has
side-effects like decrementing the counter.

-- 
Catalin

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>