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

[Andrew Morton]

On Tue, 24 Jul 2018 13:16:33 -0700 Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:

> On Tue, Jul 24, 2018 at 1:03 PM Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
> >
> >
> > I'd sleep better if this became a kmem_cache_alloc() and the memset
> > was moved into vma_init().
> 
> Yeah, with the vma_init(), I guess the advantage of using
> kmem_cache_zalloc() is pretty dubious.
> 
> Make it so.
> 

Did I get everything?


From: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
Subject: mm: zero out the vma in vma_init()

Rather than in vm_area_alloc().  To ensure that the various oddball
stack-based vmas are in a good state.  Some of the callers were zeroing
them out, others were not.

Cc: Russell King <rmk+kernel@xxxxxxxxxxxxxxxx>
Cc: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
Cc: "Kirill A. Shutemov" <kirill.shutemov@xxxxxxxxxxxxxxx>
Cc: Dmitry Vyukov <dvyukov@xxxxxxxxxx>
Cc: Oleg Nesterov <oleg@xxxxxxxxxx>
Cc: Andrea Arcangeli <aarcange@xxxxxxxxxx>
Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
---

 arch/arm/kernel/process.c |    9 ++++-----
 fs/hugetlbfs/inode.c      |    2 --
 include/linux/mm.h        |    1 +
 kernel/fork.c             |    3 ++-
 mm/mempolicy.c            |    1 -
 mm/shmem.c                |    1 -
 6 files changed, 7 insertions(+), 10 deletions(-)

diff -puN arch/arm/kernel/process.c~mm-zero-out-the-vma-in-vma_init arch/arm/kernel/process.c
--- a/arch/arm/kernel/process.c~mm-zero-out-the-vma-in-vma_init
+++ a/arch/arm/kernel/process.c
@@ -330,16 +330,15 @@ unsigned long arch_randomize_brk(struct
  * atomic helpers. Insert it into the gate_vma so that it is visible
  * through ptrace and /proc/<pid>/mem.
  */
-static struct vm_area_struct gate_vma = {
-	.vm_start	= 0xffff0000,
-	.vm_end		= 0xffff0000 + PAGE_SIZE,
-	.vm_flags	= VM_READ | VM_EXEC | VM_MAYREAD | VM_MAYEXEC,
-};
+static struct vm_area_struct gate_vma;
 
 static int __init gate_vma_init(void)
 {
 	vma_init(&gate_vma, NULL);
 	gate_vma.vm_page_prot = PAGE_READONLY_EXEC;
+	gate_vma.vm_start = 0xffff0000;
+	gate_vma.vm_end	= 0xffff0000 + PAGE_SIZE;
+	gate_vma.vm_flags = VM_READ | VM_EXEC | VM_MAYREAD | VM_MAYEXEC;
 	return 0;
 }
 arch_initcall(gate_vma_init);
diff -puN fs/hugetlbfs/inode.c~mm-zero-out-the-vma-in-vma_init fs/hugetlbfs/inode.c
--- a/fs/hugetlbfs/inode.c~mm-zero-out-the-vma-in-vma_init
+++ a/fs/hugetlbfs/inode.c
@@ -410,7 +410,6 @@ static void remove_inode_hugepages(struc
 	int i, freed = 0;
 	bool truncate_op = (lend == LLONG_MAX);
 
-	memset(&pseudo_vma, 0, sizeof(struct vm_area_struct));
 	vma_init(&pseudo_vma, current->mm);
 	pseudo_vma.vm_flags = (VM_HUGETLB | VM_MAYSHARE | VM_SHARED);
 	pagevec_init(&pvec);
@@ -595,7 +594,6 @@ static long hugetlbfs_fallocate(struct f
 	 * allocation routines.  If NUMA is configured, use page index
 	 * as input to create an allocation policy.
 	 */
-	memset(&pseudo_vma, 0, sizeof(struct vm_area_struct));
 	vma_init(&pseudo_vma, mm);
 	pseudo_vma.vm_flags = (VM_HUGETLB | VM_MAYSHARE | VM_SHARED);
 	pseudo_vma.vm_file = file;
diff -puN include/linux/mm.h~mm-zero-out-the-vma-in-vma_init include/linux/mm.h
--- a/include/linux/mm.h~mm-zero-out-the-vma-in-vma_init
+++ a/include/linux/mm.h
@@ -456,6 +456,7 @@ static inline void vma_init(struct vm_ar
 {
 	static const struct vm_operations_struct dummy_vm_ops = {};
 
+	memset(vma, 0, sizeof(*vma));
 	vma->vm_mm = mm;
 	vma->vm_ops = &dummy_vm_ops;
 	INIT_LIST_HEAD(&vma->anon_vma_chain);
diff -puN kernel/fork.c~mm-zero-out-the-vma-in-vma_init kernel/fork.c
--- a/kernel/fork.c~mm-zero-out-the-vma-in-vma_init
+++ a/kernel/fork.c
@@ -310,8 +310,9 @@ static struct kmem_cache *mm_cachep;
 
 struct vm_area_struct *vm_area_alloc(struct mm_struct *mm)
 {
-	struct vm_area_struct *vma = kmem_cache_zalloc(vm_area_cachep, GFP_KERNEL);
+	struct vm_area_struct *vma;
 
+	vma = kmem_cache_alloc(vm_area_cachep, GFP_KERNEL);
 	if (vma)
 		vma_init(vma, mm);
 	return vma;
diff -puN mm/mempolicy.c~mm-zero-out-the-vma-in-vma_init mm/mempolicy.c
--- a/mm/mempolicy.c~mm-zero-out-the-vma-in-vma_init
+++ a/mm/mempolicy.c
@@ -2504,7 +2504,6 @@ void mpol_shared_policy_init(struct shar
 			goto put_new;
 
 		/* Create pseudo-vma that contains just the policy */
-		memset(&pvma, 0, sizeof(struct vm_area_struct));
 		vma_init(&pvma, NULL);
 		pvma.vm_end = TASK_SIZE;	/* policy covers entire file */
 		mpol_set_shared_policy(sp, &pvma, new); /* adds ref */
diff -puN mm/shmem.c~mm-zero-out-the-vma-in-vma_init mm/shmem.c
--- a/mm/shmem.c~mm-zero-out-the-vma-in-vma_init
+++ a/mm/shmem.c
@@ -1421,7 +1421,6 @@ static void shmem_pseudo_vma_init(struct
 		struct shmem_inode_info *info, pgoff_t index)
 {
 	/* Create a pseudo vma that just contains the policy */
-	memset(vma, 0, sizeof(*vma));
 	vma_init(vma, NULL);
 	/* Bias interleave by inode number to distribute better across nodes */
 	vma->vm_pgoff = index + info->vfs_inode.i_ino;
_