[PATCH 0/3] vmalloc enhancements

[PATCH 0/3] 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) separates memory allocation and data initialization
  in alloc_vmap_area()
Patch (3) adds vmalloc counter to /proc/meminfo.

v3->v2:
  - switched back to atomic after more accurate perf measurements:
  no visible perf difference
  - added perf stacktraces in commmit message of (1)

v2->v1:
  - rebased on top of current mm tree
  - switch from atomic to percpu vmalloc page counter

RFC->v1:
  - removed bogus empty lines (suggested by Matthew Wilcox)
  - made nr_vmalloc_pages static (suggested by Matthew Wilcox)
  - dropped patch 3 from RFC patchset, will post later with
  some other changes
  - dropped RFC

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

 fs/proc/meminfo.c       |   2 +-
 include/linux/vmalloc.h |   2 +
 mm/vmalloc.c            | 107 ++++++++++++++++++++++++++--------------
 3 files changed, 73 insertions(+), 38 deletions(-)

-- 
2.20.1

[PATCH 1/3] 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 5-10% speed up on vfree()'ing 1000000
of 4-pages vmalloc blocks.

Perf report before:
  22.64%  cat      [kernel.vmlinux]  [k] free_pcppages_bulk
  10.30%  cat      [kernel.vmlinux]  [k] __vunmap
   9.80%  cat      [kernel.vmlinux]  [k] find_vmap_area
   8.11%  cat      [kernel.vmlinux]  [k] vunmap_page_range
   4.20%  cat      [kernel.vmlinux]  [k] __slab_free
   3.56%  cat      [kernel.vmlinux]  [k] __list_del_entry_valid
   3.46%  cat      [kernel.vmlinux]  [k] smp_call_function_many
   3.33%  cat      [kernel.vmlinux]  [k] kfree
   3.32%  cat      [kernel.vmlinux]  [k] free_unref_page

Perf report after:
  23.01%  cat      [kernel.kallsyms]  [k] free_pcppages_bulk
   9.46%  cat      [kernel.kallsyms]  [k] __vunmap
   9.15%  cat      [kernel.kallsyms]  [k] vunmap_page_range
   6.17%  cat      [kernel.kallsyms]  [k] __slab_free
   5.61%  cat      [kernel.kallsyms]  [k] kfree
   4.86%  cat      [kernel.kallsyms]  [k] bad_range
   4.67%  cat      [kernel.kallsyms]  [k] free_unref_page_commit
   4.24%  cat      [kernel.kallsyms]  [k] __list_del_entry_valid
   3.68%  cat      [kernel.kallsyms]  [k] free_unref_page
   3.65%  cat      [kernel.kallsyms]  [k] __list_add_valid
   3.19%  cat      [kernel.kallsyms]  [k] __purge_vmap_area_lazy
   3.10%  cat      [kernel.kallsyms]  [k] find_vmap_area
   3.05%  cat      [kernel.kallsyms]  [k] rcu_cblist_dequeue

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

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index b7455d4c8c12..8f0179895fb5 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -1477,6 +1477,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
@@ -1489,31 +1507,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;
@@ -1522,17 +1529,18 @@ static void __vunmap(const void *addr, int deallocate_pages)
 			addr))
 		return;
 
-	area = find_vm_area(addr);
-	if (unlikely(!area)) {
+	va = find_vmap_area((unsigned long)addr);
+	if (unlikely(!va || !(va->flags & VM_VM_AREA))) {
 		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;
 
@@ -1547,7 +1555,6 @@ static void __vunmap(const void *addr, int deallocate_pages)
 	}
 
 	kfree(area);
-	return;
 }
 
 static inline void __vfree_deferred(const void *addr)
-- 
2.20.1

[PATCH 2/3] 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>
Reviewed-by: Matthew Wilcox <willy@infradead.org>
---
 mm/vmalloc.c | 50 +++++++++++++++++++++++++++++++++-----------------
 1 file changed, 33 insertions(+), 17 deletions(-)

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 8f0179895fb5..f1f19d1105c4 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -395,16 +395,10 @@ 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 +410,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.
@@ -516,7 +505,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);
@@ -538,8 +527,35 @@ 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)
-- 
2.20.1

[PATCH 3/3] 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 |  2 ++
 mm/vmalloc.c            | 10 ++++++++++
 3 files changed, 13 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 398e9c95cd61..0b497408272b 100644
--- a/include/linux/vmalloc.h
+++ b/include/linux/vmalloc.h
@@ -63,10 +63,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);
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index f1f19d1105c4..3a1872ee8294 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;
 
+static 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;
@@ -1566,6 +1573,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);
 
 		kvfree(area->pages);
 	}
@@ -1742,12 +1750,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, pages))
 		goto fail;
-- 
2.20.1

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

[Vlastimil Babka]

On 2/25/19 9:30 PM, 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 5-10% speed up on vfree()'ing 1000000
> of 4-pages vmalloc blocks.
> 
> Perf report before:
>   22.64%  cat      [kernel.vmlinux]  [k] free_pcppages_bulk
>   10.30%  cat      [kernel.vmlinux]  [k] __vunmap
>    9.80%  cat      [kernel.vmlinux]  [k] find_vmap_area
>    8.11%  cat      [kernel.vmlinux]  [k] vunmap_page_range
>    4.20%  cat      [kernel.vmlinux]  [k] __slab_free
>    3.56%  cat      [kernel.vmlinux]  [k] __list_del_entry_valid
>    3.46%  cat      [kernel.vmlinux]  [k] smp_call_function_many
>    3.33%  cat      [kernel.vmlinux]  [k] kfree
>    3.32%  cat      [kernel.vmlinux]  [k] free_unref_page
> 
> Perf report after:
>   23.01%  cat      [kernel.kallsyms]  [k] free_pcppages_bulk
>    9.46%  cat      [kernel.kallsyms]  [k] __vunmap
>    9.15%  cat      [kernel.kallsyms]  [k] vunmap_page_range
>    6.17%  cat      [kernel.kallsyms]  [k] __slab_free
>    5.61%  cat      [kernel.kallsyms]  [k] kfree
>    4.86%  cat      [kernel.kallsyms]  [k] bad_range
>    4.67%  cat      [kernel.kallsyms]  [k] free_unref_page_commit
>    4.24%  cat      [kernel.kallsyms]  [k] __list_del_entry_valid
>    3.68%  cat      [kernel.kallsyms]  [k] free_unref_page
>    3.65%  cat      [kernel.kallsyms]  [k] __list_add_valid
>    3.19%  cat      [kernel.kallsyms]  [k] __purge_vmap_area_lazy
>    3.10%  cat      [kernel.kallsyms]  [k] find_vmap_area
>    3.05%  cat      [kernel.kallsyms]  [k] rcu_cblist_dequeue
> 
> Signed-off-by: Roman Gushchin <guro@fb.com>
> Cc: Johannes Weiner <hannes@cmpxchg.org>
> Cc: Matthew Wilcox <willy@infradead.org>

Acked-by: Vlastimil Babka <vbabka@suse.cz>

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

[Vlastimil Babka]

On 2/25/19 9:30 PM, 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.

Hmm, but that doesn't happen here or in the later patch, right? The only
caller of init_vmap_area() is alloc_vmap_area(). What am I missing?

> 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>
> Reviewed-by: Matthew Wilcox <willy@infradead.org>

Re: [PATCH 3/3] mm: show number of vmalloc pages in /proc/meminfo

[Vlastimil Babka]

On 2/25/19 9:30 PM, 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

Hm that commit is not that old (2015) and talks about two caching
approaches from Linus and Ingo, so CCing them here for input, as
apparently it was not deemed worth the trouble at that time.

> 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 |  2 ++
>  mm/vmalloc.c            | 10 ++++++++++
>  3 files changed, 13 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 398e9c95cd61..0b497408272b 100644
> --- a/include/linux/vmalloc.h
> +++ b/include/linux/vmalloc.h
> @@ -63,10 +63,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);
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index f1f19d1105c4..3a1872ee8294 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;
>  
> +static 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;
> @@ -1566,6 +1573,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);
>  
>  		kvfree(area->pages);
>  	}
> @@ -1742,12 +1750,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, pages))
>  		goto fail;
> 

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

[Roman Gushchin]

On Fri, Mar 01, 2019 at 03:43:19PM +0100, Vlastimil Babka wrote:
> On 2/25/19 9:30 PM, 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.
> 
> Hmm, but that doesn't happen here or in the later patch, right? The only
> caller of init_vmap_area() is alloc_vmap_area(). What am I missing?

So initially the patch was a part of a bigger patchset, which
tried to minimize the number of separate allocations during vmalloc(),
e.g. by inlining vm_struct->pages into vm_struct for small areas.

I temporarily dropped the rest of the patchset for some rework,
but decided to leave this patch, because it looks like a nice refactoring
in any case, and also it has been already reviewed and acked by Matthew
and Johannes.

Thank you for looking into it!

Re: [PATCH 0/3] vmalloc enhancements

[Roman Gushchin]

On Mon, Feb 25, 2019 at 12:30:34PM -0800, Roman Gushchin wrote:
> 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) separates memory allocation and data initialization
>   in alloc_vmap_area()
> Patch (3) adds vmalloc counter to /proc/meminfo.
> 
> v3->v2:
>   - switched back to atomic after more accurate perf measurements:
>   no visible perf difference
>   - added perf stacktraces in commmit message of (1)
> 
> v2->v1:
>   - rebased on top of current mm tree
>   - switch from atomic to percpu vmalloc page counter
> 
> RFC->v1:
>   - removed bogus empty lines (suggested by Matthew Wilcox)
>   - made nr_vmalloc_pages static (suggested by Matthew Wilcox)
>   - dropped patch 3 from RFC patchset, will post later with
>   some other changes
>   - dropped RFC
> 
> Roman Gushchin (3):
>   mm: refactor __vunmap() to avoid duplicated call to find_vm_area()
>   mm: separate memory allocation and actual work in alloc_vmap_area()
>   mm: show number of vmalloc pages in /proc/meminfo
> 
>  fs/proc/meminfo.c       |   2 +-
>  include/linux/vmalloc.h |   2 +
>  mm/vmalloc.c            | 107 ++++++++++++++++++++++++++--------------
>  3 files changed, 73 insertions(+), 38 deletions(-)
> 
> -- 
> 2.20.1
> 

Ping. Any comments/suggestions/objections?

Thanks!

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

[Vlastimil Babka]

On 3/1/19 5:48 PM, Roman Gushchin wrote:
> On Fri, Mar 01, 2019 at 03:43:19PM +0100, Vlastimil Babka wrote:
>> On 2/25/19 9:30 PM, 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.
>>
>> Hmm, but that doesn't happen here or in the later patch, right? The only
>> caller of init_vmap_area() is alloc_vmap_area(). What am I missing?
> 
> So initially the patch was a part of a bigger patchset, which
> tried to minimize the number of separate allocations during vmalloc(),
> e.g. by inlining vm_struct->pages into vm_struct for small areas.
> 
> I temporarily dropped the rest of the patchset for some rework,
> but decided to leave this patch, because it looks like a nice refactoring
> in any case, and also it has been already reviewed and acked by Matthew
> and Johannes.

OK then,

Acked-by: Vlastimil Babka <vbabka@suse.cz>

> Thank you for looking into it!
> 

Re: [PATCH 3/3] mm: show number of vmalloc pages in /proc/meminfo

[Vlastimil Babka]

On 3/1/19 4:05 PM, Vlastimil Babka wrote:
> On 2/25/19 9:30 PM, 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
> 
> Hm that commit is not that old (2015) and talks about two caching
> approaches from Linus and Ingo, so CCing them here for input, as
> apparently it was not deemed worth the trouble at that time.

No reply, so I've dug up the 2015 threads [1] [2] and went through them
quickly. Seems like the idea was to keep the expensive
get_vmalloc_info() implementation but cache its results. Dunno why a
continuously updated atomic counter was not proposed, like you did now.
Perhaps the implementation changed since then. Anyway it makes a lot of
sense to me. The updates shouldn't be too frequent to cause contention.

[1]
https://lore.kernel.org/lkml/CA+55aFxzOAx7365Mx2o55TZOS+bZGh_Pfr=vVF3QGg0btsDumg@mail.gmail.com/T/#u
[2]
https://lore.kernel.org/lkml/20150825125951.GR16853@twins.programming.kicks-ass.net/T/#e25cc3fd84ccfc5f03a347ba31fa99a132e8c8ca3

> 
>> physical memory consumption of vmalloc().
>>
>> Signed-off-by: Roman Gushchin <guro@fb.com>
>> Acked-by: Johannes Weiner <hannes@cmpxchg.org>

Acked-by: Vlastimil Babka <vbabka@suse.cz>

>> ---
>>  fs/proc/meminfo.c       |  2 +-
>>  include/linux/vmalloc.h |  2 ++
>>  mm/vmalloc.c            | 10 ++++++++++
>>  3 files changed, 13 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 398e9c95cd61..0b497408272b 100644
>> --- a/include/linux/vmalloc.h
>> +++ b/include/linux/vmalloc.h
>> @@ -63,10 +63,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);
>> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
>> index f1f19d1105c4..3a1872ee8294 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;
>>  
>> +static 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;
>> @@ -1566,6 +1573,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);
>>  
>>  		kvfree(area->pages);
>>  	}
>> @@ -1742,12 +1750,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, pages))
>>  		goto fail;
>>
> 

Re: [PATCH 0/3] vmalloc enhancements

[Vlastimil Babka]

On 3/29/19 11:07 PM, Roman Gushchin wrote:
>> Roman Gushchin (3):
>>   mm: refactor __vunmap() to avoid duplicated call to find_vm_area()
>>   mm: separate memory allocation and actual work in alloc_vmap_area()
>>   mm: show number of vmalloc pages in /proc/meminfo
>>
>>  fs/proc/meminfo.c       |   2 +-
>>  include/linux/vmalloc.h |   2 +
>>  mm/vmalloc.c            | 107 ++++++++++++++++++++++++++--------------
>>  3 files changed, 73 insertions(+), 38 deletions(-)
>>
>> -- 
>> 2.20.1
>>
> 
> Ping. Any comments/suggestions/objections?

I suspect my questions for 3/3 effectively blocked this, so now it
should be unblocked. Haven't checked how this is affected by the
"improve vmap allocation" series, but you were following it closely and
thus should know better.

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

[Roman Gushchin]

On Wed, Apr 17, 2019 at 03:27:56PM +0200, Vlastimil Babka wrote:
> On 3/1/19 5:48 PM, Roman Gushchin wrote:
> > On Fri, Mar 01, 2019 at 03:43:19PM +0100, Vlastimil Babka wrote:
> >> On 2/25/19 9:30 PM, 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.
> >>
> >> Hmm, but that doesn't happen here or in the later patch, right? The only
> >> caller of init_vmap_area() is alloc_vmap_area(). What am I missing?
> > 
> > So initially the patch was a part of a bigger patchset, which
> > tried to minimize the number of separate allocations during vmalloc(),
> > e.g. by inlining vm_struct->pages into vm_struct for small areas.
> > 
> > I temporarily dropped the rest of the patchset for some rework,
> > but decided to leave this patch, because it looks like a nice refactoring
> > in any case, and also it has been already reviewed and acked by Matthew
> > and Johannes.
> 
> OK then,
> 
> Acked-by: Vlastimil Babka <vbabka@suse.cz>

Thank you for looking into this and other patches from the series!

Btw, it looks like that recent changes in vmalloc code are in a conflict
with this patch, so I'll drop it for now, and will resend two other as v4.

Thanks!

Re: [PATCH 0/3] vmalloc enhancements

[Roman Gushchin]

On Mon, Feb 11, 2019 at 11:18:45AM -0800, Andrew Morton wrote:
> On Mon, 11 Feb 2019 14:08:22 -0500 Johannes Weiner <hannes@cmpxchg.org> wrote:
> 
> > On Wed, Dec 19, 2018 at 09:37:48AM -0800, Roman Gushchin wrote:
> > > 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.
> >
> > These slipped through the cracks. +CC Andrew directly.
> > Andrew, if it's not too late yet, could you consider them for 5.1?
> > 
> 
> There's been some activity in vmalloc.c lately and these have
> bitrotted.  They'll need a redo, please.
> 

Will do. Thanks!

Re: [PATCH 0/3] vmalloc enhancements

[Andrew Morton]

On Mon, 11 Feb 2019 14:08:22 -0500 Johannes Weiner <hannes@cmpxchg.org> wrote:

> On Wed, Dec 19, 2018 at 09:37:48AM -0800, Roman Gushchin wrote:
> > 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.
>
> These slipped through the cracks. +CC Andrew directly.
> Andrew, if it's not too late yet, could you consider them for 5.1?
> 

There's been some activity in vmalloc.c lately and these have
bitrotted.  They'll need a redo, please.

Re: [PATCH 0/3] vmalloc enhancements

[Johannes Weiner]

These slipped through the cracks. +CC Andrew directly.
Andrew, if it's not too late yet, could you consider them for 5.1?

On Wed, Dec 19, 2018 at 09:37:48AM -0800, Roman Gushchin wrote:
> 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) separates memory allocation and data initialization
>   in alloc_vmap_area()
> Patch (3) adds vmalloc counter to /proc/meminfo.
> 
> RFC->v1:
>   - removed bogus empty lines (suggested by Matthew Wilcox)
>   - made nr_vmalloc_pages static (suggested by Matthew Wilcox)
>   - dropped patch 3 from RFC patchset, will post later with
>   some other changes
>   - dropped RFC
> 
> Roman Gushchin (3):
>   mm: refactor __vunmap() to avoid duplicated call to find_vm_area()
>   mm: separate memory allocation and actual work in alloc_vmap_area()
>   mm: show number of vmalloc pages in /proc/meminfo
> 
>  fs/proc/meminfo.c       |   2 +-
>  include/linux/vmalloc.h |   2 +
>  mm/vmalloc.c            | 107 ++++++++++++++++++++++++++--------------
>  3 files changed, 73 insertions(+), 38 deletions(-)
> 
> -- 
> 2.19.2
> 

[PATCH 0/3] 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) separates memory allocation and data initialization
  in alloc_vmap_area()
Patch (3) adds vmalloc counter to /proc/meminfo.

RFC->v1:
  - removed bogus empty lines (suggested by Matthew Wilcox)
  - made nr_vmalloc_pages static (suggested by Matthew Wilcox)
  - dropped patch 3 from RFC patchset, will post later with
  some other changes
  - dropped RFC

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

 fs/proc/meminfo.c       |   2 +-
 include/linux/vmalloc.h |   2 +
 mm/vmalloc.c            | 107 ++++++++++++++++++++++++++--------------
 3 files changed, 73 insertions(+), 38 deletions(-)

-- 
2.19.2