| From: "Liam R. Howlett" <Liam.Howlett@Oracle.com> |
| Subject: mmap: clean up mmap_region() unrolling |
| Date: Fri, 20 Jan 2023 11:26:38 -0500 |
| |
| Move logic of unrolling to the error path as apposed to duplicating it |
| within the function body. This reduces the potential of missing an update |
| to one path when making changes. |
| |
| Link: https://lkml.kernel.org/r/20230120162650.984577-38-Liam.Howlett@oracle.com |
| Signed-off-by: Liam R. Howlett <Liam.Howlett@oracle.com> |
| Cc: Li Zetao <lizetao1@huawei.com> |
| Signed-off-by: Andrew Morton <akpm@linux-foundation.org> |
| --- |
| |
| mm/mmap.c | 47 +++++++++++++++++++---------------------------- |
| 1 file changed, 19 insertions(+), 28 deletions(-) |
| |
| --- a/mm/mmap.c~mmap-clean-up-mmap_region-unrolling |
| +++ a/mm/mmap.c |
| @@ -2601,12 +2601,11 @@ cannot_expand: |
| * Expansion is handled above, merging is handled below. |
| * Drivers should not alter the address of the VMA. |
| */ |
| - if (WARN_ON((addr != vma->vm_start))) { |
| - error = -EINVAL; |
| + error = -EINVAL; |
| + if (WARN_ON((addr != vma->vm_start))) |
| goto close_and_free_vma; |
| - } |
| - vma_iter_set(&vmi, addr); |
| |
| + vma_iter_set(&vmi, addr); |
| /* |
| * If vm_flags changed after call_mmap(), we should try merge |
| * vma again as we may succeed this time. |
| @@ -2653,25 +2652,13 @@ cannot_expand: |
| } |
| |
| /* Allow architectures to sanity-check the vm_flags */ |
| - if (!arch_validate_flags(vma->vm_flags)) { |
| - error = -EINVAL; |
| - if (file) |
| - goto close_and_free_vma; |
| - else if (vma->vm_file) |
| - goto unmap_and_free_vma; |
| - else |
| - goto free_vma; |
| - } |
| - |
| - if (vma_iter_prealloc(&vmi)) { |
| - error = -ENOMEM; |
| - if (file) |
| - goto close_and_free_vma; |
| - else if (vma->vm_file) |
| - goto unmap_and_free_vma; |
| - else |
| - goto free_vma; |
| - } |
| + error = -EINVAL; |
| + if (!arch_validate_flags(vma->vm_flags)) |
| + goto close_and_free_vma; |
| + |
| + error = -ENOMEM; |
| + if (vma_iter_prealloc(&vmi)) |
| + goto close_and_free_vma; |
| |
| if (vma->vm_file) |
| i_mmap_lock_write(vma->vm_file->f_mapping); |
| @@ -2730,14 +2717,18 @@ expanded: |
| return addr; |
| |
| close_and_free_vma: |
| - if (vma->vm_ops && vma->vm_ops->close) |
| + if (file && vma->vm_ops && vma->vm_ops->close) |
| vma->vm_ops->close(vma); |
| + |
| + if (file || vma->vm_file) { |
| unmap_and_free_vma: |
| - fput(vma->vm_file); |
| - vma->vm_file = NULL; |
| + fput(vma->vm_file); |
| + vma->vm_file = NULL; |
| |
| - /* Undo any partial mapping done by a device driver. */ |
| - unmap_region(mm, &mm->mm_mt, vma, prev, next, vma->vm_start, vma->vm_end); |
| + /* Undo any partial mapping done by a device driver. */ |
| + unmap_region(mm, &mm->mm_mt, vma, prev, next, vma->vm_start, |
| + vma->vm_end); |
| + } |
| if (file && (vm_flags & VM_SHARED)) |
| mapping_unmap_writable(file->f_mapping); |
| free_vma: |
| _ |