Re: [PATCHv3 1/3] mm: Introduce vma_init()

From: Andrew Morton
Date: Tue Jul 24 2018 - 16:03:12 EST


On Tue, 24 Jul 2018 15:11:37 +0300 "Kirill A. Shutemov" <kirill.shutemov@xxxxxxxxxxxxxxx> wrote:

> Not all VMAs allocated with vm_area_alloc(). Some of them allocated on
> stack or in data segment.
>
> The new helper can be use to initialize VMA properly regardless where
> it was allocated.
>
> ...
>
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -452,6 +452,12 @@ struct vm_operations_struct {
> unsigned long addr);
> };
>
> +static inline void vma_init(struct vm_area_struct *vma, struct mm_struct *mm)
> +{
> + vma->vm_mm = mm;
> + INIT_LIST_HEAD(&vma->anon_vma_chain);
> +}
> +
> struct mmu_gather;
> struct inode;
>
> diff --git a/kernel/fork.c b/kernel/fork.c
> index a191c05e757d..1b27babc4c78 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -312,10 +312,8 @@ struct vm_area_struct *vm_area_alloc(struct mm_struct *mm)
> {
> struct vm_area_struct *vma = kmem_cache_zalloc(vm_area_cachep, GFP_KERNEL);

I'd sleep better if this became a kmem_cache_alloc() and the memset
was moved into vma_init(). A bunch of the vma_init() callers are
already doing the memset and the others risk leaving uninitialized
stack fields in the vma.

>
> - if (vma) {
> - vma->vm_mm = mm;
> - INIT_LIST_HEAD(&vma->anon_vma_chain);
> - }
> + if (vma)
> + vma_init(vma, mm);
> return vma;
> }