linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] mm: refactor __vunmap() to avoid duplicated call to find_vm_area()
@ 2019-05-14 23:51 Roman Gushchin
  2019-05-14 23:51 ` [PATCH RESEND] mm: show number of vmalloc pages in /proc/meminfo Roman Gushchin
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Roman Gushchin @ 2019-05-14 23:51 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-mm, linux-kernel, kernel-team, Johannes Weiner,
	Roman Gushchin, Matthew Wilcox, Vlastimil Babka

__vunmap() calls find_vm_area() twice without an obvious reason:
first directly to get the area pointer, second indirectly by calling
vm_remove_mappings()->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. Let's pass
a pointer to the vm_area instead of vm_struct to vm_remove_mappings(),
so it can pass it to __remove_vm_area() and avoid the redundant area
lookup.

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

Perf report before:
  29.44%  cat      [kernel.kallsyms]  [k] free_unref_page
  11.88%  cat      [kernel.kallsyms]  [k] find_vmap_area
   9.28%  cat      [kernel.kallsyms]  [k] __free_pages
   7.44%  cat      [kernel.kallsyms]  [k] __slab_free
   7.28%  cat      [kernel.kallsyms]  [k] vunmap_page_range
   4.56%  cat      [kernel.kallsyms]  [k] __vunmap
   3.64%  cat      [kernel.kallsyms]  [k] __purge_vmap_area_lazy
   3.04%  cat      [kernel.kallsyms]  [k] __free_vmap_area

Perf report after:
  32.41%  cat      [kernel.kallsyms]  [k] free_unref_page
   7.79%  cat      [kernel.kallsyms]  [k] find_vmap_area
   7.40%  cat      [kernel.kallsyms]  [k] __slab_free
   7.31%  cat      [kernel.kallsyms]  [k] vunmap_page_range
   6.84%  cat      [kernel.kallsyms]  [k] __free_pages
   6.01%  cat      [kernel.kallsyms]  [k] __vunmap
   3.98%  cat      [kernel.kallsyms]  [k] smp_call_function_single
   3.81%  cat      [kernel.kallsyms]  [k] __purge_vmap_area_lazy
   2.77%  cat      [kernel.kallsyms]  [k] __free_vmap_area

Signed-off-by: Roman Gushchin <guro@fb.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Vlastimil Babka <vbabka@suse.cz>
---
 mm/vmalloc.c | 52 +++++++++++++++++++++++++++++-----------------------
 1 file changed, 29 insertions(+), 23 deletions(-)

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index c42872ed82ac..8d4907865614 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -2075,6 +2075,22 @@ 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;
+
+	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
@@ -2087,26 +2103,14 @@ 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 inline void set_area_direct_map(const struct vm_struct *area,
@@ -2119,9 +2123,10 @@ static inline void set_area_direct_map(const struct vm_struct *area,
 			set_direct_map(area->pages[i]);
 }
 
-/* Handle removing and resetting vm mappings related to the vm_struct. */
-static void vm_remove_mappings(struct vm_struct *area, int deallocate_pages)
+/* Handle removing and resetting vm mappings related to the va->vm vm_struct. */
+static void vm_remove_mappings(struct vmap_area *va, int deallocate_pages)
 {
+	struct vm_struct *area = va->vm;
 	unsigned long addr = (unsigned long)area->addr;
 	unsigned long start = ULONG_MAX, end = 0;
 	int flush_reset = area->flags & VM_FLUSH_RESET_PERMS;
@@ -2138,7 +2143,7 @@ static void vm_remove_mappings(struct vm_struct *area, int deallocate_pages)
 		set_memory_rw(addr, area->nr_pages);
 	}
 
-	remove_vm_area(area->addr);
+	__remove_vm_area(va);
 
 	/* If this is not VM_FLUSH_RESET_PERMS memory, no need for the below. */
 	if (!flush_reset)
@@ -2178,6 +2183,7 @@ static void vm_remove_mappings(struct vm_struct *area, int deallocate_pages)
 static void __vunmap(const void *addr, int deallocate_pages)
 {
 	struct vm_struct *area;
+	struct vmap_area *va;
 
 	if (!addr)
 		return;
@@ -2186,17 +2192,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));
 
-	vm_remove_mappings(area, deallocate_pages);
+	vm_remove_mappings(va, deallocate_pages);
 
 	if (deallocate_pages) {
 		int i;
@@ -2212,7 +2219,6 @@ static void __vunmap(const void *addr, int deallocate_pages)
 	}
 
 	kfree(area);
-	return;
 }
 
 static inline void __vfree_deferred(const void *addr)
-- 
2.20.1


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

* [PATCH RESEND] mm: show number of vmalloc pages in /proc/meminfo
  2019-05-14 23:51 [PATCH] mm: refactor __vunmap() to avoid duplicated call to find_vm_area() Roman Gushchin
@ 2019-05-14 23:51 ` Roman Gushchin
  2019-05-15  7:14   ` Anshuman Khandual
  2019-07-09  5:59   ` Minchan Kim
  2019-05-15  4:27 ` [PATCH] mm: refactor __vunmap() to avoid duplicated call to find_vm_area() Anshuman Khandual
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 11+ messages in thread
From: Roman Gushchin @ 2019-05-14 23:51 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-mm, linux-kernel, kernel-team, Johannes Weiner, 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>
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 51e131245379..9b21d0047710 100644
--- a/include/linux/vmalloc.h
+++ b/include/linux/vmalloc.h
@@ -72,10 +72,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 8d4907865614..65871ddba497 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -398,6 +398,13 @@ static void purge_vmap_area_lazy(void);
 static BLOCKING_NOTIFIER_HEAD(vmap_notify_list);
 static unsigned long lazy_max_pages(void);
 
+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;
@@ -2214,6 +2221,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);
 	}
@@ -2390,12 +2398,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


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

* Re: [PATCH] mm: refactor __vunmap() to avoid duplicated call to find_vm_area()
  2019-05-14 23:51 [PATCH] mm: refactor __vunmap() to avoid duplicated call to find_vm_area() Roman Gushchin
  2019-05-14 23:51 ` [PATCH RESEND] mm: show number of vmalloc pages in /proc/meminfo Roman Gushchin
@ 2019-05-15  4:27 ` Anshuman Khandual
  2019-05-15 17:18   ` Roman Gushchin
  2019-05-15  7:41 ` Anshuman Khandual
  2019-05-15 17:35 ` Ira Weiny
  3 siblings, 1 reply; 11+ messages in thread
From: Anshuman Khandual @ 2019-05-15  4:27 UTC (permalink / raw)
  To: Roman Gushchin, Andrew Morton
  Cc: linux-mm, linux-kernel, kernel-team, Johannes Weiner,
	Matthew Wilcox, Vlastimil Babka



On 05/15/2019 05:21 AM, Roman Gushchin wrote:
> __vunmap() calls find_vm_area() twice without an obvious reason:
> first directly to get the area pointer, second indirectly by calling
> vm_remove_mappings()->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. Let's pass
> a pointer to the vm_area instead of vm_struct to vm_remove_mappings(),
> so it can pass it to __remove_vm_area() and avoid the redundant area
> lookup.
> 
> On my test setup, I've got 5-10% speed up on vfree()'ing 1000000
> of 4-pages vmalloc blocks.

Though results from  1000000 single page vmalloc blocks remain inconclusive,
4-page based vmalloc block's result shows improvement in the range of 5-10%.

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

* Re: [PATCH RESEND] mm: show number of vmalloc pages in /proc/meminfo
  2019-05-14 23:51 ` [PATCH RESEND] mm: show number of vmalloc pages in /proc/meminfo Roman Gushchin
@ 2019-05-15  7:14   ` Anshuman Khandual
  2019-07-09  5:59   ` Minchan Kim
  1 sibling, 0 replies; 11+ messages in thread
From: Anshuman Khandual @ 2019-05-15  7:14 UTC (permalink / raw)
  To: Roman Gushchin, Andrew Morton
  Cc: linux-mm, linux-kernel, kernel-team, Johannes Weiner



On 05/15/2019 05:21 AM, 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().
The primary concern which got addressed with a5ad88ce8c7f was that computing
get_vmalloc_info() was taking long time. But here its reads an already updated
value which gets added or subtracted during __vmalloc_area_node/__vunmap cycle.
Hence this should not cost much (like get_vmalloc_info). But is not this similar
to the caching solution Linus mentioned.

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

* Re: [PATCH] mm: refactor __vunmap() to avoid duplicated call to find_vm_area()
  2019-05-14 23:51 [PATCH] mm: refactor __vunmap() to avoid duplicated call to find_vm_area() Roman Gushchin
  2019-05-14 23:51 ` [PATCH RESEND] mm: show number of vmalloc pages in /proc/meminfo Roman Gushchin
  2019-05-15  4:27 ` [PATCH] mm: refactor __vunmap() to avoid duplicated call to find_vm_area() Anshuman Khandual
@ 2019-05-15  7:41 ` Anshuman Khandual
  2019-05-15 17:17   ` Roman Gushchin
  2019-05-15 17:35 ` Ira Weiny
  3 siblings, 1 reply; 11+ messages in thread
From: Anshuman Khandual @ 2019-05-15  7:41 UTC (permalink / raw)
  To: Roman Gushchin, Andrew Morton
  Cc: linux-mm, linux-kernel, kernel-team, Johannes Weiner,
	Matthew Wilcox, Vlastimil Babka



On 05/15/2019 05:21 AM, Roman Gushchin wrote:
> __vunmap() calls find_vm_area() twice without an obvious reason:
> first directly to get the area pointer, second indirectly by calling
> vm_remove_mappings()->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. Let's pass
> a pointer to the vm_area instead of vm_struct to vm_remove_mappings(),
> so it can pass it to __remove_vm_area() and avoid the redundant area
> lookup.
> 
> On my test setup, I've got 5-10% speed up on vfree()'ing 1000000
> of 4-pages vmalloc blocks.
> 
> Perf report before:
>   29.44%  cat      [kernel.kallsyms]  [k] free_unref_page
>   11.88%  cat      [kernel.kallsyms]  [k] find_vmap_area
>    9.28%  cat      [kernel.kallsyms]  [k] __free_pages
>    7.44%  cat      [kernel.kallsyms]  [k] __slab_free
>    7.28%  cat      [kernel.kallsyms]  [k] vunmap_page_range
>    4.56%  cat      [kernel.kallsyms]  [k] __vunmap
>    3.64%  cat      [kernel.kallsyms]  [k] __purge_vmap_area_lazy
>    3.04%  cat      [kernel.kallsyms]  [k] __free_vmap_area
> 
> Perf report after:
>   32.41%  cat      [kernel.kallsyms]  [k] free_unref_page
>    7.79%  cat      [kernel.kallsyms]  [k] find_vmap_area
>    7.40%  cat      [kernel.kallsyms]  [k] __slab_free
>    7.31%  cat      [kernel.kallsyms]  [k] vunmap_page_range
>    6.84%  cat      [kernel.kallsyms]  [k] __free_pages
>    6.01%  cat      [kernel.kallsyms]  [k] __vunmap
>    3.98%  cat      [kernel.kallsyms]  [k] smp_call_function_single
>    3.81%  cat      [kernel.kallsyms]  [k] __purge_vmap_area_lazy
>    2.77%  cat      [kernel.kallsyms]  [k] __free_vmap_area
> 
> Signed-off-by: Roman Gushchin <guro@fb.com>
> Cc: Johannes Weiner <hannes@cmpxchg.org>
> Cc: Matthew Wilcox <willy@infradead.org>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> ---
>  mm/vmalloc.c | 52 +++++++++++++++++++++++++++++-----------------------
>  1 file changed, 29 insertions(+), 23 deletions(-)
> 
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index c42872ed82ac..8d4907865614 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -2075,6 +2075,22 @@ 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;
> +
> +	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
> @@ -2087,26 +2103,14 @@ 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();

Is not this necessary any more ?

> -
>  	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;
>  }

Other callers of remove_vm_area() cannot use __remove_vm_area() directly as well
to save a look up ?

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

* Re: [PATCH] mm: refactor __vunmap() to avoid duplicated call to find_vm_area()
  2019-05-15  7:41 ` Anshuman Khandual
@ 2019-05-15 17:17   ` Roman Gushchin
  0 siblings, 0 replies; 11+ messages in thread
From: Roman Gushchin @ 2019-05-15 17:17 UTC (permalink / raw)
  To: Anshuman Khandual
  Cc: Andrew Morton, linux-mm, linux-kernel, Kernel Team,
	Johannes Weiner, Matthew Wilcox, Vlastimil Babka

On Wed, May 15, 2019 at 01:11:46PM +0530, Anshuman Khandual wrote:
> 
> 
> On 05/15/2019 05:21 AM, Roman Gushchin wrote:
> > __vunmap() calls find_vm_area() twice without an obvious reason:
> > first directly to get the area pointer, second indirectly by calling
> > vm_remove_mappings()->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. Let's pass
> > a pointer to the vm_area instead of vm_struct to vm_remove_mappings(),
> > so it can pass it to __remove_vm_area() and avoid the redundant area
> > lookup.
> > 
> > On my test setup, I've got 5-10% speed up on vfree()'ing 1000000
> > of 4-pages vmalloc blocks.
> > 
> > Perf report before:
> >   29.44%  cat      [kernel.kallsyms]  [k] free_unref_page
> >   11.88%  cat      [kernel.kallsyms]  [k] find_vmap_area
> >    9.28%  cat      [kernel.kallsyms]  [k] __free_pages
> >    7.44%  cat      [kernel.kallsyms]  [k] __slab_free
> >    7.28%  cat      [kernel.kallsyms]  [k] vunmap_page_range
> >    4.56%  cat      [kernel.kallsyms]  [k] __vunmap
> >    3.64%  cat      [kernel.kallsyms]  [k] __purge_vmap_area_lazy
> >    3.04%  cat      [kernel.kallsyms]  [k] __free_vmap_area
> > 
> > Perf report after:
> >   32.41%  cat      [kernel.kallsyms]  [k] free_unref_page
> >    7.79%  cat      [kernel.kallsyms]  [k] find_vmap_area
> >    7.40%  cat      [kernel.kallsyms]  [k] __slab_free
> >    7.31%  cat      [kernel.kallsyms]  [k] vunmap_page_range
> >    6.84%  cat      [kernel.kallsyms]  [k] __free_pages
> >    6.01%  cat      [kernel.kallsyms]  [k] __vunmap
> >    3.98%  cat      [kernel.kallsyms]  [k] smp_call_function_single
> >    3.81%  cat      [kernel.kallsyms]  [k] __purge_vmap_area_lazy
> >    2.77%  cat      [kernel.kallsyms]  [k] __free_vmap_area
> > 
> > Signed-off-by: Roman Gushchin <guro@fb.com>
> > Cc: Johannes Weiner <hannes@cmpxchg.org>
> > Cc: Matthew Wilcox <willy@infradead.org>
> > Cc: Vlastimil Babka <vbabka@suse.cz>
> > ---
> >  mm/vmalloc.c | 52 +++++++++++++++++++++++++++++-----------------------
> >  1 file changed, 29 insertions(+), 23 deletions(-)
> > 
> > diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> > index c42872ed82ac..8d4907865614 100644
> > --- a/mm/vmalloc.c
> > +++ b/mm/vmalloc.c
> > @@ -2075,6 +2075,22 @@ 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;
> > +
> > +	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
> > @@ -2087,26 +2103,14 @@ 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();
> 
> Is not this necessary any more ?

We've discussed it here: https://lkml.org/lkml/2019/4/17/1098 .
Tl;dr it's not that useful.

> 
> > -
> >  	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;
> >  }
> 
> Other callers of remove_vm_area() cannot use __remove_vm_area() directly as well
> to save a look up ?
> 

I'll take a look. Good idea, thanks!

Roman

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

* Re: [PATCH] mm: refactor __vunmap() to avoid duplicated call to find_vm_area()
  2019-05-15  4:27 ` [PATCH] mm: refactor __vunmap() to avoid duplicated call to find_vm_area() Anshuman Khandual
@ 2019-05-15 17:18   ` Roman Gushchin
  0 siblings, 0 replies; 11+ messages in thread
From: Roman Gushchin @ 2019-05-15 17:18 UTC (permalink / raw)
  To: Anshuman Khandual
  Cc: Andrew Morton, linux-mm, linux-kernel, Kernel Team,
	Johannes Weiner, Matthew Wilcox, Vlastimil Babka

On Wed, May 15, 2019 at 09:57:11AM +0530, Anshuman Khandual wrote:
> 
> 
> On 05/15/2019 05:21 AM, Roman Gushchin wrote:
> > __vunmap() calls find_vm_area() twice without an obvious reason:
> > first directly to get the area pointer, second indirectly by calling
> > vm_remove_mappings()->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. Let's pass
> > a pointer to the vm_area instead of vm_struct to vm_remove_mappings(),
> > so it can pass it to __remove_vm_area() and avoid the redundant area
> > lookup.
> > 
> > On my test setup, I've got 5-10% speed up on vfree()'ing 1000000
> > of 4-pages vmalloc blocks.
> 
> Though results from  1000000 single page vmalloc blocks remain inconclusive,
> 4-page based vmalloc block's result shows improvement in the range of 5-10%.

So you can confirm my numbers? Great, thank you!

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

* Re: [PATCH] mm: refactor __vunmap() to avoid duplicated call to find_vm_area()
  2019-05-14 23:51 [PATCH] mm: refactor __vunmap() to avoid duplicated call to find_vm_area() Roman Gushchin
                   ` (2 preceding siblings ...)
  2019-05-15  7:41 ` Anshuman Khandual
@ 2019-05-15 17:35 ` Ira Weiny
  2019-05-15 17:45   ` Roman Gushchin
  3 siblings, 1 reply; 11+ messages in thread
From: Ira Weiny @ 2019-05-15 17:35 UTC (permalink / raw)
  To: Roman Gushchin
  Cc: Andrew Morton, linux-mm, linux-kernel, kernel-team,
	Johannes Weiner, Matthew Wilcox, Vlastimil Babka

>  
> -/* Handle removing and resetting vm mappings related to the vm_struct. */
> -static void vm_remove_mappings(struct vm_struct *area, int deallocate_pages)
> +/* Handle removing and resetting vm mappings related to the va->vm vm_struct. */
> +static void vm_remove_mappings(struct vmap_area *va, int deallocate_pages)

Does this apply to 5.1?  I'm confused because I can't find vm_remove_mappings()
in 5.1.

Ira

>  {
> +	struct vm_struct *area = va->vm;
>  	unsigned long addr = (unsigned long)area->addr;
>  	unsigned long start = ULONG_MAX, end = 0;
>  	int flush_reset = area->flags & VM_FLUSH_RESET_PERMS;
> @@ -2138,7 +2143,7 @@ static void vm_remove_mappings(struct vm_struct *area, int deallocate_pages)
>  		set_memory_rw(addr, area->nr_pages);
>  	}
>  
> -	remove_vm_area(area->addr);
> +	__remove_vm_area(va);
>  
>  	/* If this is not VM_FLUSH_RESET_PERMS memory, no need for the below. */
>  	if (!flush_reset)
> @@ -2178,6 +2183,7 @@ static void vm_remove_mappings(struct vm_struct *area, int deallocate_pages)
>  static void __vunmap(const void *addr, int deallocate_pages)
>  {
>  	struct vm_struct *area;
> +	struct vmap_area *va;
>  
>  	if (!addr)
>  		return;
> @@ -2186,17 +2192,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));
>  
> -	vm_remove_mappings(area, deallocate_pages);
> +	vm_remove_mappings(va, deallocate_pages);
>  
>  	if (deallocate_pages) {
>  		int i;
> @@ -2212,7 +2219,6 @@ static void __vunmap(const void *addr, int deallocate_pages)
>  	}
>  
>  	kfree(area);
> -	return;
>  }
>  
>  static inline void __vfree_deferred(const void *addr)
> -- 
> 2.20.1
> 

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

* Re: [PATCH] mm: refactor __vunmap() to avoid duplicated call to find_vm_area()
  2019-05-15 17:35 ` Ira Weiny
@ 2019-05-15 17:45   ` Roman Gushchin
  0 siblings, 0 replies; 11+ messages in thread
From: Roman Gushchin @ 2019-05-15 17:45 UTC (permalink / raw)
  To: Ira Weiny
  Cc: Andrew Morton, linux-mm, linux-kernel, Kernel Team,
	Johannes Weiner, Matthew Wilcox, Vlastimil Babka

On Wed, May 15, 2019 at 10:35:26AM -0700, Ira Weiny wrote:
> >  
> > -/* Handle removing and resetting vm mappings related to the vm_struct. */
> > -static void vm_remove_mappings(struct vm_struct *area, int deallocate_pages)
> > +/* Handle removing and resetting vm mappings related to the va->vm vm_struct. */
> > +static void vm_remove_mappings(struct vmap_area *va, int deallocate_pages)
> 
> Does this apply to 5.1?  I'm confused because I can't find vm_remove_mappings()
> in 5.1.

Not really, it's based on top of the current mm tree.
You can find the earlier version which applies on 5.1 here:
https://lkml.org/lkml/2019/4/17/954

Thanks!

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

* Re: [PATCH RESEND] mm: show number of vmalloc pages in /proc/meminfo
  2019-05-14 23:51 ` [PATCH RESEND] mm: show number of vmalloc pages in /proc/meminfo Roman Gushchin
  2019-05-15  7:14   ` Anshuman Khandual
@ 2019-07-09  5:59   ` Minchan Kim
  2019-07-10  1:31     ` Roman Gushchin
  1 sibling, 1 reply; 11+ messages in thread
From: Minchan Kim @ 2019-07-09  5:59 UTC (permalink / raw)
  To: Roman Gushchin
  Cc: Andrew Morton, linux-mm, linux-kernel, kernel-team, Johannes Weiner

Hi Roman,


On Wed, May 15, 2019 at 8:51 AM Roman Gushchin <guro@fb.com> 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().
>
> Signed-off-by: Roman Gushchin <guro@fb.com>
> Acked-by: Johannes Weiner <hannes@cmpxchg.org>
> Acked-by: Vlastimil Babka <vbabka@suse.cz>
Acked-by: Minchan Kim <minchan@kernel.org>

How it's going on?
Android needs this patch since it has gathered vmalloc pages from
/proc/vmallocinfo. It's too slow.

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

* Re: [PATCH RESEND] mm: show number of vmalloc pages in /proc/meminfo
  2019-07-09  5:59   ` Minchan Kim
@ 2019-07-10  1:31     ` Roman Gushchin
  0 siblings, 0 replies; 11+ messages in thread
From: Roman Gushchin @ 2019-07-10  1:31 UTC (permalink / raw)
  To: Andrew Morton, Minchan Kim
  Cc: Andrew Morton, linux-mm, linux-kernel, Kernel Team, Johannes Weiner

On Tue, Jul 09, 2019 at 02:59:42PM +0900, Minchan Kim wrote:
> Hi Roman,
> 
> 
> On Wed, May 15, 2019 at 8:51 AM Roman Gushchin <guro@fb.com> 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().
> >
> > Signed-off-by: Roman Gushchin <guro@fb.com>
> > Acked-by: Johannes Weiner <hannes@cmpxchg.org>
> > Acked-by: Vlastimil Babka <vbabka@suse.cz>
> Acked-by: Minchan Kim <minchan@kernel.org>
> 
> How it's going on?
> Android needs this patch since it has gathered vmalloc pages from
> /proc/vmallocinfo. It's too slow.
> 

Andrew, can you, please, pick this one?

It has been in the mm tree already, but then it was dropped
because of some other non-related patches in the series
conflicted with some x86 changes. This patch is useful
by itself, and doesn't depend on anything else.

Thanks!

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

end of thread, other threads:[~2019-07-10  1:31 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-14 23:51 [PATCH] mm: refactor __vunmap() to avoid duplicated call to find_vm_area() Roman Gushchin
2019-05-14 23:51 ` [PATCH RESEND] mm: show number of vmalloc pages in /proc/meminfo Roman Gushchin
2019-05-15  7:14   ` Anshuman Khandual
2019-07-09  5:59   ` Minchan Kim
2019-07-10  1:31     ` Roman Gushchin
2019-05-15  4:27 ` [PATCH] mm: refactor __vunmap() to avoid duplicated call to find_vm_area() Anshuman Khandual
2019-05-15 17:18   ` Roman Gushchin
2019-05-15  7:41 ` Anshuman Khandual
2019-05-15 17:17   ` Roman Gushchin
2019-05-15 17:35 ` Ira Weiny
2019-05-15 17:45   ` Roman Gushchin

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