2017-04-18 05:49:56

by Hoeun Ryu

[permalink] [raw]
Subject: [PATCH v2] mm: add VM_STATIC flag to vmalloc and prevent from removing the areas

vm_area_add_early/vm_area_register_early() are used to reserve vmalloc area
during boot process and those virtually mapped areas are never unmapped.
So `OR` VM_STATIC flag to the areas in vmalloc_init() when importing
existing vmlist entries and prevent those areas from being removed from the
rbtree by accident. This flags can be also used by other vmalloc APIs to
specify that the area will never go away.
This makes remove_vm_area() more robust against other kind of errors (eg.
programming errors).

Signed-off-by: Hoeun Ryu <[email protected]>
---
v2:
- update changelog
- add description to VM_STATIC
- check VM_STATIC first before VM_VM_AREA in remove_vm_area()

include/linux/vmalloc.h | 7 +++++++
mm/vmalloc.c | 9 ++++++---
2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
index 46991ad..a42c210 100644
--- a/include/linux/vmalloc.h
+++ b/include/linux/vmalloc.h
@@ -19,6 +19,13 @@ 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 */
+/*
+ * static area, vmap_area will never go away.
+ * This flag is OR-ed automatically in vmalloc_init() if the area is inserted
+ * by vm_area_add_early()/vm_area_register_early() during early boot process
+ * or you can give this flag manually using other vmalloc APIs.
+ */
+#define VM_STATIC 0x00000200
/* bits [20..32] reserved for arch specific ioremap internals */

/*
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 8ef8ea1..6bc6c39 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -1262,7 +1262,7 @@ void __init vmalloc_init(void)
/* Import existing vmlist entries. */
for (tmp = vmlist; tmp; tmp = tmp->next) {
va = kzalloc(sizeof(struct vmap_area), GFP_NOWAIT);
- va->flags = VM_VM_AREA;
+ va->flags = VM_VM_AREA | VM_STATIC;
va->va_start = (unsigned long)tmp->addr;
va->va_end = va->va_start + tmp->size;
va->vm = tmp;
@@ -1480,7 +1480,7 @@ struct vm_struct *remove_vm_area(const void *addr)
might_sleep();

va = find_vmap_area((unsigned long)addr);
- if (va && va->flags & VM_VM_AREA) {
+ if (va && likely(!(va->flags & VM_STATIC)) && va->flags & VM_VM_AREA) {
struct vm_struct *vm = va->vm;

spin_lock(&vmap_area_lock);
@@ -1510,7 +1510,7 @@ static void __vunmap(const void *addr, int deallocate_pages)

area = remove_vm_area(addr);
if (unlikely(!area)) {
- WARN(1, KERN_ERR "Trying to vfree() nonexistent vm area (%p)\n",
+ WARN(1, KERN_ERR "Trying to vfree() nonexistent or static vm area (%p)\n",
addr);
return;
}
@@ -2708,6 +2708,9 @@ static int s_show(struct seq_file *m, void *p)
if (v->phys_addr)
seq_printf(m, " phys=%pa", &v->phys_addr);

+ if (v->flags & VM_STATIC)
+ seq_puts(m, " static");
+
if (v->flags & VM_IOREMAP)
seq_puts(m, " ioremap");

--
2.7.4


2017-04-18 06:59:55

by Michal Hocko

[permalink] [raw]
Subject: Re: [PATCH v2] mm: add VM_STATIC flag to vmalloc and prevent from removing the areas

On Tue 18-04-17 14:48:39, Hoeun Ryu wrote:
> vm_area_add_early/vm_area_register_early() are used to reserve vmalloc area
> during boot process and those virtually mapped areas are never unmapped.
> So `OR` VM_STATIC flag to the areas in vmalloc_init() when importing
> existing vmlist entries and prevent those areas from being removed from the
> rbtree by accident.

Has this been a problem in the past or currently so that it is worth
handling?

> This flags can be also used by other vmalloc APIs to
> specify that the area will never go away.

Do we have a user for that?

> This makes remove_vm_area() more robust against other kind of errors (eg.
> programming errors).

Well, yes it will help to prevent from vfree(early_mem) but we have 4
users of vm_area_register_early so I am really wondering whether this is
worth additional code. It would really help to understand your
motivation for the patch if we were explicit about the problem you are
trying to solve.

Thanks

--
Michal Hocko
SUSE Labs

2017-04-19 05:32:28

by Hoeun Ryu

[permalink] [raw]
Subject: Re: [PATCH v2] mm: add VM_STATIC flag to vmalloc and prevent from removing the areas


> On Apr 18, 2017, at 3:59 PM, Michal Hocko <[email protected]> wrote:
>
>> On Tue 18-04-17 14:48:39, Hoeun Ryu wrote:
>> vm_area_add_early/vm_area_register_early() are used to reserve vmalloc area
>> during boot process and those virtually mapped areas are never unmapped.
>> So `OR` VM_STATIC flag to the areas in vmalloc_init() when importing
>> existing vmlist entries and prevent those areas from being removed from the
>> rbtree by accident.
>
> Has this been a problem in the past or currently so that it is worth
> handling?
>
>> This flags can be also used by other vmalloc APIs to
>> specify that the area will never go away.
>
> Do we have a user for that?
>
>> This makes remove_vm_area() more robust against other kind of errors (eg.
>> programming errors).
>
> Well, yes it will help to prevent from vfree(early_mem) but we have 4
> users of vm_area_register_early so I am really wondering whether this is
> worth additional code. It would really help to understand your
> motivation for the patch if we were explicit about the problem you are
> trying to solve.

I just think that it would be good to make it robust against various kind of errors.
You might think that's not an enough reason to do so though.

>
> Thanks
>
> --
> Michal Hocko
> SUSE Labs