2019-05-14 23:53:02

by Roman Gushchin

[permalink] [raw]
Subject: [PATCH] mm: refactor __vunmap() to avoid duplicated call to find_vm_area()

__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 <[email protected]>
Cc: Johannes Weiner <[email protected]>
Cc: Matthew Wilcox <[email protected]>
Cc: Vlastimil Babka <[email protected]>
---
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


2019-05-14 23:54:34

by Roman Gushchin

[permalink] [raw]
Subject: [PATCH RESEND] mm: show number of vmalloc pages in /proc/meminfo

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 <[email protected]>
Acked-by: Johannes Weiner <[email protected]>
Acked-by: Vlastimil Babka <[email protected]>
---
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

2019-05-15 04:29:34

by Anshuman Khandual

[permalink] [raw]
Subject: Re: [PATCH] mm: refactor __vunmap() to avoid duplicated call to find_vm_area()



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%.

2019-05-15 07:15:31

by Anshuman Khandual

[permalink] [raw]
Subject: Re: [PATCH RESEND] mm: show number of vmalloc pages in /proc/meminfo



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.

2019-05-15 07:43:03

by Anshuman Khandual

[permalink] [raw]
Subject: Re: [PATCH] mm: refactor __vunmap() to avoid duplicated call to find_vm_area()



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 <[email protected]>
> Cc: Johannes Weiner <[email protected]>
> Cc: Matthew Wilcox <[email protected]>
> Cc: Vlastimil Babka <[email protected]>
> ---
> 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 ?

2019-05-15 17:19:21

by Roman Gushchin

[permalink] [raw]
Subject: Re: [PATCH] mm: refactor __vunmap() to avoid duplicated call to find_vm_area()

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 <[email protected]>
> > Cc: Johannes Weiner <[email protected]>
> > Cc: Matthew Wilcox <[email protected]>
> > Cc: Vlastimil Babka <[email protected]>
> > ---
> > 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

2019-05-15 17:20:31

by Roman Gushchin

[permalink] [raw]
Subject: Re: [PATCH] mm: refactor __vunmap() to avoid duplicated call to find_vm_area()

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!

2019-05-15 17:36:06

by Ira Weiny

[permalink] [raw]
Subject: Re: [PATCH] mm: refactor __vunmap() to avoid duplicated call to find_vm_area()

>
> -/* 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
>

2019-05-15 17:47:40

by Roman Gushchin

[permalink] [raw]
Subject: Re: [PATCH] mm: refactor __vunmap() to avoid duplicated call to find_vm_area()

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!

2019-07-09 06:02:26

by Minchan Kim

[permalink] [raw]
Subject: Re: [PATCH RESEND] mm: show number of vmalloc pages in /proc/meminfo

Hi Roman,


On Wed, May 15, 2019 at 8:51 AM Roman Gushchin <[email protected]> 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 <[email protected]>
> Acked-by: Johannes Weiner <[email protected]>
> Acked-by: Vlastimil Babka <[email protected]>
Acked-by: Minchan Kim <[email protected]>

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

2019-07-10 01:31:59

by Roman Gushchin

[permalink] [raw]
Subject: Re: [PATCH RESEND] mm: show number of vmalloc pages in /proc/meminfo

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 <[email protected]> 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 <[email protected]>
> > Acked-by: Johannes Weiner <[email protected]>
> > Acked-by: Vlastimil Babka <[email protected]>
> Acked-by: Minchan Kim <[email protected]>
>
> 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!