From 408a60eddd206134fd306dfbc53bbde093b8deb0 Mon Sep 17 00:00:00 2001 From: Wei Yang Date: Sat, 30 Nov 2019 17:50:37 -0800 Subject: mm/mmap.c: remove a never-triggered warning in __vma_adjust() The upper level of "if" makes sure (end >= next->vm_end), which means there are only two possibilities: 1) end == next->vm_end 2) end > next->vm_end remove_next is assigned to be (1 + end > next->vm_end). This means if remove_next is 1, end must equal to next->vm_end. The VM_WARN_ON will never trigger. Link: http://lkml.kernel.org/r/20190912063126.13250-1-richardw.yang@linux.intel.com Signed-off-by: Wei Yang Reviewed-by: Andrew Morton Cc: Vlastimil Babka Cc: Yang Shi Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- mm/mmap.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'mm/mmap.c') diff --git a/mm/mmap.c b/mm/mmap.c index a7d8c84d19b7..e27bc5dcd6c4 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -769,8 +769,6 @@ int __vma_adjust(struct vm_area_struct *vma, unsigned long start, remove_next = 1 + (end > next->vm_end); VM_WARN_ON(remove_next == 2 && end != next->vm_next->vm_end); - VM_WARN_ON(remove_next == 1 && - end != next->vm_end); /* trim end to next, for case 6 first pass */ end = next->vm_end; } -- cgit v1.2.3 From 93b343ab2d2fc9a22767f6eeb95c78420bfedf4a Mon Sep 17 00:00:00 2001 From: Wei Yang Date: Sat, 30 Nov 2019 17:50:43 -0800 Subject: mm/mmap.c: prev could be retrieved from vma->vm_prev Currently __vma_unlink_common handles two cases: * has_prev * or not When has_prev is false, it is obvious prev is calculated from vma->vm_prev in __vma_unlink_common. When has_prev is true, the prev is passed through from __vma_unlink_prev in __vma_adjust for non-case 8. And at the beginning next is calculated from vma->vm_next, which implies vma is next->vm_prev. The above statement sounds a little complicated, while to think in another point of view, no matter whether vma and next is swapped, the mmap link list still preserves its property. It is proper to access vma->vm_prev. Link: http://lkml.kernel.org/r/20191006012636.31521-1-richardw.yang@linux.intel.com Signed-off-by: Wei Yang Cc: Mel Gorman Cc: Vlastimil Babka Cc: Oscar Salvador Cc: Christoph Hellwig Cc: Matthew Wilcox (Oracle) Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- mm/mmap.c | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) (limited to 'mm/mmap.c') diff --git a/mm/mmap.c b/mm/mmap.c index e27bc5dcd6c4..4473c5e2c57c 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -684,23 +684,17 @@ static void __insert_vm_struct(struct mm_struct *mm, struct vm_area_struct *vma) static __always_inline void __vma_unlink_common(struct mm_struct *mm, struct vm_area_struct *vma, - struct vm_area_struct *prev, - bool has_prev, struct vm_area_struct *ignore) { - struct vm_area_struct *next; + struct vm_area_struct *prev, *next; vma_rb_erase_ignore(vma, &mm->mm_rb, ignore); next = vma->vm_next; - if (has_prev) + prev = vma->vm_prev; + if (prev) prev->vm_next = next; - else { - prev = vma->vm_prev; - if (prev) - prev->vm_next = next; - else - mm->mmap = next; - } + else + mm->mmap = next; if (next) next->vm_prev = prev; @@ -712,7 +706,7 @@ static inline void __vma_unlink_prev(struct mm_struct *mm, struct vm_area_struct *vma, struct vm_area_struct *prev) { - __vma_unlink_common(mm, vma, prev, true, vma); + __vma_unlink_common(mm, vma, vma); } /* @@ -898,7 +892,7 @@ again: * "next" (which is stored in post-swap() * "vma"). */ - __vma_unlink_common(mm, next, NULL, false, vma); + __vma_unlink_common(mm, next, vma); if (file) __remove_shared_vm_struct(next, file, mapping); } else if (insert) { -- cgit v1.2.3 From 9d81fbe09a5669acf28fccd4f51f00b43534a0c9 Mon Sep 17 00:00:00 2001 From: Wei Yang Date: Sat, 30 Nov 2019 17:50:46 -0800 Subject: mm/mmap.c: __vma_unlink_prev() is not necessary now The third parameter of __vma_unlink_common() could differentiate these two types. __vma_unlink_prev() is not necessary now. Link: http://lkml.kernel.org/r/20191006012636.31521-2-richardw.yang@linux.intel.com Signed-off-by: Wei Yang Cc: Christoph Hellwig Cc: Matthew Wilcox (Oracle) Cc: Mel Gorman Cc: Oscar Salvador Cc: Vlastimil Babka Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- mm/mmap.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) (limited to 'mm/mmap.c') diff --git a/mm/mmap.c b/mm/mmap.c index 4473c5e2c57c..270abd223681 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -702,13 +702,6 @@ static __always_inline void __vma_unlink_common(struct mm_struct *mm, vmacache_invalidate(mm); } -static inline void __vma_unlink_prev(struct mm_struct *mm, - struct vm_area_struct *vma, - struct vm_area_struct *prev) -{ - __vma_unlink_common(mm, vma, vma); -} - /* * We cannot adjust vm_start, vm_end, vm_pgoff fields of a vma that * is already present in an i_mmap tree without adjusting the tree. @@ -881,7 +874,7 @@ again: * us to remove next before dropping the locks. */ if (remove_next != 3) - __vma_unlink_prev(mm, next, vma); + __vma_unlink_common(mm, next, next); else /* * vma is not before next if they've been -- cgit v1.2.3 From 1b9fc5b24fa2e7c0e67778cda77ac231fb4bcac7 Mon Sep 17 00:00:00 2001 From: Wei Yang Date: Sat, 30 Nov 2019 17:50:49 -0800 Subject: mm/mmap.c: extract __vma_unlink_list() as counterpart for __vma_link_list() Just make the code a little easier to read. Link: http://lkml.kernel.org/r/20191006012636.31521-3-richardw.yang@linux.intel.com Signed-off-by: Wei Yang Cc: Christoph Hellwig Cc: Matthew Wilcox (Oracle) Cc: Mel Gorman Cc: Oscar Salvador Cc: Vlastimil Babka Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- mm/internal.h | 1 + mm/mmap.c | 12 +----------- mm/nommu.c | 8 +------- mm/util.c | 14 ++++++++++++++ 4 files changed, 17 insertions(+), 18 deletions(-) (limited to 'mm/mmap.c') diff --git a/mm/internal.h b/mm/internal.h index 7dd7fbb577a9..523d2a3ee923 100644 --- a/mm/internal.h +++ b/mm/internal.h @@ -291,6 +291,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); +void __vma_unlink_list(struct mm_struct *mm, struct vm_area_struct *vma); #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 270abd223681..148b175352c9 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -686,18 +686,8 @@ static __always_inline void __vma_unlink_common(struct mm_struct *mm, struct vm_area_struct *vma, struct vm_area_struct *ignore) { - struct vm_area_struct *prev, *next; - vma_rb_erase_ignore(vma, &mm->mm_rb, ignore); - next = vma->vm_next; - prev = vma->vm_prev; - if (prev) - prev->vm_next = next; - else - mm->mmap = next; - if (next) - next->vm_prev = prev; - + __vma_unlink_list(mm, vma); /* Kill the cache */ vmacache_invalidate(mm); } diff --git a/mm/nommu.c b/mm/nommu.c index 7de592058ab4..47a58b32fdc9 100644 --- a/mm/nommu.c +++ b/mm/nommu.c @@ -684,13 +684,7 @@ static void delete_vma_from_mm(struct vm_area_struct *vma) /* remove from the MM's tree and list */ rb_erase(&vma->vm_rb, &mm->mm_rb); - if (vma->vm_prev) - vma->vm_prev->vm_next = vma->vm_next; - else - mm->mmap = vma->vm_next; - - if (vma->vm_next) - vma->vm_next->vm_prev = vma->vm_prev; + __vma_unlink_list(mm, vma); } /* diff --git a/mm/util.c b/mm/util.c index 3ad6db9a722e..7fbaadb7fb1f 100644 --- a/mm/util.c +++ b/mm/util.c @@ -292,6 +292,20 @@ void __vma_link_list(struct mm_struct *mm, struct vm_area_struct *vma, next->vm_prev = vma; } +void __vma_unlink_list(struct mm_struct *mm, struct vm_area_struct *vma) +{ + struct vm_area_struct *prev, *next; + + next = vma->vm_next; + prev = vma->vm_prev; + if (prev) + prev->vm_next = next; + else + mm->mmap = next; + if (next) + next->vm_prev = prev; +} + /* Check if the vma is being used as a stack by this task */ int vma_is_stack_for_current(struct vm_area_struct *vma) { -- cgit v1.2.3 From aba6dfb75fe15650991442efd137c32fbf2e2b85 Mon Sep 17 00:00:00 2001 From: Wei Yang Date: Sat, 30 Nov 2019 17:50:53 -0800 Subject: mm/mmap.c: rb_parent is not necessary in __vma_link_list() 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. Link: http://lkml.kernel.org/r/20190813032656.16625-1-richardw.yang@linux.intel.com Signed-off-by: Wei Yang Acked-by: Andrew Morton Cc: Mel Gorman Cc: Vlastimil Babka Cc: Matthew Wilcox Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- mm/internal.h | 2 +- mm/mmap.c | 2 +- mm/nommu.c | 2 +- mm/util.c | 8 ++------ 4 files changed, 5 insertions(+), 9 deletions(-) (limited to 'mm/mmap.c') diff --git a/mm/internal.h b/mm/internal.h index 523d2a3ee923..a246c516ade2 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); void __vma_unlink_list(struct mm_struct *mm, struct vm_area_struct *vma); #ifdef CONFIG_MMU diff --git a/mm/mmap.c b/mm/mmap.c index 148b175352c9..311b08f780ce 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -641,7 +641,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 47a58b32fdc9..bd2b4e5ef144 100644 --- a/mm/nommu.c +++ b/mm/nommu.c @@ -648,7 +648,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 7fbaadb7fb1f..988d11e6c17c 100644 --- a/mm/util.c +++ b/mm/util.c @@ -271,7 +271,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; @@ -280,12 +280,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) -- cgit v1.2.3 From ff68dac6d65cd1347dad5d780dd8c90f29dc1b0b Mon Sep 17 00:00:00 2001 From: Gaowei Pu Date: Sat, 30 Nov 2019 17:51:03 -0800 Subject: mm/mmap.c: use IS_ERR_VALUE to check return value of get_unmapped_area MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit get_unmapped_area() returns an address or -errno on failure. Historically we have checked for the failure by offset_in_page() which is correct but quite hard to read. Newer code started using IS_ERR_VALUE which is much easier to read. Convert remaining users of offset_in_page as well. [mhocko@suse.com: rewrite changelog] [mhocko@kernel.org: fix mremap.c and uprobes.c sites also] Link: http://lkml.kernel.org/r/20191012102512.28051-1-pugaowei@gmail.com Signed-off-by: Gaowei Pu Reviewed-by: Andrew Morton Acked-by: Michal Hocko Cc: Vlastimil Babka Cc: Wei Yang Cc: Konstantin Khlebnikov Cc: Kirill A. Shutemov Cc: "Jérôme Glisse" Cc: Mike Kravetz Cc: Rik van Riel Cc: Qian Cai Cc: Shakeel Butt Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- kernel/events/uprobes.c | 2 +- mm/mmap.c | 9 +++++---- mm/mremap.c | 4 ++-- 3 files changed, 8 insertions(+), 7 deletions(-) (limited to 'mm/mmap.c') diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c index c74761004ee5..ece7e13f6e4a 100644 --- a/kernel/events/uprobes.c +++ b/kernel/events/uprobes.c @@ -1457,7 +1457,7 @@ static int xol_add_vma(struct mm_struct *mm, struct xol_area *area) /* Try to map as high as possible, this is only a hint. */ area->vaddr = get_unmapped_area(NULL, TASK_SIZE - PAGE_SIZE, PAGE_SIZE, 0, 0); - if (area->vaddr & ~PAGE_MASK) { + if (IS_ERR_VALUE(area->vaddr)) { ret = area->vaddr; goto fail; } diff --git a/mm/mmap.c b/mm/mmap.c index 311b08f780ce..b9d0c2f3f6bf 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -1417,7 +1417,7 @@ unsigned long do_mmap(struct file *file, unsigned long addr, * that it represents a valid section of the address space. */ addr = get_unmapped_area(file, addr, len, pgoff, flags); - if (offset_in_page(addr)) + if (IS_ERR_VALUE(addr)) return addr; if (flags & MAP_FIXED_NOREPLACE) { @@ -2981,15 +2981,16 @@ static int do_brk_flags(unsigned long addr, unsigned long len, unsigned long fla struct rb_node **rb_link, *rb_parent; pgoff_t pgoff = addr >> PAGE_SHIFT; int error; + unsigned long mapped_addr; /* Until we need other flags, refuse anything except VM_EXEC. */ if ((flags & (~VM_EXEC)) != 0) return -EINVAL; flags |= VM_DATA_DEFAULT_FLAGS | VM_ACCOUNT | mm->def_flags; - error = get_unmapped_area(NULL, addr, len, 0, MAP_FIXED); - if (offset_in_page(error)) - return error; + mapped_addr = get_unmapped_area(NULL, addr, len, 0, MAP_FIXED); + if (IS_ERR_VALUE(mapped_addr)) + return mapped_addr; error = mlock_future_check(mm, mm->def_flags, len); if (error) diff --git a/mm/mremap.c b/mm/mremap.c index 1fc8a29fbe3f..122938dcec15 100644 --- a/mm/mremap.c +++ b/mm/mremap.c @@ -558,7 +558,7 @@ static unsigned long mremap_to(unsigned long addr, unsigned long old_len, ret = get_unmapped_area(vma->vm_file, new_addr, new_len, vma->vm_pgoff + ((addr - vma->vm_start) >> PAGE_SHIFT), map_flags); - if (offset_in_page(ret)) + if (IS_ERR_VALUE(ret)) goto out1; ret = move_vma(vma, addr, old_len, new_len, new_addr, locked, uf, @@ -706,7 +706,7 @@ SYSCALL_DEFINE5(mremap, unsigned long, addr, unsigned long, old_len, vma->vm_pgoff + ((addr - vma->vm_start) >> PAGE_SHIFT), map_flags); - if (offset_in_page(new_addr)) { + if (IS_ERR_VALUE(new_addr)) { ret = new_addr; goto out; } -- cgit v1.2.3 From 5d42ab293f5181609ea18f1f2ab85cd4cfc8efb2 Mon Sep 17 00:00:00 2001 From: Wei Yang Date: Sat, 30 Nov 2019 17:57:39 -0800 Subject: mm/mmap.c: make vma_merge() comment more easy to understand Case 1/6, 2/7 and 3/8 have the same pattern and we handle them in the same logic. Rearrange the comment to make it a little easy for audience to understand. Link: http://lkml.kernel.org/r/20191030012445.16944-1-richardw.yang@linux.intel.com Signed-off-by: Wei Yang Cc: Mike Rapoport Cc: Will Deacon Cc: Michal Hocko Cc: Catalin Marinas Cc: Andrea Arcangeli Cc: Jann Horn Cc: Darrick J. Wong Cc: Steve Capper Cc: Michel Lespinasse Cc: Dave Hansen Cc: Yangtao Li Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- mm/mmap.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) (limited to 'mm/mmap.c') diff --git a/mm/mmap.c b/mm/mmap.c index b9d0c2f3f6bf..9c648524e4dc 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -1091,15 +1091,18 @@ can_vma_merge_after(struct vm_area_struct *vma, unsigned long vm_flags, * the area passed down from mprotect_fixup, never extending beyond one * vma, PPPPPP is the prev vma specified, and NNNNNN the next vma after: * - * AAAA AAAA AAAA AAAA - * PPPPPPNNNNNN PPPPPPNNNNNN PPPPPPNNNNNN PPPPNNNNXXXX - * cannot merge might become might become might become - * PPNNNNNNNNNN PPPPPPPPPPNN PPPPPPPPPPPP 6 or - * mmap, brk or case 4 below case 5 below PPPPPPPPXXXX 7 or - * mremap move: PPPPXXXXXXXX 8 - * AAAA - * PPPP NNNN PPPPPPPPPPPP PPPPPPPPNNNN PPPPNNNNNNNN - * might become case 1 below case 2 below case 3 below + * AAAA AAAA AAAA + * PPPPPPNNNNNN PPPPPPNNNNNN PPPPPPNNNNNN + * cannot merge might become might become + * PPNNNNNNNNNN PPPPPPPPPPNN + * mmap, brk or case 4 below case 5 below + * mremap move: + * AAAA AAAA + * PPPP NNNN PPPPNNNNXXXX + * might become might become + * PPPPPPPPPPPP 1 or PPPPPPPPPPPP 6 or + * PPPPPPPPNNNN 2 or PPPPPPPPXXXX 7 or + * PPPPNNNNNNNN 3 PPPPXXXXXXXX 8 * * It is important for case 8 that the vma NNNN overlapping the * region AAAA is never going to extended over XXXX. Instead XXXX must -- cgit v1.2.3