Re: [PATCH] mm: add VM_STATIC flag to vmalloc and prevent from removing the areas
> On Apr 13, 2017, at 1:17 PM, Anshuman Khandual> wrote: > >> On 04/12/2017 10:31 AM, 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. > > I am wondering whether protection against accidental deletion > of any vmap area should be done in remove_vm_area() function > or the callers should take care of it. But I guess either way > it works. > >> >> Signed-off-by: Hoeun Ryu >> --- >> include/linux/vmalloc.h | 1 + >> mm/vmalloc.c| 9 ++--- >> 2 files changed, 7 insertions(+), 3 deletions(-) >> >> diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h >> index 46991ad..3df53fc 100644 >> --- a/include/linux/vmalloc.h >> +++ b/include/linux/vmalloc.h >> @@ -19,6 +19,7 @@ struct notifier_block;/* in notifier.h */ >> #define VM_UNINITIALIZED0x0020/* vm_struct is not fully >> initialized */ >> #define VM_NO_GUARD0x0040 /* don't add guard page */ >> #define VM_KASAN0x0080 /* has allocated kasan shadow memory >> */ >> +#define VM_STATIC0x0200 > > You might want to add some description in the comment saying > its a sticky VM area which will never go away or something. > OK. I will add some description. >> /* bits [20..32] reserved for arch specific ioremap internals */ >> >> /* >> diff --git a/mm/vmalloc.c b/mm/vmalloc.c >> index 8ef8ea1..fb5049a 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 && va->flags & VM_VM_AREA && likely(!(va->flags & VM_STATIC))) { > > > You might want to move the VM_STATIC check before the VM_VM_AREA > check so in cases where the former is set we can save one more > conditional check. > OK, I'll fix this in the next version Thank you for the review.
Re: [PATCH] mm: add VM_STATIC flag to vmalloc and prevent from removing the areas
> On Apr 13, 2017, at 1:17 PM, Anshuman Khandual > wrote: > >> On 04/12/2017 10:31 AM, 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. > > I am wondering whether protection against accidental deletion > of any vmap area should be done in remove_vm_area() function > or the callers should take care of it. But I guess either way > it works. > >> >> Signed-off-by: Hoeun Ryu >> --- >> include/linux/vmalloc.h | 1 + >> mm/vmalloc.c| 9 ++--- >> 2 files changed, 7 insertions(+), 3 deletions(-) >> >> diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h >> index 46991ad..3df53fc 100644 >> --- a/include/linux/vmalloc.h >> +++ b/include/linux/vmalloc.h >> @@ -19,6 +19,7 @@ struct notifier_block;/* in notifier.h */ >> #define VM_UNINITIALIZED0x0020/* vm_struct is not fully >> initialized */ >> #define VM_NO_GUARD0x0040 /* don't add guard page */ >> #define VM_KASAN0x0080 /* has allocated kasan shadow memory >> */ >> +#define VM_STATIC0x0200 > > You might want to add some description in the comment saying > its a sticky VM area which will never go away or something. > OK. I will add some description. >> /* bits [20..32] reserved for arch specific ioremap internals */ >> >> /* >> diff --git a/mm/vmalloc.c b/mm/vmalloc.c >> index 8ef8ea1..fb5049a 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 && va->flags & VM_VM_AREA && likely(!(va->flags & VM_STATIC))) { > > > You might want to move the VM_STATIC check before the VM_VM_AREA > check so in cases where the former is set we can save one more > conditional check. > OK, I'll fix this in the next version Thank you for the review.
Re: [PATCH] mm: add VM_STATIC flag to vmalloc and prevent from removing the areas
On 04/12/2017 10:31 AM, 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. I am wondering whether protection against accidental deletion of any vmap area should be done in remove_vm_area() function or the callers should take care of it. But I guess either way it works. > > Signed-off-by: Hoeun Ryu> --- > include/linux/vmalloc.h | 1 + > mm/vmalloc.c| 9 ++--- > 2 files changed, 7 insertions(+), 3 deletions(-) > > diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h > index 46991ad..3df53fc 100644 > --- a/include/linux/vmalloc.h > +++ b/include/linux/vmalloc.h > @@ -19,6 +19,7 @@ struct notifier_block; /* in notifier.h */ > #define VM_UNINITIALIZED 0x0020 /* vm_struct is not fully > initialized */ > #define VM_NO_GUARD 0x0040 /* don't add guard page */ > #define VM_KASAN 0x0080 /* has allocated kasan shadow > memory */ > +#define VM_STATIC0x0200 You might want to add some description in the comment saying its a sticky VM area which will never go away or something. > /* bits [20..32] reserved for arch specific ioremap internals */ > > /* > diff --git a/mm/vmalloc.c b/mm/vmalloc.c > index 8ef8ea1..fb5049a 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 && va->flags & VM_VM_AREA && likely(!(va->flags & VM_STATIC))) { You might want to move the VM_STATIC check before the VM_VM_AREA check so in cases where the former is set we can save one more conditional check.
Re: [PATCH] mm: add VM_STATIC flag to vmalloc and prevent from removing the areas
On 04/12/2017 10:31 AM, 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. I am wondering whether protection against accidental deletion of any vmap area should be done in remove_vm_area() function or the callers should take care of it. But I guess either way it works. > > Signed-off-by: Hoeun Ryu > --- > include/linux/vmalloc.h | 1 + > mm/vmalloc.c| 9 ++--- > 2 files changed, 7 insertions(+), 3 deletions(-) > > diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h > index 46991ad..3df53fc 100644 > --- a/include/linux/vmalloc.h > +++ b/include/linux/vmalloc.h > @@ -19,6 +19,7 @@ struct notifier_block; /* in notifier.h */ > #define VM_UNINITIALIZED 0x0020 /* vm_struct is not fully > initialized */ > #define VM_NO_GUARD 0x0040 /* don't add guard page */ > #define VM_KASAN 0x0080 /* has allocated kasan shadow > memory */ > +#define VM_STATIC0x0200 You might want to add some description in the comment saying its a sticky VM area which will never go away or something. > /* bits [20..32] reserved for arch specific ioremap internals */ > > /* > diff --git a/mm/vmalloc.c b/mm/vmalloc.c > index 8ef8ea1..fb5049a 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 && va->flags & VM_VM_AREA && likely(!(va->flags & VM_STATIC))) { You might want to move the VM_STATIC check before the VM_VM_AREA check so in cases where the former is set we can save one more conditional check.
Re: [PATCH] mm: add VM_STATIC flag to vmalloc and prevent from removing the areas
> On 13 Apr 2017, at 2:31 AM, Christoph Hellwigwrote: > > On Wed, Apr 12, 2017 at 08:42:08PM +0900, Hoeun Ryu wrote: >> >>> On Apr 12, 2017, at 3:02 PM, Christoph Hellwig wrote: >>> On Wed, Apr 12, 2017 at 02:01:59PM +0900, 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. >>> >>> How would they be removed "by accident"? >> >> I don't mean actual use-cases, but I just want to make it robust against >> like programming errors. > > Oh, ok. The patch makes sense then, although the changelog could use > a little update. OK, I will. Any other suggestions for code itself ?
Re: [PATCH] mm: add VM_STATIC flag to vmalloc and prevent from removing the areas
> On 13 Apr 2017, at 2:31 AM, Christoph Hellwig wrote: > > On Wed, Apr 12, 2017 at 08:42:08PM +0900, Hoeun Ryu wrote: >> >>> On Apr 12, 2017, at 3:02 PM, Christoph Hellwig wrote: >>> On Wed, Apr 12, 2017 at 02:01:59PM +0900, 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. >>> >>> How would they be removed "by accident"? >> >> I don't mean actual use-cases, but I just want to make it robust against >> like programming errors. > > Oh, ok. The patch makes sense then, although the changelog could use > a little update. OK, I will. Any other suggestions for code itself ?
Re: [PATCH] mm: add VM_STATIC flag to vmalloc and prevent from removing the areas
On Wed, Apr 12, 2017 at 08:42:08PM +0900, Hoeun Ryu wrote: > > > On Apr 12, 2017, at 3:02 PM, Christoph Hellwigwrote: > > > >> On Wed, Apr 12, 2017 at 02:01:59PM +0900, 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. > > > > How would they be removed "by accident"? > > I don't mean actual use-cases, but I just want to make it robust against like > programming errors. Oh, ok. The patch makes sense then, although the changelog could use a little update.
Re: [PATCH] mm: add VM_STATIC flag to vmalloc and prevent from removing the areas
On Wed, Apr 12, 2017 at 08:42:08PM +0900, Hoeun Ryu wrote: > > > On Apr 12, 2017, at 3:02 PM, Christoph Hellwig wrote: > > > >> On Wed, Apr 12, 2017 at 02:01:59PM +0900, 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. > > > > How would they be removed "by accident"? > > I don't mean actual use-cases, but I just want to make it robust against like > programming errors. Oh, ok. The patch makes sense then, although the changelog could use a little update.
Re: [PATCH] mm: add VM_STATIC flag to vmalloc and prevent from removing the areas
> On Apr 12, 2017, at 3:02 PM, Christoph Hellwigwrote: > >> On Wed, Apr 12, 2017 at 02:01:59PM +0900, 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. > > How would they be removed "by accident"? I don't mean actual use-cases, but I just want to make it robust against like programming errors.
Re: [PATCH] mm: add VM_STATIC flag to vmalloc and prevent from removing the areas
> On Apr 12, 2017, at 3:02 PM, Christoph Hellwig wrote: > >> On Wed, Apr 12, 2017 at 02:01:59PM +0900, 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. > > How would they be removed "by accident"? I don't mean actual use-cases, but I just want to make it robust against like programming errors.
Re: [PATCH] mm: add VM_STATIC flag to vmalloc and prevent from removing the areas
On Wed, Apr 12, 2017 at 02:01:59PM +0900, 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. How would they be removed "by accident"?
Re: [PATCH] mm: add VM_STATIC flag to vmalloc and prevent from removing the areas
On Wed, Apr 12, 2017 at 02:01:59PM +0900, 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. How would they be removed "by accident"?
[PATCH] 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. Signed-off-by: Hoeun Ryu--- include/linux/vmalloc.h | 1 + mm/vmalloc.c| 9 ++--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h index 46991ad..3df53fc 100644 --- a/include/linux/vmalloc.h +++ b/include/linux/vmalloc.h @@ -19,6 +19,7 @@ struct notifier_block;/* in notifier.h */ #define VM_UNINITIALIZED 0x0020 /* vm_struct is not fully initialized */ #define VM_NO_GUARD0x0040 /* don't add guard page */ #define VM_KASAN 0x0080 /* has allocated kasan shadow memory */ +#define VM_STATIC 0x0200 /* bits [20..32] reserved for arch specific ioremap internals */ /* diff --git a/mm/vmalloc.c b/mm/vmalloc.c index 8ef8ea1..fb5049a 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 && va->flags & VM_VM_AREA && likely(!(va->flags & VM_STATIC))) { struct vm_struct *vm = va->vm; spin_lock(_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", >phys_addr); + if (v->flags & VM_STATIC) + seq_puts(m, " static"); + if (v->flags & VM_IOREMAP) seq_puts(m, " ioremap"); -- 2.7.4
[PATCH] 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. Signed-off-by: Hoeun Ryu --- include/linux/vmalloc.h | 1 + mm/vmalloc.c| 9 ++--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h index 46991ad..3df53fc 100644 --- a/include/linux/vmalloc.h +++ b/include/linux/vmalloc.h @@ -19,6 +19,7 @@ struct notifier_block;/* in notifier.h */ #define VM_UNINITIALIZED 0x0020 /* vm_struct is not fully initialized */ #define VM_NO_GUARD0x0040 /* don't add guard page */ #define VM_KASAN 0x0080 /* has allocated kasan shadow memory */ +#define VM_STATIC 0x0200 /* bits [20..32] reserved for arch specific ioremap internals */ /* diff --git a/mm/vmalloc.c b/mm/vmalloc.c index 8ef8ea1..fb5049a 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 && va->flags & VM_VM_AREA && likely(!(va->flags & VM_STATIC))) { struct vm_struct *vm = va->vm; spin_lock(_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", >phys_addr); + if (v->flags & VM_STATIC) + seq_puts(m, " static"); + if (v->flags & VM_IOREMAP) seq_puts(m, " ioremap"); -- 2.7.4