[RFC 0/4] vmalloc enhancements

[RFC 0/4] vmalloc enhancements

[Roman Gushchin]

The patchset contains few changes to the vmalloc code, which are
leading to some performance gains and code simplification.

Also, it exports a number of pages, used by vmalloc(),
in /proc/meminfo.

Patch (1) removes some redundancy on __vunmap().
Patch (2) is a preparation for patch (3).
Patch (3) merges independent 3 metadata allocations into one.
Patch (4) adds vmalloc counter to /proc/meminfo.

Roman Gushchin (4):
  mm: refactor __vunmap() to avoid duplicated call to find_vm_area()
  mm: separate memory allocation and actual work in alloc_vmap_area()
  mm: allocate vmalloc metadata in one allocation
  mm: show number of vmalloc pages in /proc/meminfo

 arch/mips/mm/ioremap.c      |   7 +-
 arch/nios2/mm/ioremap.c     |   4 +-
 arch/sh/kernel/cpu/sh4/sq.c |   5 +-
 arch/sh/mm/ioremap.c        |   8 +-
 arch/x86/mm/ioremap.c       |   4 +-
 fs/proc/meminfo.c           |   2 +-
 include/linux/vmalloc.h     |   6 +-
 mm/vmalloc.c                | 206 ++++++++++++++++++++++--------------
 8 files changed, 140 insertions(+), 102 deletions(-)

-- 
2.19.2

[RFC 1/4] mm: refactor __vunmap() to avoid duplicated call to find_vm_area()

[Roman Gushchin]

__vunmap() calls find_vm_area() twice without an obvious reason:
first directly to get the area pointer, second indirectly by calling
remove_vm_area(), which is again searching for the area.

To remove this redundancy, let's split remove_vm_area() into
__remove_vm_area(struct vmap_area *), which performs the actual area
removal, and remove_vm_area(const void *addr) wrapper, which can
be used everywhere, where it has been used before.

On my test setup, I've got up to 12% speed up on vfree()'ing 1000000
of 4-pages vmalloc blocks.

Signed-off-by: Roman Gushchin <guro@fb.com>
Acked-by: Johannes Weiner <hannes@cmpxchg.org>
---
 mm/vmalloc.c | 47 +++++++++++++++++++++++++++--------------------
 1 file changed, 27 insertions(+), 20 deletions(-)

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 97d4b25d0373..24a84d584609 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -1462,6 +1462,24 @@ struct vm_struct *find_vm_area(const void *addr)
 	return NULL;
 }
 
+static struct vm_struct *__remove_vm_area(struct vmap_area *va)
+{
+	struct vm_struct *vm = va->vm;
+
+	might_sleep();
+
+	spin_lock(&vmap_area_lock);
+	va->vm = NULL;
+	va->flags &= ~VM_VM_AREA;
+	va->flags |= VM_LAZY_FREE;
+	spin_unlock(&vmap_area_lock);
+
+	kasan_free_shadow(vm);
+	free_unmap_vmap_area(va);
+
+	return vm;
+}
+
 /**
  *	remove_vm_area  -  find and remove a continuous kernel virtual area
  *	@addr:		base address
@@ -1472,31 +1490,20 @@ struct vm_struct *find_vm_area(const void *addr)
  */
 struct vm_struct *remove_vm_area(const void *addr)
 {
+	struct vm_struct *vm = NULL;
 	struct vmap_area *va;
 
-	might_sleep();
-
 	va = find_vmap_area((unsigned long)addr);
-	if (va && va->flags & VM_VM_AREA) {
-		struct vm_struct *vm = va->vm;
-
-		spin_lock(&vmap_area_lock);
-		va->vm = NULL;
-		va->flags &= ~VM_VM_AREA;
-		va->flags |= VM_LAZY_FREE;
-		spin_unlock(&vmap_area_lock);
-
-		kasan_free_shadow(vm);
-		free_unmap_vmap_area(va);
+	if (va && va->flags & VM_VM_AREA)
+		vm = __remove_vm_area(va);
 
-		return vm;
-	}
-	return NULL;
+	return vm;
 }
 
 static void __vunmap(const void *addr, int deallocate_pages)
 {
 	struct vm_struct *area;
+	struct vmap_area *va;
 
 	if (!addr)
 		return;
@@ -1505,17 +1512,18 @@ static void __vunmap(const void *addr, int deallocate_pages)
 			addr))
 		return;
 
-	area = find_vmap_area((unsigned long)addr)->vm;
-	if (unlikely(!area)) {
+	va = find_vmap_area((unsigned long)addr);
+	if (unlikely(!va || !va->vm)) {
 		WARN(1, KERN_ERR "Trying to vfree() nonexistent vm area (%p)\n",
 				addr);
 		return;
 	}
 
+	area = va->vm;
 	debug_check_no_locks_freed(area->addr, get_vm_area_size(area));
 	debug_check_no_obj_freed(area->addr, get_vm_area_size(area));
 
-	remove_vm_area(addr);
+	__remove_vm_area(va);
 	if (deallocate_pages) {
 		int i;
 
@@ -1530,7 +1538,6 @@ static void __vunmap(const void *addr, int deallocate_pages)
 	}
 
 	kfree(area);
-	return;
 }
 
 static inline void __vfree_deferred(const void *addr)
-- 
2.19.2

Re: [RFC 1/4] mm: refactor __vunmap() to avoid duplicated call to find_vm_area()

[Matthew Wilcox]

On Fri, Dec 14, 2018 at 10:07:17AM -0800, Roman Gushchin wrote:
> __vunmap() calls find_vm_area() twice without an obvious reason:
> first directly to get the area pointer, second indirectly by calling
> remove_vm_area(), which is again searching for the area.
> 
> To remove this redundancy, let's split remove_vm_area() into
> __remove_vm_area(struct vmap_area *), which performs the actual area
> removal, and remove_vm_area(const void *addr) wrapper, which can
> be used everywhere, where it has been used before.
> 
> On my test setup, I've got up to 12% speed up on vfree()'ing 1000000
> of 4-pages vmalloc blocks.
> 
> Signed-off-by: Roman Gushchin <guro@fb.com>
> Acked-by: Johannes Weiner <hannes@cmpxchg.org>

Reviewed-by: Matthew Wilcox <willy@infradead.org>

[RFC 2/4] mm: separate memory allocation and actual work in alloc_vmap_area()

[Roman Gushchin]

alloc_vmap_area() is allocating memory for the vmap_area, and
performing the actual lookup of the vm area and vmap_area
initialization.

This prevents us from using a pre-allocated memory for the map_area
structure, which can be used in some cases to minimize the number
of required memory allocations.

Let's keep the memory allocation part in alloc_vmap_area() and
separate everything else into init_vmap_area().

Signed-off-by: Roman Gushchin <guro@fb.com>
Acked-by: Johannes Weiner <hannes@cmpxchg.org>
---
 mm/vmalloc.c | 52 +++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 35 insertions(+), 17 deletions(-)

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 24a84d584609..24c8ab28254d 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -395,16 +395,11 @@ static void purge_vmap_area_lazy(void);
 
 static BLOCKING_NOTIFIER_HEAD(vmap_notify_list);
 
-/*
- * Allocate a region of KVA of the specified size and alignment, within the
- * vstart and vend.
- */
-static struct vmap_area *alloc_vmap_area(unsigned long size,
-				unsigned long align,
-				unsigned long vstart, unsigned long vend,
-				int node, gfp_t gfp_mask)
+static int init_vmap_area(struct vmap_area *va, unsigned long size,
+			  unsigned long align, unsigned long vstart,
+			  unsigned long vend, int node, gfp_t gfp_mask)
 {
-	struct vmap_area *va;
+
 	struct rb_node *n;
 	unsigned long addr;
 	int purged = 0;
@@ -416,11 +411,6 @@ static struct vmap_area *alloc_vmap_area(unsigned long size,
 
 	might_sleep();
 
-	va = kmalloc_node(sizeof(struct vmap_area),
-			gfp_mask & GFP_RECLAIM_MASK, node);
-	if (unlikely(!va))
-		return ERR_PTR(-ENOMEM);
-
 	/*
 	 * Only scan the relevant parts containing pointers to other objects
 	 * to avoid false negatives.
@@ -512,7 +502,7 @@ static struct vmap_area *alloc_vmap_area(unsigned long size,
 	BUG_ON(va->va_start < vstart);
 	BUG_ON(va->va_end > vend);
 
-	return va;
+	return 0;
 
 overflow:
 	spin_unlock(&vmap_area_lock);
@@ -534,10 +524,38 @@ static struct vmap_area *alloc_vmap_area(unsigned long size,
 	if (!(gfp_mask & __GFP_NOWARN) && printk_ratelimit())
 		pr_warn("vmap allocation for size %lu failed: use vmalloc=<size> to increase size\n",
 			size);
-	kfree(va);
-	return ERR_PTR(-EBUSY);
+
+	return -EBUSY;
+}
+
+/*
+ * Allocate a region of KVA of the specified size and alignment, within the
+ * vstart and vend.
+ */
+static struct vmap_area *alloc_vmap_area(unsigned long size,
+					 unsigned long align,
+					 unsigned long vstart,
+					 unsigned long vend,
+					 int node, gfp_t gfp_mask)
+{
+	struct vmap_area *va;
+	int ret;
+
+	va = kmalloc_node(sizeof(struct vmap_area),
+			gfp_mask & GFP_RECLAIM_MASK, node);
+	if (unlikely(!va))
+		return ERR_PTR(-ENOMEM);
+
+	ret = init_vmap_area(va, size, align, vstart, vend, node, gfp_mask);
+	if (ret) {
+		kfree(va);
+		return ERR_PTR(ret);
+	}
+
+	return va;
 }
 
+
 int register_vmap_purge_notifier(struct notifier_block *nb)
 {
 	return blocking_notifier_chain_register(&vmap_notify_list, nb);
-- 
2.19.2

Re: [RFC 2/4] mm: separate memory allocation and actual work in alloc_vmap_area()

[Matthew Wilcox]

On Fri, Dec 14, 2018 at 10:07:18AM -0800, Roman Gushchin wrote:
> alloc_vmap_area() is allocating memory for the vmap_area, and
> performing the actual lookup of the vm area and vmap_area
> initialization.
> 
> This prevents us from using a pre-allocated memory for the map_area
> structure, which can be used in some cases to minimize the number
> of required memory allocations.
> 
> Let's keep the memory allocation part in alloc_vmap_area() and
> separate everything else into init_vmap_area().

Oh good, I was considering doing something like this eventually.

> +++ b/mm/vmalloc.c
> @@ -395,16 +395,11 @@ static void purge_vmap_area_lazy(void);
>  
>  static BLOCKING_NOTIFIER_HEAD(vmap_notify_list);
>  
> -/*
> - * Allocate a region of KVA of the specified size and alignment, within the
> - * vstart and vend.
> - */
> -static struct vmap_area *alloc_vmap_area(unsigned long size,
> -				unsigned long align,
> -				unsigned long vstart, unsigned long vend,
> -				int node, gfp_t gfp_mask)
> +static int init_vmap_area(struct vmap_area *va, unsigned long size,
> +			  unsigned long align, unsigned long vstart,
> +			  unsigned long vend, int node, gfp_t gfp_mask)
>  {
> -	struct vmap_area *va;
> +

Why the blank line?

>  	struct rb_node *n;
>  	unsigned long addr;
>  	int purged = 0;
> @@ -416,11 +411,6 @@ static struct vmap_area *alloc_vmap_area(unsigned long size,
>  
>  	might_sleep();
>  
> -	va = kmalloc_node(sizeof(struct vmap_area),
> -			gfp_mask & GFP_RECLAIM_MASK, node);
> -	if (unlikely(!va))
> -		return ERR_PTR(-ENOMEM);
> -
>  	/*
>  	 * Only scan the relevant parts containing pointers to other objects
>  	 * to avoid false negatives.
> @@ -512,7 +502,7 @@ static struct vmap_area *alloc_vmap_area(unsigned long size,
>  	BUG_ON(va->va_start < vstart);
>  	BUG_ON(va->va_end > vend);
>  
> -	return va;
> +	return 0;
>  
>  overflow:
>  	spin_unlock(&vmap_area_lock);
> @@ -534,10 +524,38 @@ static struct vmap_area *alloc_vmap_area(unsigned long size,
>  	if (!(gfp_mask & __GFP_NOWARN) && printk_ratelimit())
>  		pr_warn("vmap allocation for size %lu failed: use vmalloc=<size> to increase size\n",
>  			size);
> -	kfree(va);
> -	return ERR_PTR(-EBUSY);
> +
> +	return -EBUSY;
> +}
> +
> +/*
> + * Allocate a region of KVA of the specified size and alignment, within the
> + * vstart and vend.
> + */
> +static struct vmap_area *alloc_vmap_area(unsigned long size,
> +					 unsigned long align,
> +					 unsigned long vstart,
> +					 unsigned long vend,
> +					 int node, gfp_t gfp_mask)
> +{
> +	struct vmap_area *va;
> +	int ret;
> +
> +	va = kmalloc_node(sizeof(struct vmap_area),
> +			gfp_mask & GFP_RECLAIM_MASK, node);
> +	if (unlikely(!va))
> +		return ERR_PTR(-ENOMEM);
> +
> +	ret = init_vmap_area(va, size, align, vstart, vend, node, gfp_mask);
> +	if (ret) {
> +		kfree(va);
> +		return ERR_PTR(ret);
> +	}
> +
> +	return va;
>  }
>  
> +

Another spurious blank line?

With these two fixed,
Reviewed-by: Matthew Wilcox <willy@infradead.org>

Re: [RFC 2/4] mm: separate memory allocation and actual work in alloc_vmap_area()

[Joe Perches]

On Fri, 2018-12-14 at 10:13 -0800, Matthew Wilcox wrote:
> On Fri, Dec 14, 2018 at 10:07:18AM -0800, Roman Gushchin wrote:
> > +/*
> > + * Allocate a region of KVA of the specified size and alignment, within the
> > + * vstart and vend.
> > + */
> > +static struct vmap_area *alloc_vmap_area(unsigned long size,
> > +					 unsigned long align,
> > +					 unsigned long vstart,
> > +					 unsigned long vend,
> > +					 int node, gfp_t gfp_mask)
> > +{
> > +	struct vmap_area *va;
> > +	int ret;
> > +
> > +	va = kmalloc_node(sizeof(struct vmap_area),
> > +			gfp_mask & GFP_RECLAIM_MASK, node);
> > +	if (unlikely(!va))
> > +		return ERR_PTR(-ENOMEM);
> > +
> > +	ret = init_vmap_area(va, size, align, vstart, vend, node, gfp_mask);
> > +	if (ret) {
> > +		kfree(va);
> > +		return ERR_PTR(ret);
> > +	}
> > +
> > +	return va;
> >  }
> >  
> > +
> 
> Another spurious blank line?

I don't think so.

I think it is the better style to separate
the error return from the normal return.

> With these two fixed,
> Reviewed-by: Matthew Wilcox <willy@infradead.org>
> 

Re: [RFC 2/4] mm: separate memory allocation and actual work in alloc_vmap_area()

[Matthew Wilcox]

On Fri, Dec 14, 2018 at 11:40:45AM -0800, Joe Perches wrote:
> On Fri, 2018-12-14 at 10:13 -0800, Matthew Wilcox wrote:
> > On Fri, Dec 14, 2018 at 10:07:18AM -0800, Roman Gushchin wrote:
> > > +/*
> > > + * Allocate a region of KVA of the specified size and alignment, within the
> > > + * vstart and vend.
> > > + */
> > > +static struct vmap_area *alloc_vmap_area(unsigned long size,
> > > +					 unsigned long align,
> > > +					 unsigned long vstart,
> > > +					 unsigned long vend,
> > > +					 int node, gfp_t gfp_mask)
> > > +{
> > > +	struct vmap_area *va;
> > > +	int ret;
> > > +
> > > +	va = kmalloc_node(sizeof(struct vmap_area),
> > > +			gfp_mask & GFP_RECLAIM_MASK, node);
> > > +	if (unlikely(!va))
> > > +		return ERR_PTR(-ENOMEM);
> > > +
> > > +	ret = init_vmap_area(va, size, align, vstart, vend, node, gfp_mask);
> > > +	if (ret) {
> > > +		kfree(va);
> > > +		return ERR_PTR(ret);
> > > +	}
> > > +
> > > +	return va;
> > >  }
> > >  
> > > +
> > 
> > Another spurious blank line?
> 
> I don't think so.
> 
> I think it is the better style to separate
> the error return from the normal return.

Umm ... this blank line changed the file from having one blank line
after the function to having two blank lines after the function.

Re: [RFC 2/4] mm: separate memory allocation and actual work in alloc_vmap_area()

[Roman Gushchin]

On Fri, Dec 14, 2018 at 11:45:00AM -0800, Matthew Wilcox wrote:
> On Fri, Dec 14, 2018 at 11:40:45AM -0800, Joe Perches wrote:
> > On Fri, 2018-12-14 at 10:13 -0800, Matthew Wilcox wrote:
> > > On Fri, Dec 14, 2018 at 10:07:18AM -0800, Roman Gushchin wrote:
> > > > +/*
> > > > + * Allocate a region of KVA of the specified size and alignment, within the
> > > > + * vstart and vend.
> > > > + */
> > > > +static struct vmap_area *alloc_vmap_area(unsigned long size,
> > > > +					 unsigned long align,
> > > > +					 unsigned long vstart,
> > > > +					 unsigned long vend,
> > > > +					 int node, gfp_t gfp_mask)
> > > > +{
> > > > +	struct vmap_area *va;
> > > > +	int ret;
> > > > +
> > > > +	va = kmalloc_node(sizeof(struct vmap_area),
> > > > +			gfp_mask & GFP_RECLAIM_MASK, node);
> > > > +	if (unlikely(!va))
> > > > +		return ERR_PTR(-ENOMEM);
> > > > +
> > > > +	ret = init_vmap_area(va, size, align, vstart, vend, node, gfp_mask);
> > > > +	if (ret) {
> > > > +		kfree(va);
> > > > +		return ERR_PTR(ret);
> > > > +	}
> > > > +
> > > > +	return va;
> > > >  }
> > > >  
> > > > +
> > > 
> > > Another spurious blank line?
> > 
> > I don't think so.
> > 
> > I think it is the better style to separate
> > the error return from the normal return.
> 
> Umm ... this blank line changed the file from having one blank line
> after the function to having two blank lines after the function.
> 

Yes, it's an odd free line (here and above), will remove them in v2.
Thanks!

Re: [RFC 2/4] mm: separate memory allocation and actual work in alloc_vmap_area()

[Joe Perches]

On Fri, 2018-12-14 at 11:45 -0800, Matthew Wilcox wrote:
> On Fri, Dec 14, 2018 at 11:40:45AM -0800, Joe Perches wrote:
> > On Fri, 2018-12-14 at 10:13 -0800, Matthew Wilcox wrote:
> > > On Fri, Dec 14, 2018 at 10:07:18AM -0800, Roman Gushchin wrote:
> > > > +/*
> > > > + * Allocate a region of KVA of the specified size and alignment, within the
> > > > + * vstart and vend.
> > > > + */
> > > > +static struct vmap_area *alloc_vmap_area(unsigned long size,
> > > > +					 unsigned long align,
> > > > +					 unsigned long vstart,
> > > > +					 unsigned long vend,
> > > > +					 int node, gfp_t gfp_mask)
> > > > +{
> > > > +	struct vmap_area *va;
> > > > +	int ret;
> > > > +
> > > > +	va = kmalloc_node(sizeof(struct vmap_area),
> > > > +			gfp_mask & GFP_RECLAIM_MASK, node);
> > > > +	if (unlikely(!va))
> > > > +		return ERR_PTR(-ENOMEM);
> > > > +
> > > > +	ret = init_vmap_area(va, size, align, vstart, vend, node, gfp_mask);
> > > > +	if (ret) {
> > > > +		kfree(va);
> > > > +		return ERR_PTR(ret);
> > > > +	}
> > > > +
> > > > +	return va;
> > > >  }
> > > >  
> > > > +
> > > 
> > > Another spurious blank line?

[wrong spacing noticed]

> Umm ... this blank line changed the file from having one blank line
> after the function to having two blank lines after the function.

right. thanks.

[RFC 3/4] mm: allocate vmalloc metadata in one allocation

[Roman Gushchin]

Currently any vmalloc() call leads to at least 3 metadata allocations:
1) struct vm_struct
2) struct vm_area
3) struct vm_struct->pages array

Especially for small allocations (e.g. 4 pages for kernel stacks,
which seems to be one of the main vmalloc() usages these days),
it creates some measurable overhead.

vm_struct->pages array has almost the same lifetime as vm_struct,
and vm_area has a similar, but is freed lazily. So, it's perfectly
possible to squeeze all 3 allocations into one:
  [ struct vm_struct | struct vm_area | pages array ]

This also slightly simplifies freeing and error handling paths
(e.g. fewer ENOMEM handling points).

On my setup it saves up to 4% cpu on allocation of 1000000 4-pages
areas.

Signed-off-by: Roman Gushchin <guro@fb.com>
Acked-by: Johannes Weiner <hannes@cmpxchg.org>
---
 arch/mips/mm/ioremap.c      |   7 +--
 arch/nios2/mm/ioremap.c     |   4 +-
 arch/sh/kernel/cpu/sh4/sq.c |   5 +-
 arch/sh/mm/ioremap.c        |   8 +--
 arch/x86/mm/ioremap.c       |   4 +-
 include/linux/vmalloc.h     |   3 +-
 mm/vmalloc.c                | 113 +++++++++++++++++++++---------------
 7 files changed, 72 insertions(+), 72 deletions(-)

diff --git a/arch/mips/mm/ioremap.c b/arch/mips/mm/ioremap.c
index 1601d90b087b..22fa47217a52 100644
--- a/arch/mips/mm/ioremap.c
+++ b/arch/mips/mm/ioremap.c
@@ -190,16 +190,11 @@ void __iomem * __ioremap(phys_addr_t phys_addr, phys_addr_t size, unsigned long
 
 void __iounmap(const volatile void __iomem *addr)
 {
-	struct vm_struct *p;
-
 	if (IS_KSEG1(addr))
 		return;
 
-	p = remove_vm_area((void *) (PAGE_MASK & (unsigned long __force) addr));
-	if (!p)
+	if (remove_vm_area((void *) (PAGE_MASK & (unsigned long __force) addr)))
 		printk(KERN_ERR "iounmap: bad address %p\n", addr);
-
-	kfree(p);
 }
 
 EXPORT_SYMBOL(__ioremap);
diff --git a/arch/nios2/mm/ioremap.c b/arch/nios2/mm/ioremap.c
index 3a28177a01eb..0098494940ae 100644
--- a/arch/nios2/mm/ioremap.c
+++ b/arch/nios2/mm/ioremap.c
@@ -179,9 +179,7 @@ void __iounmap(void __iomem *addr)
 	if ((unsigned long) addr > CONFIG_NIOS2_IO_REGION_BASE)
 		return;
 
-	p = remove_vm_area((void *) (PAGE_MASK & (unsigned long __force) addr));
-	if (!p)
+	if (remove_vm_area((void *) (PAGE_MASK & (unsigned long __force) addr)))
 		pr_err("iounmap: bad address %p\n", addr);
-	kfree(p);
 }
 EXPORT_SYMBOL(__iounmap);
diff --git a/arch/sh/kernel/cpu/sh4/sq.c b/arch/sh/kernel/cpu/sh4/sq.c
index 4ca78ed71ad2..50be8449a52d 100644
--- a/arch/sh/kernel/cpu/sh4/sq.c
+++ b/arch/sh/kernel/cpu/sh4/sq.c
@@ -229,10 +229,7 @@ void sq_unmap(unsigned long vaddr)
 		/*
 		 * Tear down the VMA in the MMU case.
 		 */
-		struct vm_struct *vma;
-
-		vma = remove_vm_area((void *)(map->sq_addr & PAGE_MASK));
-		if (!vma) {
+		if (remove_vm_area((void *)(map->sq_addr & PAGE_MASK))) {
 			printk(KERN_ERR "%s: bad address 0x%08lx\n",
 			       __func__, map->sq_addr);
 			return;
diff --git a/arch/sh/mm/ioremap.c b/arch/sh/mm/ioremap.c
index d09ddfe58fd8..160853473afc 100644
--- a/arch/sh/mm/ioremap.c
+++ b/arch/sh/mm/ioremap.c
@@ -106,7 +106,6 @@ static inline int iomapping_nontranslatable(unsigned long offset)
 void __iounmap(void __iomem *addr)
 {
 	unsigned long vaddr = (unsigned long __force)addr;
-	struct vm_struct *p;
 
 	/*
 	 * Nothing to do if there is no translatable mapping.
@@ -126,12 +125,7 @@ void __iounmap(void __iomem *addr)
 	if (pmb_unmap(addr) == 0)
 		return;
 
-	p = remove_vm_area((void *)(vaddr & PAGE_MASK));
-	if (!p) {
+	if (remove_vm_area((void *)(vaddr & PAGE_MASK)))
 		printk(KERN_ERR "%s: bad address %p\n", __func__, addr);
-		return;
-	}
-
-	kfree(p);
 }
 EXPORT_SYMBOL(__iounmap);
diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index 5378d10f1d31..6fb570b8874a 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -433,9 +433,7 @@ void iounmap(volatile void __iomem *addr)
 	free_memtype(p->phys_addr, p->phys_addr + get_vm_area_size(p));
 
 	/* Finally remove it */
-	o = remove_vm_area((void __force *)addr);
-	BUG_ON(p != o || o == NULL);
-	kfree(p);
+	BUG_ON(remove_vm_area((void __force *)addr));
 }
 EXPORT_SYMBOL(iounmap);
 
diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
index 398e9c95cd61..1205f7a03b48 100644
--- a/include/linux/vmalloc.h
+++ b/include/linux/vmalloc.h
@@ -21,6 +21,7 @@ struct notifier_block;		/* in notifier.h */
 #define VM_UNINITIALIZED	0x00000020	/* vm_struct is not fully initialized */
 #define VM_NO_GUARD		0x00000040      /* don't add guard page */
 #define VM_KASAN		0x00000080      /* has allocated kasan shadow memory */
+#define VM_EXT_PAGES		0x00000100	/* pages array is not embedded */
 /* bits [20..32] reserved for arch specific ioremap internals */
 
 /*
@@ -132,7 +133,7 @@ extern struct vm_struct *__get_vm_area_caller(unsigned long size,
 					unsigned long flags,
 					unsigned long start, unsigned long end,
 					const void *caller);
-extern struct vm_struct *remove_vm_area(const void *addr);
+extern int remove_vm_area(const void *addr);
 extern struct vm_struct *find_vm_area(const void *addr);
 
 extern int map_vm_area(struct vm_struct *area, pgprot_t prot,
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 24c8ab28254d..edd76953c23c 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -1388,12 +1388,20 @@ static void clear_vm_uninitialized_flag(struct vm_struct *vm)
 	vm->flags &= ~VM_UNINITIALIZED;
 }
 
+static void *__vmalloc_node(unsigned long size, unsigned long align,
+			    gfp_t gfp_mask, pgprot_t prot,
+			    int node, const void *caller);
+
 static struct vm_struct *__get_vm_area_node(unsigned long size,
 		unsigned long align, unsigned long flags, unsigned long start,
 		unsigned long end, int node, gfp_t gfp_mask, const void *caller)
 {
 	struct vmap_area *va;
 	struct vm_struct *area;
+	size_t area_size = sizeof(struct vmap_area) + sizeof(struct vm_struct);
+	unsigned long nr_pages = 0;
+	unsigned long pages_size;
+	struct page **pages = NULL;
 
 	BUG_ON(in_interrupt());
 	size = PAGE_ALIGN(size);
@@ -1404,19 +1412,57 @@ static struct vm_struct *__get_vm_area_node(unsigned long size,
 		align = 1ul << clamp_t(int, get_count_order_long(size),
 				       PAGE_SHIFT, IOREMAP_MAX_ORDER);
 
-	area = kzalloc_node(sizeof(*area), gfp_mask & GFP_RECLAIM_MASK, node);
-	if (unlikely(!area))
+	if (flags & VM_UNINITIALIZED) {
+		nr_pages = size >> PAGE_SHIFT;
+		pages_size = nr_pages * sizeof(struct page *);
+
+		if (area_size + pages_size <= PAGE_SIZE) {
+			/* area->pages array can be embedded */
+			area_size += nr_pages * sizeof(struct page *);
+		} else {
+			/* area->pages array has to be allocated externally */
+			const gfp_t nested_gfp = (gfp_mask & GFP_RECLAIM_MASK) |
+				__GFP_ZERO;
+			const gfp_t highmem_mask =
+				(gfp_mask & (GFP_DMA | GFP_DMA32)) ?
+				0 : __GFP_HIGHMEM;
+
+			flags |= VM_EXT_PAGES;
+			pages = __vmalloc_node(pages_size, 1,
+					       nested_gfp | highmem_mask,
+					       PAGE_KERNEL, node, caller);
+			if (!pages)
+				return NULL;
+		}
+	}
+
+	/*
+	 * We allocate va, area and optionally area->pages at once.
+	 */
+	va = kzalloc_node(area_size, gfp_mask & GFP_RECLAIM_MASK, node);
+	if (unlikely(!va)) {
+		vfree(pages);
 		return NULL;
+	}
 
 	if (!(flags & VM_NO_GUARD))
 		size += PAGE_SIZE;
 
-	va = alloc_vmap_area(size, align, start, end, node, gfp_mask);
-	if (IS_ERR(va)) {
-		kfree(area);
+	if (init_vmap_area(va, size, align, start, end, node, gfp_mask)) {
+		vfree(pages);
+		kfree(va);
 		return NULL;
 	}
 
+	area = (struct vm_struct *)(va + 1);
+	if (flags & VM_UNINITIALIZED) {
+		area->nr_pages = nr_pages;
+		if (flags & VM_EXT_PAGES)
+			area->pages = pages;
+		else
+			area->pages = (struct page **)(area + 1);
+	}
+
 	setup_vmalloc_vm(area, va, flags, caller);
 
 	return area;
@@ -1480,7 +1526,7 @@ struct vm_struct *find_vm_area(const void *addr)
 	return NULL;
 }
 
-static struct vm_struct *__remove_vm_area(struct vmap_area *va)
+static void __remove_vm_area(struct vmap_area *va)
 {
 	struct vm_struct *vm = va->vm;
 
@@ -1494,8 +1540,6 @@ static struct vm_struct *__remove_vm_area(struct vmap_area *va)
 
 	kasan_free_shadow(vm);
 	free_unmap_vmap_area(va);
-
-	return vm;
 }
 
 /**
@@ -1503,19 +1547,18 @@ static struct vm_struct *__remove_vm_area(struct vmap_area *va)
  *	@addr:		base address
  *
  *	Search for the kernel VM area starting at @addr, and remove it.
- *	This function returns the found VM area, but using it is NOT safe
- *	on SMP machines, except for its size or flags.
  */
-struct vm_struct *remove_vm_area(const void *addr)
+int remove_vm_area(const void *addr)
 {
-	struct vm_struct *vm = NULL;
 	struct vmap_area *va;
 
 	va = find_vmap_area((unsigned long)addr);
-	if (va && va->flags & VM_VM_AREA)
-		vm = __remove_vm_area(va);
+	if (va && va->flags & VM_VM_AREA) {
+		__remove_vm_area(va);
+		return 0;
+	}
 
-	return vm;
+	return -EFAULT;
 }
 
 static void __vunmap(const void *addr, int deallocate_pages)
@@ -1552,10 +1595,11 @@ static void __vunmap(const void *addr, int deallocate_pages)
 			__free_pages(page, 0);
 		}
 
-		kvfree(area->pages);
+		if (area->flags & VM_EXT_PAGES)
+			kvfree(area->pages);
 	}
 
-	kfree(area);
+	WARN_ON(area != (struct vm_struct *)(va + 1));
 }
 
 static inline void __vfree_deferred(const void *addr)
@@ -1676,37 +1720,13 @@ void *vmap(struct page **pages, unsigned int count,
 }
 EXPORT_SYMBOL(vmap);
 
-static void *__vmalloc_node(unsigned long size, unsigned long align,
-			    gfp_t gfp_mask, pgprot_t prot,
-			    int node, const void *caller);
 static void *__vmalloc_area_node(struct vm_struct *area, gfp_t gfp_mask,
 				 pgprot_t prot, int node)
 {
-	struct page **pages;
-	unsigned int nr_pages, array_size, i;
-	const gfp_t nested_gfp = (gfp_mask & GFP_RECLAIM_MASK) | __GFP_ZERO;
 	const gfp_t alloc_mask = gfp_mask | __GFP_NOWARN;
 	const gfp_t highmem_mask = (gfp_mask & (GFP_DMA | GFP_DMA32)) ?
-					0 :
-					__GFP_HIGHMEM;
-
-	nr_pages = get_vm_area_size(area) >> PAGE_SHIFT;
-	array_size = (nr_pages * sizeof(struct page *));
-
-	area->nr_pages = nr_pages;
-	/* Please note that the recursion is strictly bounded. */
-	if (array_size > PAGE_SIZE) {
-		pages = __vmalloc_node(array_size, 1, nested_gfp|highmem_mask,
-				PAGE_KERNEL, node, area->caller);
-	} else {
-		pages = kmalloc_node(array_size, nested_gfp, node);
-	}
-	area->pages = pages;
-	if (!area->pages) {
-		remove_vm_area(area->addr);
-		kfree(area);
-		return NULL;
-	}
+		0 : __GFP_HIGHMEM;
+	unsigned int i;
 
 	for (i = 0; i < area->nr_pages; i++) {
 		struct page *page;
@@ -1726,7 +1746,7 @@ static void *__vmalloc_area_node(struct vm_struct *area, gfp_t gfp_mask,
 			cond_resched();
 	}
 
-	if (map_vm_area(area, prot, pages))
+	if (map_vm_area(area, prot, area->pages))
 		goto fail;
 	return area->addr;
 
@@ -2377,10 +2397,7 @@ EXPORT_SYMBOL_GPL(alloc_vm_area);
 
 void free_vm_area(struct vm_struct *area)
 {
-	struct vm_struct *ret;
-	ret = remove_vm_area(area->addr);
-	BUG_ON(ret != area);
-	kfree(area);
+	BUG_ON(remove_vm_area(area->addr));
 }
 EXPORT_SYMBOL_GPL(free_vm_area);
 
-- 
2.19.2

[RFC 4/4] mm: show number of vmalloc pages in /proc/meminfo

[Roman Gushchin]

Vmalloc() is getting more and more used these days (kernel stacks,
bpf and percpu allocator are new top users), and the total %
of memory consumed by vmalloc() can be pretty significant
and changes dynamically.

/proc/meminfo is the best place to display this information:
its top goal is to show top consumers of the memory.

Since the VmallocUsed field in /proc/meminfo is not in use
for quite a long time (it has been defined to 0 by the
commit a5ad88ce8c7f ("mm: get rid of 'vmalloc_info' from
/proc/meminfo")), let's reuse it for showing the actual
physical memory consumption of vmalloc().

Signed-off-by: Roman Gushchin <guro@fb.com>
Acked-by: Johannes Weiner <hannes@cmpxchg.org>
---
 fs/proc/meminfo.c       |  2 +-
 include/linux/vmalloc.h |  3 +++
 mm/vmalloc.c            | 10 ++++++++++
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/fs/proc/meminfo.c b/fs/proc/meminfo.c
index 568d90e17c17..465ea0153b2a 100644
--- a/fs/proc/meminfo.c
+++ b/fs/proc/meminfo.c
@@ -120,7 +120,7 @@ static int meminfo_proc_show(struct seq_file *m, void *v)
 	show_val_kb(m, "Committed_AS:   ", committed);
 	seq_printf(m, "VmallocTotal:   %8lu kB\n",
 		   (unsigned long)VMALLOC_TOTAL >> 10);
-	show_val_kb(m, "VmallocUsed:    ", 0ul);
+	show_val_kb(m, "VmallocUsed:    ", vmalloc_nr_pages());
 	show_val_kb(m, "VmallocChunk:   ", 0ul);
 	show_val_kb(m, "Percpu:         ", pcpu_nr_pages());
 
diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
index 1205f7a03b48..ea3a0fb3a60f 100644
--- a/include/linux/vmalloc.h
+++ b/include/linux/vmalloc.h
@@ -64,10 +64,12 @@ extern void vm_unmap_aliases(void);
 
 #ifdef CONFIG_MMU
 extern void __init vmalloc_init(void);
+extern unsigned long vmalloc_nr_pages(void);
 #else
 static inline void vmalloc_init(void)
 {
 }
+static inline unsigned long vmalloc_nr_pages(void) { return 0; }
 #endif
 
 extern void *vmalloc(unsigned long size);
@@ -83,6 +85,7 @@ extern void *__vmalloc_node_range(unsigned long size, unsigned long align,
 			unsigned long start, unsigned long end, gfp_t gfp_mask,
 			pgprot_t prot, unsigned long vm_flags, int node,
 			const void *caller);
+
 #ifndef CONFIG_MMU
 extern void *__vmalloc_node_flags(unsigned long size, int node, gfp_t flags);
 static inline void *__vmalloc_node_flags_caller(unsigned long size, int node,
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index edd76953c23c..39db12fe8931 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -340,6 +340,13 @@ static unsigned long cached_align;
 
 static unsigned long vmap_area_pcpu_hole;
 
+atomic_long_t nr_vmalloc_pages;
+
+unsigned long vmalloc_nr_pages(void)
+{
+	return atomic_long_read(&nr_vmalloc_pages);
+}
+
 static struct vmap_area *__find_vmap_area(unsigned long addr)
 {
 	struct rb_node *n = vmap_area_root.rb_node;
@@ -1594,6 +1601,7 @@ static void __vunmap(const void *addr, int deallocate_pages)
 			BUG_ON(!page);
 			__free_pages(page, 0);
 		}
+		atomic_long_sub(area->nr_pages, &nr_vmalloc_pages);
 
 		if (area->flags & VM_EXT_PAGES)
 			kvfree(area->pages);
@@ -1739,12 +1747,14 @@ static void *__vmalloc_area_node(struct vm_struct *area, gfp_t gfp_mask,
 		if (unlikely(!page)) {
 			/* Successfully allocated i pages, free them in __vunmap() */
 			area->nr_pages = i;
+			atomic_long_add(area->nr_pages, &nr_vmalloc_pages);
 			goto fail;
 		}
 		area->pages[i] = page;
 		if (gfpflags_allow_blocking(gfp_mask|highmem_mask))
 			cond_resched();
 	}
+	atomic_long_add(area->nr_pages, &nr_vmalloc_pages);
 
 	if (map_vm_area(area, prot, area->pages))
 		goto fail;
-- 
2.19.2

Re: [RFC 4/4] mm: show number of vmalloc pages in /proc/meminfo

[Matthew Wilcox]

On Fri, Dec 14, 2018 at 10:07:20AM -0800, Roman Gushchin wrote:
> Vmalloc() is getting more and more used these days (kernel stacks,
> bpf and percpu allocator are new top users), and the total %
> of memory consumed by vmalloc() can be pretty significant
> and changes dynamically.
> 
> /proc/meminfo is the best place to display this information:
> its top goal is to show top consumers of the memory.
> 
> Since the VmallocUsed field in /proc/meminfo is not in use
> for quite a long time (it has been defined to 0 by the
> commit a5ad88ce8c7f ("mm: get rid of 'vmalloc_info' from
> /proc/meminfo")), let's reuse it for showing the actual
> physical memory consumption of vmalloc().

Do you see significant contention on nr_vmalloc_pages?  Also, if it's
just an atomic_long_t, is it worth having an accessor for it?  And if
it is worth having an accessor for it, then it can be static.

Also, I seem to be missing 3/4.

Re: [RFC 4/4] mm: show number of vmalloc pages in /proc/meminfo

[Roman Gushchin]

On Fri, Dec 14, 2018 at 10:29:04AM -0800, Matthew Wilcox wrote:
> On Fri, Dec 14, 2018 at 10:07:20AM -0800, Roman Gushchin wrote:
> > Vmalloc() is getting more and more used these days (kernel stacks,
> > bpf and percpu allocator are new top users), and the total %
> > of memory consumed by vmalloc() can be pretty significant
> > and changes dynamically.
> > 
> > /proc/meminfo is the best place to display this information:
> > its top goal is to show top consumers of the memory.
> > 
> > Since the VmallocUsed field in /proc/meminfo is not in use
> > for quite a long time (it has been defined to 0 by the
> > commit a5ad88ce8c7f ("mm: get rid of 'vmalloc_info' from
> > /proc/meminfo")), let's reuse it for showing the actual
> > physical memory consumption of vmalloc().
> 
> Do you see significant contention on nr_vmalloc_pages?  Also, if it's
> just an atomic_long_t, is it worth having an accessor for it?  And if
> it is worth having an accessor for it, then it can be static.

Not really, so I decided that per-cpu counter is an overkill
right now; but we can easily switch over once we'll notice any contention.
Will add static.

> 
> Also, I seem to be missing 3/4.
> 

Hm, https://lkml.org/lkml/2018/12/14/1048 ?

Thanks!