Re: [PATCH] mm/mmap.c: rb_parent is not necessary in __vma_link_list

[Wei Yang]

On Tue, Aug 13, 2019 at 11:26:56AM +0800, Wei Yang wrote:
>Now we use rb_parent to get next, while this is not necessary.
>
>When prev is NULL, this means vma should be the first element in the
>list. Then next should be current first one (mm->mmap), no matter
>whether we have parent or not.
>
>After removing it, the code shows the beauty of symmetry.
>
>Signed-off-by: Wei Yang <richardw.yang@xxxxxxxxxxxxxxx>

Ping~

>---
> mm/internal.h | 2 +-
> mm/mmap.c     | 2 +-
> mm/nommu.c    | 2 +-
> mm/util.c     | 8 ++------
> 4 files changed, 5 insertions(+), 9 deletions(-)
>
>diff --git a/mm/internal.h b/mm/internal.h
>index e32390802fd3..41a49574acc3 100644
>--- a/mm/internal.h
>+++ b/mm/internal.h
>@@ -290,7 +290,7 @@ static inline bool is_data_mapping(vm_flags_t flags)
> 
> /* mm/util.c */
> void __vma_link_list(struct mm_struct *mm, struct vm_area_struct *vma,
>-		struct vm_area_struct *prev, struct rb_node *rb_parent);
>+		struct vm_area_struct *prev);
> 
> #ifdef CONFIG_MMU
> extern long populate_vma_page_range(struct vm_area_struct *vma,
>diff --git a/mm/mmap.c b/mm/mmap.c
>index f7ed0afb994c..b8072630766f 100644
>--- a/mm/mmap.c
>+++ b/mm/mmap.c
>@@ -632,7 +632,7 @@ __vma_link(struct mm_struct *mm, struct vm_area_struct *vma,
> 	struct vm_area_struct *prev, struct rb_node **rb_link,
> 	struct rb_node *rb_parent)
> {
>-	__vma_link_list(mm, vma, prev, rb_parent);
>+	__vma_link_list(mm, vma, prev);
> 	__vma_link_rb(mm, vma, rb_link, rb_parent);
> }
> 
>diff --git a/mm/nommu.c b/mm/nommu.c
>index fed1b6e9c89b..12a66fbeb988 100644
>--- a/mm/nommu.c
>+++ b/mm/nommu.c
>@@ -637,7 +637,7 @@ static void add_vma_to_mm(struct mm_struct *mm, struct vm_area_struct *vma)
> 	if (rb_prev)
> 		prev = rb_entry(rb_prev, struct vm_area_struct, vm_rb);
> 
>-	__vma_link_list(mm, vma, prev, parent);
>+	__vma_link_list(mm, vma, prev);
> }
> 
> /*
>diff --git a/mm/util.c b/mm/util.c
>index e6351a80f248..80632db29247 100644
>--- a/mm/util.c
>+++ b/mm/util.c
>@@ -264,7 +264,7 @@ void *memdup_user_nul(const void __user *src, size_t len)
> EXPORT_SYMBOL(memdup_user_nul);
> 
> void __vma_link_list(struct mm_struct *mm, struct vm_area_struct *vma,
>-		struct vm_area_struct *prev, struct rb_node *rb_parent)
>+		struct vm_area_struct *prev)
> {
> 	struct vm_area_struct *next;
> 
>@@ -273,12 +273,8 @@ void __vma_link_list(struct mm_struct *mm, struct vm_area_struct *vma,
> 		next = prev->vm_next;
> 		prev->vm_next = vma;
> 	} else {
>+		next = mm->mmap;
> 		mm->mmap = vma;
>-		if (rb_parent)
>-			next = rb_entry(rb_parent,
>-					struct vm_area_struct, vm_rb);
>-		else
>-			next = NULL;
> 	}
> 	vma->vm_next = next;
> 	if (next)
>-- 
>2.17.1

-- 
Wei Yang
Help you, Help me